file uploads/send file fix #340

Author: jknack

coverage-report/src/test/java/org/jooby/DownloadFeature.java

@@ -8,6 +8,11 @@
 public class DownloadFeature extends ServerFeature {
 
   {
+    get("/content-disposition", (req, rsp) -> rsp
+        .header("content-disposition", "attachment; filename=myfile.txt;")
+        .download("name.js", new File("src/test/resources/"
+            + DownloadFeature.class.getName().replace('.', '/') + ".js")));
+
     get("/download-reader", (req, rsp) -> rsp.download("name.js",
         new File("src/test/resources/"
             + DownloadFeature.class.getName().replace('.', '/') + ".js")));
@@ -20,7 +25,8 @@ public class DownloadFeature extends ServerFeature {
 
     get("/location", (req, rsp) -> rsp.download("name", req.param("file").value()));
 
-    get("/file", (req, rsp) -> rsp.download(DownloadFeature.class.getName().replace('.', '/') + ".json"));
+    get("/file",
+        (req, rsp) -> rsp.download(DownloadFeature.class.getName().replace('.', '/') + ".json"));
 
     get("/fs", (req, rsp) -> rsp.download(new File(req.param("file").value())));
 
@@ -39,6 +45,16 @@ public void downloadReader() throws Exception {
         .header("Content-Type", "application/javascript;charset=UTF-8");
   }
 
+  @Test
+  public void contentDisposition() throws Exception {
+    request()
+        .get("/content-disposition")
+        .expect(200)
+        .header("Content-Disposition", "attachment; filename=myfile.txt;")
+        .header("Content-Length", "20")
+        .header("Content-Type", "application/javascript;charset=UTF-8");
+  }
+
   @Test
   public void downloadBinWithLocation() throws Exception {
     request()
@@ -54,7 +70,8 @@ public void downloadBinWithFsLocation() throws Exception {
     request()
         .get("/fs?file=src/test/resources/" + getClass().getName().replace('.', '/') + ".ico")
         .expect(200)
-        .header("Content-Disposition", "attachment; filename=\"DownloadFeature.ico\"; filename*=utf-8''DownloadFeature.ico")
+        .header("Content-Disposition",
+            "attachment; filename=\"DownloadFeature.ico\"; filename*=utf-8''DownloadFeature.ico")
         .header("Content-Length", "2238")
         .header("Content-Type", "image/x-icon");
   }
@@ -82,7 +99,8 @@ public void downloadTextWithFsLocation() throws Exception {
     request()
         .get("/fs?file=src/test/resources/" + getClass().getName().replace('.', '/') + ".js")
         .expect("(function () {})();\n")
-        .header("Content-Disposition", "attachment; filename=\"DownloadFeature.js\"; filename*=utf-8''DownloadFeature.js")
+        .header("Content-Disposition",
+            "attachment; filename=\"DownloadFeature.js\"; filename*=utf-8''DownloadFeature.js")
         .header("Content-Length", "20")
         .header("Content-Type", "application/javascript;charset=UTF-8");
   }
@@ -92,7 +110,8 @@ public void customType() throws Exception {
     request()
         .get("/customtype?type=json")
         .expect("{}\n")
-        .header("Content-Disposition", "attachment; filename=\"name.json\"; filename*=utf-8''name.json")
+        .header("Content-Disposition",
+            "attachment; filename=\"name.json\"; filename*=utf-8''name.json")
         .header("Content-Length", "3")
         .header("Content-Type", "application/json;charset=UTF-8");
   }
@@ -102,7 +121,8 @@ public void downladInputStream() throws Exception {
     request()
         .get("/favicon.ico")
         .expect(200)
-        .header("Content-Disposition", "attachment; filename=\"favicon.ico\"; filename*=utf-8''favicon.ico")
+        .header("Content-Disposition",
+            "attachment; filename=\"favicon.ico\"; filename*=utf-8''favicon.ico")
         .header("Content-Length", "2238")
         .header("Content-Type", "image/x-icon");
   }
@@ -112,7 +132,8 @@ public void download() throws Exception {
     request()
         .get("/file")
         .expect(200)
-        .header("Content-Disposition", "attachment; filename=\"downloadfeature.json\"; filename*=utf-8''downloadfeature.json")
+        .header("Content-Disposition",
+            "attachment; filename=\"downloadfeature.json\"; filename*=utf-8''downloadfeature.json")
         .header("Content-Length", "3")
         .header("Content-Type", "application/json;charset=utf-8");
   }

coverage-report/src/test/java/org/jooby/MultipartFormParamFeature.java

@@ -67,7 +67,7 @@ public String optional(final Optional<Upload> upload) throws IOException {
     });
 
     post("/form/files", (req, rsp) -> {
-      List<Upload> uploads = req.param("uploads").toList(Upload.class);
+      List<Upload> uploads = req.files("uploads");
       StringBuilder buffer = new StringBuilder();
       for (Upload upload : uploads) {
         try (Upload u = upload) {
@@ -78,7 +78,7 @@ public String optional(final Optional<Upload> upload) throws IOException {
     });
 
     post("/form/use/file", (req, rsp) -> {
-      Upload upload = req.param("myfile").toUpload();
+      Upload upload = req.file("myfile");
       File file = upload.file();
       try (Upload u = upload) {
         assertEquals("p=1", Files.readAllLines(file.toPath()).stream()

jooby/src/main/java/org/jooby/Request.java

@@ -138,6 +138,16 @@ public Mutant param(final String name) {
       return req.param(name);
     }
 
+    @Override
+    public Upload file(final String name) {
+      return req.file(name);
+    }
+
+    @Override
+    public List<Upload> files(final String name) {
+      return req.files(name);
+    }
+
     @Override
     public Mutant header(final String name) {
       return req.header(name);
@@ -548,6 +558,28 @@ default Optional<MediaType> accepts(final MediaType... types) {
    */
   Mutant param(String name);
 
+  /**
+   * Get a file {@link Upload} with the given name. The request must be a POST with
+   * <code>multipart/form-data</code> content-type.
+   *
+   * @param name File's name.
+   * @return An {@link Upload}.
+   */
+  default Upload file(final String name) {
+    return param(name).toUpload();
+  }
+
+  /**
+   * Get a list of file {@link Upload} with the given name. The request must be a POST with
+   * <code>multipart/form-data</code> content-type.
+   *
+   * @param name File's name.
+   * @return A list of {@link Upload}.
+   */
+  default List<Upload> files(final String name) {
+    return param(name).toList(Upload.class);
+  }
+
   /**
    * Get a HTTP header.
    *

jooby/src/main/java/org/jooby/internal/RequestImpl.java

@@ -56,6 +56,8 @@
 import com.google.inject.Injector;
 import com.google.inject.Key;
 
+import javaslang.control.Try;
+
 public class RequestImpl implements Request {
 
   private final Map<String, Mutant> params = new HashMap<>();
@@ -182,7 +184,8 @@ public Mutant params() {
   public Mutant param(final String name) {
     Mutant param = this.params.get(name);
     if (param == null) {
-      List<NativeUpload> files = files(name);
+      List<NativeUpload> files = Try.of(() -> req.files(name)).getOrElseThrow(
+          ex -> new Err(Status.BAD_REQUEST, "Upload " + name + " resulted in error", ex));
       if (files.size() > 0) {
         List<Upload> uploads = files.stream()
             .map(upload -> new UploadImpl(injector, upload))
@@ -276,13 +279,13 @@ public long length() {
   @Override
   public List<Locale> locales(
       final BiFunction<List<Locale.LanguageRange>, List<Locale>, List<Locale>> filter) {
-    return lang.map(h ->  filter.apply(LocaleUtils.range(h), locales))
+    return lang.map(h -> filter.apply(LocaleUtils.range(h), locales))
         .orElseGet(() -> filter.apply(ImmutableList.of(), locales));
   }
 
   @Override
   public Locale locale(final BiFunction<List<LanguageRange>, List<Locale>, Locale> filter) {
-    return lang.map(h ->  filter.apply(LocaleUtils.range(h), locales))
+    return lang.map(h -> filter.apply(LocaleUtils.range(h), locales))
         .orElseGet(() -> filter.apply(ImmutableList.of(), locales));
   }
 
@@ -369,14 +372,6 @@ public String toString() {
     return route().toString();
   }
 
-  private List<NativeUpload> files(final String name) {
-    try {
-      return req.files(name);
-    } catch (Exception ex) {
-      throw new Err(Status.BAD_REQUEST, "Upload " + name + " resulted in error", ex);
-    }
-  }
-
   private List<String> paramNames() {
     try {
       return req.paramNames();

jooby/src/main/java/org/jooby/internal/ResponseImpl.java

@@ -99,8 +99,7 @@ public void download(final String filename, final InputStream stream) throws Exc
     requireNonNull(stream, "A stream is required.");
 
     // handle type
-    MediaType type = MediaType.byPath(filename).orElse(MediaType.octetstream);
-    type(type().orElseGet(() -> type));
+    type(type().orElseGet(() -> MediaType.byPath(filename).orElse(MediaType.octetstream)));
 
     Asset asset = new InputStreamAsset(stream, filename, type().get());
     contentDisposition(filename);
@@ -114,9 +113,8 @@ public void download(final String filename, final String location) throws Except
       throw new FileNotFoundException(location);
     }
     // handle type
-    MediaType type = MediaType.byPath(filename).orElse(MediaType.byPath(location)
-        .orElse(MediaType.octetstream));
-    type(type().orElseGet(() -> type));
+    type(type().orElseGet(() -> MediaType.byPath(filename).orElse(MediaType.byPath(location)
+        .orElse(MediaType.octetstream))));
 
     URLAsset asset = new URLAsset(url, location, type().get());
     length(asset.length());
@@ -341,15 +339,18 @@ void route(final Route route) {
   }
 
   private void contentDisposition(final String filename) throws IOException {
-    String basename = filename;
-    int last = filename.lastIndexOf('/');
-    if (last >= 0) {
-      basename = basename.substring(last + 1);
-    }
+    List<String> headers = rsp.headers("Content-Disposition");
+    if (headers.isEmpty()) {
+      String basename = filename;
+      int last = filename.lastIndexOf('/');
+      if (last >= 0) {
+        basename = basename.substring(last + 1);
+      }
 
-    String ebasename = URLEncoder.encode(basename, charset.name()).replaceAll("\\+", "%20");
-    header("Content-Disposition",
-        String.format(CONTENT_DISPOSITION, basename, charset.name(), ebasename));
+      String cs = charset.name();
+      String ebasename = URLEncoder.encode(basename, cs).replaceAll("\\+", "%20");
+      header("Content-Disposition", String.format(CONTENT_DISPOSITION, basename, cs, ebasename));
+    }
   }
 
   @SuppressWarnings("unchecked")

jooby/src/test/java/org/jooby/RequestForwardingTest.java

@@ -341,6 +341,33 @@ public void charset() throws Exception {
         });
   }
 
+  @Test
+  public void file() throws Exception {
+    new MockUnit(Request.class, Upload.class)
+        .expect(unit -> {
+          Request req = unit.get(Request.class);
+          expect(req.file("f")).andReturn(unit.get(Upload.class));
+        })
+        .run(unit -> {
+          assertEquals(unit.get(Upload.class),
+              new Request.Forwarding(unit.get(Request.class)).file("f"));
+        });
+  }
+
+  @SuppressWarnings("unchecked")
+  @Test
+  public void files() throws Exception {
+    new MockUnit(Request.class, List.class)
+        .expect(unit -> {
+          Request req = unit.get(Request.class);
+          expect(req.files("f")).andReturn(unit.get(List.class));
+        })
+        .run(unit -> {
+          assertEquals(unit.get(List.class),
+              new Request.Forwarding(unit.get(Request.class)).files("f"));
+        });
+  }
+
   @Test
   public void length() throws Exception {
     new MockUnit(Request.class)

md/req.md

@@ -2,7 +2,7 @@
 
 The request object contains methods for reading params, headers and body (between others). In the next section we will mention the most important method of a request object, if you need more information please refer to the [javadoc]({{apidocs}}/org/jooby/Request.html).
 
-## request params
+## params
 
 Retrieval of param is done via: [req.param("name")]({{apidocs}}/org/jooby/Request.html#param-java.lang.String-) method.
 
@@ -142,7 +142,7 @@ get("/", req -> {
 });
 ```
 
-## request headers
+## headers
 
 Retrieval of request headers is done via: [request.header("name")]({{}}Request.html#header-java.lang.String-). All the explained before for [request params](#request-request-params) apply for headers too.
 
@@ -172,7 +172,7 @@ get("/", req -> {
 });
 ```
 
-## request body
+## body
 
 Retrieval of request body is done via [request.body()]({{defdocs}}/Request.html#body--).
 
@@ -202,9 +202,39 @@ public void configure(Mode mode, Config config, Binder binder) {
 }
 ```
 
-## local variables
+## file upload
 
-Local variables are bound to the current request. They are created every time a new request comes in and destroyed at the end of the request.
+File uploads are accessible via: [request.file(name)]({{defdocs}}/Request.html#file-java.lang.String-):
+
+```html
+<form enctype="multipart/form-data" action="/upload" method="post">
+  <input name="myfile" type="file"/>
+</form>
+```
+
+```java
+// Script API
+
+{
+   post("/upload", req -> {
+     Upload upload = req.file("myfile");
+     ...
+   });
+}
+
+// MVC API
+
+class Controller {
+
+  @Path("/upload") @POST
+  public Object upload(Upload myfile) {
+  }
+}
+```
+
+## locals
+
+Local attributes are bound to the current request. They are created every time a new request comes in and destroyed at the end of the request.
 
 ```java
 {