From: Anna Miroshnik Date: Thu, 12 Mar 2015 15:56:10 +0000 (+0300) Subject: Format UTF-8 filenames correctly for download (#16556) X-Git-Tag: 7.5.0.rc1~32 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d7284ccfe40f7028e880328566bb825ea31ad619;p=vaadin-framework.git 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 --- 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; @@ -317,6 +319,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> components = new ArrayList>(); 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/\303\245\303\244\303\266-\346\227\245\346\234\254\350\252\236.pdf" "b/uitest/src/com/vaadin/tests/components/embedded/\303\245\303\244\303\266-\346\227\245\346\234\254\350\252\236.pdf" new file mode 100644 index 0000000000..e44a87e9ad Binary files /dev/null and "b/uitest/src/com/vaadin/tests/components/embedded/\303\245\303\244\303\266-\346\227\245\346\234\254\350\252\236.pdf" differ