diff --git a/src/main/java/org/htmlunit/attachment/Attachment.java b/src/main/java/org/htmlunit/attachment/Attachment.java index b3054e1e4f..7dbad038e6 100644 --- a/src/main/java/org/htmlunit/attachment/Attachment.java +++ b/src/main/java/org/htmlunit/attachment/Attachment.java @@ -29,6 +29,7 @@ * @author Daniel Gredler * @author Ronald Brill * @author Lai Quang Duong + * @author Vidya m */ public class Attachment { @@ -145,7 +146,7 @@ public static String getSuggestedFilename(final String contentDispositionHeader) * @param name the name to sanitize * @return the sanitized name or {@code null} if nothing is left */ - private static String sanitizeFilename(final String name) { + static String sanitizeFilename(final String name) { if (name == null) { return null; } diff --git a/src/main/java/org/htmlunit/attachment/DownloadingAttachmentHandler.java b/src/main/java/org/htmlunit/attachment/DownloadingAttachmentHandler.java index 3d68509ec2..c10dfd8f4e 100644 --- a/src/main/java/org/htmlunit/attachment/DownloadingAttachmentHandler.java +++ b/src/main/java/org/htmlunit/attachment/DownloadingAttachmentHandler.java @@ -39,6 +39,7 @@ * * @author Marek Andreansky * @author Ronald Brill + * @author Vidya m */ public class DownloadingAttachmentHandler implements AttachmentHandler { @@ -96,7 +97,9 @@ public void handleAttachment(final Page page, final String attachmentFilename) { } private Path determineDestionationFile(final Page page, final String attachmentFilename) { - String fileName = attachmentFilename; + // the name can come from the page (download attribute), sanitize it like + // a name from the Content-Disposition header to stay inside the download folder + String fileName = Attachment.sanitizeFilename(attachmentFilename); if (StringUtils.isBlank(fileName)) { final String file = page.getWebResponse().getWebRequest().getUrl().getFile(); diff --git a/src/test/java/org/htmlunit/attachment/DownloadingAttachmentHandlerTest.java b/src/test/java/org/htmlunit/attachment/DownloadingAttachmentHandlerTest.java index dcfeb0c6d1..049afcf517 100644 --- a/src/test/java/org/htmlunit/attachment/DownloadingAttachmentHandlerTest.java +++ b/src/test/java/org/htmlunit/attachment/DownloadingAttachmentHandlerTest.java @@ -36,6 +36,7 @@ * * @author Marek Andreansky * @author Ronald Brill + * @author Vidya m */ public class DownloadingAttachmentHandlerTest extends SimpleWebTestCase { @@ -207,4 +208,60 @@ public void basicFileNameFromHeader() throws Exception { FileUtils.deleteDirectory(downloadFolder); } } + + /** + * The file name from the download attribute is sanitized like the one from the + * Content-Disposition header; a path can't be used to leave the download folder. + * + * @throws Exception if an error occurs + */ + @Test + public void fileNameFromDownloadAttributeIsSanitized() throws Exception { + final String content1 = DOCTYPE_HTML + + "\n" + + "relative\n" + + "absolute\n" + + ""; + final String content2 = "download file contents"; + + final File tmpFolder = new File(System.getProperty("java.io.tmpdir")); + final File testFolder = new File(tmpFolder, "downloading-attachment-test"); + final File downloadFolder = new File(testFolder, "downloads"); + final File absoluteFile = new File(testFolder, "absolute.txt"); + FileUtils.deleteDirectory(testFolder); + + try { + FileUtils.forceMkdir(downloadFolder); + + final WebClient client = getWebClient(); + client.setAttachmentHandler(new DownloadingAttachmentHandler(downloadFolder.toPath())); + + final MockWebConnection conn = new MockWebConnection(); + conn.setResponse(URL_FIRST, content1); + conn.setResponse(URL_SECOND, content2, MimeType.TEXT_PLAIN); + client.setWebConnection(conn); + + final HtmlPage page = client.getPage(URL_FIRST); + + final HtmlAnchor relative = page.getHtmlElementById("relative"); + relative.setAttribute("download", "../evil.txt"); + relative.click(); + + final HtmlAnchor absolute = page.getHtmlElementById("absolute"); + absolute.setAttribute("download", absoluteFile.getAbsolutePath()); + absolute.click(); + + assertFalse(absoluteFile.exists()); + + final Collection files = FileUtils.listFiles(testFolder, null, true); + assertEquals(2, files.size()); + for (final File file : files) { + assertEquals(downloadFolder, file.getParentFile()); + } + assertTrue(files.removeIf(f -> f.getName().equals("_evil.txt"))); + } + finally { + FileUtils.deleteDirectory(testFolder); + } + } }