Merge pull request #883 from xael-fry/1564_zeroLenghtFileUpload

[#1564] fix(upload): allow upload zero size file
playframework · Jun 24, 2015 · 21af232 · 21af232
2 parents ea77a3e + 304121d
commit 21af232
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 28 deletions.
diff --git a/framework/src/play/data/FileUpload.java b/framework/src/play/data/FileUpload.java
@@ -4,8 +4,10 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.io.FilenameUtils;
+
 import play.data.parsing.TempFilePlugin;
 import play.exceptions.UnexpectedException;
 import play.libs.Files;
@@ -23,22 +25,27 @@ public FileUpload() {
     public FileUpload(FileItem fileItem) {
         this.fileItem = fileItem;
         File tmp = TempFilePlugin.createTempFolder();
-        defaultFile = new File(tmp, FilenameUtils.getName(fileItem.getFieldName()) + File.separator + FilenameUtils.getName(fileItem.getName()));
-        try {
-            if(!defaultFile.getCanonicalPath().startsWith(tmp.getCanonicalPath())) {
-                throw new IOException("Temp file try to override existing file?");
+        // Check that the file has a name to avoid to override the field folder
+        if (fileItem.getName().trim().length() > 0) {
+            defaultFile = new File(tmp, FilenameUtils.getName(fileItem.getFieldName()) + File.separator
+                    + FilenameUtils.getName(fileItem.getName()));
+            try {
+                if (!defaultFile.getCanonicalPath().startsWith(tmp.getCanonicalPath())) {
+                    throw new IOException("Temp file try to override existing file?");
+                }
+                defaultFile.getParentFile().mkdirs();
+                fileItem.write(defaultFile);
+            } catch (Exception e) {
+                throw new IllegalStateException("Error when trying to write to file " + defaultFile.getAbsolutePath(), e);
             }
-            defaultFile.getParentFile().mkdirs();
-            fileItem.write(defaultFile);
-        } catch (Exception e) {
-            throw new IllegalStateException("Error when trying to write to file " + defaultFile.getAbsolutePath(), e);
         }
     }
 
+    @Override
     public File asFile() {
         return defaultFile;
     }
-    
+
     public File asFile(File file) {
         try {
             Files.copy(defaultFile, file);
@@ -47,39 +54,46 @@ public File asFile(File file) {
             throw new UnexpectedException(ex);
         }
     }
-    
+
     public File asFile(String name) {
         return asFile(new File(name));
     }
 
+    @Override
     public byte[] asBytes() {
         return IO.readContent(defaultFile);
     }
 
+    @Override
     public InputStream asStream() {
         try {
             return new FileInputStream(defaultFile);
         } catch (IOException ex) {
             throw new UnexpectedException(ex);
         }
     }
-
+
+    @Override
     public String getContentType() {
         return fileItem.getContentType();
     }
 
+    @Override
     public String getFileName() {
         return fileItem.getName();
     }
 
+    @Override
     public String getFieldName() {
         return fileItem.getFieldName();
     }
 
+    @Override
     public Long getSize() {
         return defaultFile.length();
     }
-
+
+    @Override
     public boolean isInMemory() {
         return fileItem.isInMemory();
     }

diff --git a/framework/src/play/data/binding/types/FileBinder.java b/framework/src/play/data/binding/types/FileBinder.java
@@ -1,18 +1,20 @@
 package play.data.binding.types;
 
-import play.data.binding.TypeBinder;
 import java.io.File;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.List;
+
 import play.data.Upload;
+import play.data.binding.TypeBinder;
 import play.mvc.Http.Request;
 
 /**
  * Bind file form multipart/form-data request.
  */
 public class FileBinder implements TypeBinder<File> {
 
+    @Override
     @SuppressWarnings("unchecked")
     public File bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) {
         if (value == null || value.trim().length() == 0) {
@@ -21,14 +23,13 @@ public File bind(String name, Annotation[] annotations, String value, Class actu
         Request req = Request.current();
         if (req != null && req.args != null) {
             List<Upload> uploads = (List<Upload>) req.args.get("__UPLOADS");
-            if(uploads != null){
+            if (uploads != null) {
                 for (Upload upload : uploads) {
                     if (upload.getFieldName().equals(value)) {
-                        File file = upload.asFile();
-                        if (file.length() > 0) {
+                        if (upload.getFileName().trim().length() > 0) {
+                            File file = upload.asFile();
                             return file;
                         }
-                        return null;
                     }
                 }
             }

diff --git a/framework/src/play/data/binding/types/UploadBinder.java b/framework/src/play/data/binding/types/UploadBinder.java
@@ -1,5 +1,9 @@
 package play.data.binding.types;
 
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.List;
+
 import play.Logger;
 import play.data.Upload;
 import play.data.binding.Binder;
@@ -9,12 +13,9 @@
 import play.mvc.Http.Request;
 import play.mvc.Scope.Params;
 
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Type;
-import java.util.List;
-
 public class UploadBinder implements TypeBinder<Model.BinaryField> {
 
+    @Override
     @SuppressWarnings("unchecked")
     public Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) {
         if (value == null || value.trim().length() == 0) {
@@ -24,15 +25,15 @@ public Object bind(String name, Annotation[] annotations, String value, Class ac
             Request req = Request.current();
             if (req != null && req.args != null) {
                 List<Upload> uploads = (List<Upload>) req.args.get("__UPLOADS");
-                if(uploads != null){
+                if (uploads != null) {
                     for (Upload upload : uploads) {
-                        if (upload.getFieldName().equals(value) && upload.getSize() > 0) {
+                        if (upload.getFieldName().equals(value) && upload.getFileName().trim().length() > 0) {
                             return upload;
                         }
                     }
                 }
             }
-            
+
             if (Params.current() != null && Params.current().get(value + "_delete_") != null) {
                 return null;
             }

diff --git a/samples-and-tests/just-test-cases/app/controllers/Binary.java b/samples-and-tests/just-test-cases/app/controllers/Binary.java
@@ -41,13 +41,21 @@ public static void showAvatar(Long id) {
     }
 
     public static void uploadFile(File file) {
-        Http.Response.current().headers.put("Content-Length", new Http.Header("Content-Length", String.valueOf(file.length())));
-        renderBinary(file);
+    	if(file != null){
+    		Http.Response.current().headers.put("Content-Length", new Http.Header("Content-Length", String.valueOf(file.length())));
+    		renderBinary(file);
+    	}else{
+    		renderText("File is null");
+    	}
     }
 
     public static void upload(Upload upload) {
-        Http.Response.current().headers.put("Content-Length", new Http.Header("Content-Length", String.valueOf(upload.asBytes().length)));
-        renderBinary(upload.asFile());
+    	if(upload != null){
+	        Http.Response.current().headers.put("Content-Length", new Http.Header("Content-Length", String.valueOf(upload.asBytes().length)));
+	        renderBinary(upload.asFile());
+        }else{
+    		renderText("Upload is null");
+        }
     }
 
     public static void uploadMultipleFiles(List<File> files) {

diff --git a/samples-and-tests/just-test-cases/test/BinaryTest.java b/samples-and-tests/just-test-cases/test/BinaryTest.java
@@ -130,6 +130,71 @@ public void testUploadSmallFile2() {
         assertEquals("Size does not match", "2440", size);
     }
 
+    @Test
+    public void testUploadZeroLenghtFile() {
+
+        Map<String,String> parameters= new HashMap<String,String>();
+
+        Map<String, File> files= new HashMap<String, File>();
+        File file = Play.getFile("test/zeroLenghtFile.txt");
+        assertTrue(file.exists());
+        files.put("file", file);
+        Response uploadResponse = POST("/Binary/uploadFile", parameters, files);
+
+        assertStatus(200, uploadResponse);
+
+        String size = uploadResponse.getHeader("Content-Length");
+
+        assertEquals("Size does not match", "0", size);
+    }
+
+    @Test
+    public void testUploadZeroLenghtFile2() {
+
+        Map<String,String> parameters= new HashMap<String,String>();
+
+        Map<String, File> files= new HashMap<String, File>();
+        File file = Play.getFile("test/zeroLenghtFile.txt");
+        assertTrue(file.exists());
+
+        files.put("upload", file);
+        Response uploadResponse = POST("/Binary/upload", parameters, files);
+
+        assertStatus(200, uploadResponse);
+
+        String size = uploadResponse.getHeader("Content-Length");
+
+        assertEquals("Size does not match", "0", size);
+    }
+
+    @Test
+    public void testUploadNoFile() {
+
+        Map<String,String> parameters= new HashMap<String,String>();
+
+        Map<String, File> files= new HashMap<String, File>();
+        files.put("file", null);
+        Response uploadResponse = POST("/Binary/uploadFile", parameters, files);
+
+        assertStatus(200, uploadResponse);
+
+        assertContentMatch("File is null" , uploadResponse);
+    }
+
+    @Test
+    public void testUploadNoFile2() {
+
+        Map<String,String> parameters= new HashMap<String,String>();
+
+        Map<String, File> files= new HashMap<String, File>();
+        files.put("upload", null);
+        Response uploadResponse = POST("/Binary/upload", parameters, files);
+
+        assertStatus(200, uploadResponse);
+
+        assertContentMatch("Upload is null" , uploadResponse);
+    }
+
 //  TODO: Missing possibility to upload multiple files at once
 //  See: http://play.lighthouseapp.com/projects/57987-play-framework/tickets/472-functionaltest-and-ws-client-library-dont-allow-upload-of-multiple-file#ticket-472-2 
 //    @Test

diff --git a/samples-and-tests/just-test-cases/test/zeroLenghtFile.txt b/samples-and-tests/just-test-cases/test/zeroLenghtFile.txt