Format UTF-8 filenames correctly for download (#16556)

The code is the same for both FileDownloader and DownloadStream except
that FileDownloader forces the content-type to be an "attachment".

Change-Id: I50abf3b0f019b773bc0a44b16536a9479f9f472f

5 files changed, 93 insertions, 41 deletions

diff --git a/server/src/com/vaadin/server/DownloadStream.java b/server/src/com/vaadin/server/DownloadStream.java
index 681c438967..0dfd9e42c3 100644
--- a/server/src/com/vaadin/server/DownloadStream.java
+++ b/server/src/com/vaadin/server/DownloadStream.java
@@ -20,9 +20,12 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.Serializable;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.logging.Logger;
 
 import javax.servlet.http.HttpServletResponse;
 
@@ -40,6 +43,8 @@ import javax.servlet.http.HttpServletResponse;
 @SuppressWarnings("serial")
 public class DownloadStream implements Serializable {
 
+    public static final String CONTENT_DISPOSITION = "Content-Disposition";
+
     /**
      * Maximum cache time.
      */
@@ -280,17 +285,14 @@ public class DownloadStream implements Serializable {
                     }
                 }
 
-                // suggest local filename from DownloadStream if
-                // Content-Disposition
-                // not explicitly set
-                String contentDispositionValue = getParameter("Content-Disposition");
-                if (contentDispositionValue == null) {
-                    contentDispositionValue = "filename=\"" + getFileName()
-                            + "\"";
-                    response.setHeader("Content-Disposition",
-                            contentDispositionValue);
+                // Content-Disposition: attachment generally forces download
+                String contentDisposition = getParameter(CONTENT_DISPOSITION);
+                if (contentDisposition == null) {
+                    contentDisposition = getContentDispositionFilename(getFileName());
                 }
 
+                response.setHeader(CONTENT_DISPOSITION, contentDisposition);
+
                 int bufferSize = getBufferSize();
                 if (bufferSize <= 0 || bufferSize > Constants.MAX_BUFFER_SIZE) {
                     bufferSize = Constants.DEFAULT_BUFFER_SIZE;
@@ -318,6 +320,30 @@ public class DownloadStream implements Serializable {
     }
 
     /**
+     * Returns the filename formatted for inclusion in a Content-Disposition
+     * header. Includes both a plain version of the name and a UTF-8 version
+     * 
+     * @since
+     * @param filename
+     *            The filename to include
+     * @return A value for inclusion in a Content-Disposition header
+     */
+    public static String getContentDispositionFilename(String filename) {
+        try {
+            String encodedFilename = URLEncoder.encode(filename, "UTF-8");
+            return String.format("filename=\"%s\"; filename*=utf-8''%s",
+                    encodedFilename, encodedFilename);
+        } catch (UnsupportedEncodingException e) {
+            return null;
+        }
+
+    }
+
+    public static Logger getLogger() {
+        return Logger.getLogger(DownloadStream.class.getName());
+    }
+
+    /**
      * Helper method that tries to close an output stream and ignores any
      * exceptions.
      * 
diff --git a/server/src/com/vaadin/server/FileDownloader.java b/server/src/com/vaadin/server/FileDownloader.java
index 42c2f76e1a..b0c3bb1120 100644
--- a/server/src/com/vaadin/server/FileDownloader.java
+++ b/server/src/com/vaadin/server/FileDownloader.java
@@ -141,12 +141,17 @@ public class FileDownloader extends AbstractExtension {
             }
             stream = ((ConnectorResource) resource).getStream();
 
-            if (stream.getParameter("Content-Disposition") == null) {
-                // Content-Disposition: attachment generally forces download
-                stream.setParameter("Content-Disposition",
-                        "attachment; filename=\"" + stream.getFileName() + "\"");
+            String contentDisposition = stream
+                    .getParameter(DownloadStream.CONTENT_DISPOSITION);
+            if (contentDisposition == null) {
+                contentDisposition = "attachment; "
+                        + DownloadStream.getContentDispositionFilename(stream
+                                .getFileName());
             }
 
+            stream.setParameter(DownloadStream.CONTENT_DISPOSITION,
+                    contentDisposition);
+
             // Content-Type to block eager browser plug-ins from hijacking
             // the file
             if (isOverrideContentType()) {
@@ -158,4 +163,5 @@ public class FileDownloader extends AbstractExtension {
         stream.writeResponse(request, response);
         return true;
     }
+
 }
diff --git a/server/tests/src/com/vaadin/server/DownloadStreamTest.java b/server/tests/src/com/vaadin/server/DownloadStreamTest.java
new file mode 100644
index 0000000000..180b2e348b
--- /dev/null
+++ b/server/tests/src/com/vaadin/server/DownloadStreamTest.java
@@ -0,0 +1,39 @@
+package com.vaadin.server;
+
+import static org.mockito.Matchers.contains;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URLEncoder;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class DownloadStreamTest {
+    private String filename = "日本語.png";
+    private DownloadStream stream;
+
+    @Before
+    public void setup() {
+        stream = new DownloadStream(mock(InputStream.class), "", filename);
+    }
+
+    @Test
+    public void contentDispositionFilenameIsUtf8Encoded() throws IOException {
+        VaadinResponse response = mock(VaadinResponse.class);
+
+        stream.writeResponse(mock(VaadinRequest.class), response);
+
+        String encodedFileName = URLEncoder.encode(filename, "utf-8");
+        verify(response).setHeader(eq(DownloadStream.CONTENT_DISPOSITION),
+                contains(String.format("filename=\"%s\";", encodedFileName)));
+        verify(response)
+                .setHeader(
+                        eq(DownloadStream.CONTENT_DISPOSITION),
+                        contains(String.format("filename*=utf-8''%s",
+                                encodedFileName)));
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/FileDownloaderTest.java b/uitest/src/com/vaadin/tests/components/FileDownloaderTest.java
index 6e0616b115..9eb6806d13 100644
--- a/uitest/src/com/vaadin/tests/components/FileDownloaderTest.java
+++ b/uitest/src/com/vaadin/tests/components/FileDownloaderTest.java
@@ -37,7 +37,6 @@ import com.vaadin.server.StreamResource;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.server.VaadinResponse;
 import com.vaadin.tests.components.embedded.EmbeddedPdf;
-import com.vaadin.tests.util.Log;
 import com.vaadin.ui.AbstractComponent;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
@@ -49,27 +48,18 @@ import com.vaadin.ui.Label;
 import com.vaadin.ui.Layout;
 import com.vaadin.ui.NativeButton;
 
-public class FileDownloaderTest extends AbstractTestUI {
+public class FileDownloaderTest extends AbstractTestUIWithLog {
 
     private AbstractComponent firstDownloadComponent;
-    private Log log = new Log(5);
 
     @Override
     protected void setup(VaadinRequest request) {
-        addComponent(log);
-
         List<Class<? extends Component>> components = new ArrayList<Class<? extends Component>>();
         components.add(Button.class);
         components.add(NativeButton.class);
         components.add(CssLayout.class);
         components.add(Label.class);
 
-        // Resource resource = new ExternalResource(
-        // "https://vaadin.com/download/prerelease/7.0/7.0.0/7.0.0.beta1/vaadin-all-7.0.0.beta1.zip");
-        // addComponents(resource, components);
-        // resource = new ExternalResource(
-        // "https://vaadin.com/download/book-of-vaadin/current/pdf/book-of-vaadin.pdf");
-        // addComponents(resource, components);
         ConnectorResource resource;
         resource = new StreamResource(new StreamResource.StreamSource() {
 
@@ -105,13 +95,16 @@ public class FileDownloaderTest extends AbstractTestUI {
         } catch (IOException e) {
             e.printStackTrace();
         }
-        // resource = new DynamicConnectorResource(this, "requestImage.png");
-        // addComponents(resource, components);
-        // resource = new ThemeResource("favicon.ico");
-        // addComponents(resource, components);
         resource = new ClassResource(new EmbeddedPdf().getClass(), "test.pdf");
         addComponents("Class resource pdf", resource, components);
 
+        Button downloadUtf8File = new Button("Download UTF-8 named file");
+        FileDownloader fd = new FileDownloader(new ClassResource(
+                new EmbeddedPdf().getClass(), "åäö-日本語.pdf"));
+        fd.setOverrideContentType(false);
+        fd.extend(downloadUtf8File);
+        addComponent(downloadUtf8File);
+
         addComponent(new Button("Remove first download button",
                 new ClickListener() {
 
@@ -131,7 +124,7 @@ public class FileDownloaderTest extends AbstractTestUI {
                         FileDownloader e = (FileDownloader) firstDownloadComponent
                                 .getExtensions().iterator().next();
                         e.remove();
-                        log.log("FileDownload detached");
+                        log("FileDownload detached");
                     }
                 }));
     }
@@ -213,16 +206,4 @@ public class FileDownloaderTest extends AbstractTestUI {
         return bi;
     }
 
-    @Override
-    protected String getTestDescription() {
-        // TODO Auto-generated method stub
-        return null;
-    }
-
-    @Override
-    protected Integer getTicketNumber() {
-        // TODO Auto-generated method stub
-        return null;
-    }
-
 }
diff --git a/uitest/src/com/vaadin/tests/components/embedded/åäö-日本語.pdf b/uitest/src/com/vaadin/tests/components/embedded/åäö-日本語.pdf
new file mode 100644
index 0000000000..e44a87e9ad
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/embedded/åäö-日本語.pdf