diff --git a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java index 93e3bd4cb816..1a6fab102b38 100644 --- a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java +++ b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,6 +42,10 @@ */ public final class ContentDisposition { + private static final String INVALID_HEADER_FIELD_PARAMETER_FORMAT = + "Invalid header field parameter format (as defined in RFC 5987)"; + + @Nullable private final String type; @@ -201,11 +205,11 @@ public String toString() { if (this.filename != null) { if (this.charset == null || StandardCharsets.US_ASCII.equals(this.charset)) { sb.append("; filename=\""); - sb.append(this.filename).append('\"'); + sb.append(escapeQuotationsInFilename(this.filename)).append('\"'); } else { sb.append("; filename*="); - sb.append(encodeHeaderFieldParam(this.filename, this.charset)); + sb.append(encodeFilename(this.filename, this.charset)); } } if (this.size != null) { @@ -271,15 +275,23 @@ public static ContentDisposition parse(String contentDisposition) { String attribute = part.substring(0, eqIndex); String value = (part.startsWith("\"", eqIndex + 1) && part.endsWith("\"") ? part.substring(eqIndex + 2, part.length() - 1) : - part.substring(eqIndex + 1, part.length())); + part.substring(eqIndex + 1)); if (attribute.equals("name") ) { name = value; } else if (attribute.equals("filename*") ) { - filename = decodeHeaderFieldParam(value); - charset = Charset.forName(value.substring(0, value.indexOf('\'')).trim()); - Assert.isTrue(UTF_8.equals(charset) || ISO_8859_1.equals(charset), - "Charset should be UTF-8 or ISO-8859-1"); + int idx1 = value.indexOf('\''); + int idx2 = value.indexOf('\'', idx1 + 1); + if (idx1 != -1 && idx2 != -1) { + charset = Charset.forName(value.substring(0, idx1).trim()); + Assert.isTrue(UTF_8.equals(charset) || ISO_8859_1.equals(charset), + "Charset should be UTF-8 or ISO-8859-1"); + filename = decodeFilename(value.substring(idx2 + 1), charset); + } + else { + // US ASCII + filename = decodeFilename(value, StandardCharsets.US_ASCII); + } } else if (attribute.equals("filename") && (filename == null)) { filename = value; @@ -359,22 +371,15 @@ else if (!escaped && ch == '"') { /** * Decode the given header field param as describe in RFC 5987. *

Only the US-ASCII, UTF-8 and ISO-8859-1 charsets are supported. - * @param input the header field param + * @param filename the header field param + * @param charset the charset to use * @return the encoded header field param * @see RFC 5987 */ - private static String decodeHeaderFieldParam(String input) { - Assert.notNull(input, "Input String should not be null"); - int firstQuoteIndex = input.indexOf('\''); - int secondQuoteIndex = input.indexOf('\'', firstQuoteIndex + 1); - // US_ASCII - if (firstQuoteIndex == -1 || secondQuoteIndex == -1) { - return input; - } - Charset charset = Charset.forName(input.substring(0, firstQuoteIndex).trim()); - Assert.isTrue(UTF_8.equals(charset) || ISO_8859_1.equals(charset), - "Charset should be UTF-8 or ISO-8859-1"); - byte[] value = input.substring(secondQuoteIndex + 1, input.length()).getBytes(charset); + private static String decodeFilename(String filename, Charset charset) { + Assert.notNull(filename, "'input' String` should not be null"); + Assert.notNull(charset, "'charset' should not be null"); + byte[] value = filename.getBytes(charset); ByteArrayOutputStream bos = new ByteArrayOutputStream(); int index = 0; while (index < value.length) { @@ -383,13 +388,18 @@ private static String decodeHeaderFieldParam(String input) { bos.write((char) b); index++; } - else if (b == '%') { - char[] array = { (char)value[index + 1], (char)value[index + 2]}; - bos.write(Integer.parseInt(String.valueOf(array), 16)); + else if (b == '%' && index < value.length - 2) { + char[] array = new char[]{(char) value[index + 1], (char) value[index + 2]}; + try { + bos.write(Integer.parseInt(String.valueOf(array), 16)); + } + catch (NumberFormatException ex) { + throw new IllegalArgumentException(INVALID_HEADER_FIELD_PARAMETER_FORMAT, ex); + } index+=3; } else { - throw new IllegalArgumentException("Invalid header field parameter format (as defined in RFC 5987)"); + throw new IllegalArgumentException(INVALID_HEADER_FIELD_PARAMETER_FORMAT); } } return new String(bos.toByteArray(), charset); @@ -401,6 +411,23 @@ private static boolean isRFC5987AttrChar(byte c) { c == '.' || c == '^' || c == '_' || c == '`' || c == '|' || c == '~'; } + private static String escapeQuotationsInFilename(String filename) { + if (filename.indexOf('"') == -1 && filename.indexOf('\\') == -1) { + return filename; + } + boolean escaped = false; + StringBuilder sb = new StringBuilder(); + for (char c : filename.toCharArray()) { + sb.append((c == '"' && !escaped) ? "\\\"" : c); + escaped = (!escaped && c == '\\'); + } + // Remove backslash at the end.. + if (escaped) { + sb.deleteCharAt(sb.length() - 1); + } + return sb.toString(); + } + /** * Encode the given header field param as describe in RFC 5987. * @param input the header field param @@ -409,14 +436,11 @@ private static boolean isRFC5987AttrChar(byte c) { * @return the encoded header field param * @see RFC 5987 */ - private static String encodeHeaderFieldParam(String input, Charset charset) { - Assert.notNull(input, "Input String should not be null"); - Assert.notNull(charset, "Charset should not be null"); - if (StandardCharsets.US_ASCII.equals(charset)) { - return input; - } - Assert.isTrue(UTF_8.equals(charset) || ISO_8859_1.equals(charset), - "Charset should be UTF-8 or ISO-8859-1"); + private static String encodeFilename(String input, Charset charset) { + Assert.notNull(input, "`input` is required"); + Assert.notNull(charset, "`charset` is required"); + Assert.isTrue(!StandardCharsets.US_ASCII.equals(charset), "ASCII does not require encoding"); + Assert.isTrue(UTF_8.equals(charset) || ISO_8859_1.equals(charset), "Only UTF-8 and ISO-8859-1 supported."); byte[] source = input.getBytes(charset); int len = source.length; StringBuilder sb = new StringBuilder(len << 1); @@ -449,7 +473,11 @@ public interface Builder { Builder name(String name); /** - * Set the value of the {@literal filename} parameter. + * Set the value of the {@literal filename} parameter. The given + * filename will be formatted as quoted-string, as defined in RFC 2616, + * section 2.2, and any quote characters within the filename value will + * be escaped with a backslash, e.g. {@code "foo\"bar.txt"} becomes + * {@code "foo\\\"bar.txt"}. */ Builder filename(String filename); @@ -530,12 +558,14 @@ public Builder name(String name) { @Override public Builder filename(String filename) { + Assert.hasText(filename, "No filename"); this.filename = filename; return this; } @Override public Builder filename(String filename, Charset charset) { + Assert.hasText(filename, "No filename"); this.filename = filename; this.charset = charset; return this; diff --git a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java index f195358f63cb..e0072e09e464 100644 --- a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java +++ b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,178 +17,218 @@ package org.springframework.http; import java.lang.reflect.Method; -import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; +import java.util.function.BiConsumer; +import java.util.function.Consumer; -import static org.junit.Assert.assertEquals; import org.junit.Test; import org.springframework.util.ReflectionUtils; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.springframework.http.ContentDisposition.builder; + /** * Unit tests for {@link ContentDisposition} - * * @author Sebastien Deleuze + * @author Rossen Stoyanchev */ public class ContentDispositionTests { - @Test - public void parse() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123"); - assertEquals(ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build(), disposition); - } + private static DateTimeFormatter formatter = DateTimeFormatter.RFC_1123_DATE_TIME; + @Test - public void parseType() { - ContentDisposition disposition = ContentDisposition.parse("form-data"); - assertEquals(ContentDisposition.builder("form-data").build(), disposition); + public void parse() { + assertEquals(builder("form-data").name("foo").filename("foo.txt").size(123L).build(), + parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123")); } @Test - public void parseUnquotedFilename() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; filename=unquoted"); - assertEquals(ContentDisposition.builder("form-data").filename("unquoted").build(), disposition); + public void parseFilenameUnquoted() { + assertEquals(builder("form-data").filename("unquoted").build(), + parse("form-data; filename=unquoted")); } @Test // SPR-16091 public void parseFilenameWithSemicolon() { - ContentDisposition disposition = ContentDisposition - .parse("attachment; filename=\"filename with ; semicolon.txt\""); - assertEquals(ContentDisposition.builder("attachment") - .filename("filename with ; semicolon.txt").build(), disposition); + assertEquals(builder("attachment").filename("filename with ; semicolon.txt").build(), + parse("attachment; filename=\"filename with ; semicolon.txt\"")); } @Test - public void parseAndIgnoreEmptyParts() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"foo\";; ; filename=\"foo.txt\"; size=123"); - assertEquals(ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build(), disposition); + public void parseEncodedFilename() { + assertEquals(builder("form-data").name("name").filename("中文.txt", StandardCharsets.UTF_8).build(), + parse("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt")); } @Test // gh-24112 public void parseEncodedFilenameWithPaddedCharset() { - ContentDisposition disposition = ContentDisposition - .parse("attachment; filename*= UTF-8''some-file.zip"); - assertEquals(ContentDisposition.builder("attachment") - .filename("some-file.zip", StandardCharsets.UTF_8).build(), disposition); + assertEquals(builder("attachment").filename("some-file.zip", StandardCharsets.UTF_8).build(), + parse("attachment; filename*= UTF-8''some-file.zip")); } @Test - public void parseEncodedFilename() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt"); - assertEquals(ContentDisposition.builder("form-data").name("name") - .filename("中文.txt", StandardCharsets.UTF_8).build(), disposition); + public void parseEncodedFilenameWithoutCharset() { + assertEquals(builder("form-data").name("name").filename("test.txt").build(), + parse("form-data; name=\"name\"; filename*=test.txt")); + } + + @Test(expected = IllegalArgumentException.class) + public void parseEncodedFilenameWithInvalidCharset() { + parse("form-data; name=\"name\"; filename*=UTF-16''test.txt"); + } + + @Test + public void parseEncodedFilenameWithInvalidName() { + + Consumer tester = input -> { + try { + parse(input); + fail(); + } + catch (IllegalArgumentException ex) { + // expected + } + }; + + tester.accept("form-data; name=\"name\"; filename*=UTF-8''%A"); + tester.accept("form-data; name=\"name\"; filename*=UTF-8''%A.txt"); } @Test // gh-23077 public void parseWithEscapedQuote() { - ContentDisposition disposition = ContentDisposition.parse( - "form-data; name=\"file\"; filename=\"\\\"The Twilight Zone\\\".txt\"; size=123"); - assertEquals(ContentDisposition.builder("form-data").name("file") - .filename("\\\"The Twilight Zone\\\".txt").size(123L).build(), disposition); + + BiConsumer tester = (description, filename) -> + assertEquals(description, + builder("form-data").name("file").filename(filename).size(123L).build(), + parse("form-data; name=\"file\"; filename=\"" + filename + "\"; size=123")); + + tester.accept("Escaped quotes should be ignored", + "\\\"The Twilight Zone\\\".txt"); + + tester.accept("Escaped quotes preceded by escaped backslashes should be ignored", + "\\\\\\\"The Twilight Zone\\\\\\\".txt"); + + tester.accept("Escaped backslashes should not suppress quote", + "The Twilight Zone \\\\"); + + tester.accept("Escaped backslashes should not suppress quote", + "The Twilight Zone \\\\\\\\"); + } + + @Test + public void parseWithExtraSemicolons() { + assertEquals(builder("form-data").name("foo").filename("foo.txt").size(123L).build(), + parse("form-data; name=\"foo\";; ; filename=\"foo.txt\"; size=123")); + } + + @Test + public void parseDates() { + assertEquals( + builder("attachment") + .creationDate(ZonedDateTime.parse("Mon, 12 Feb 2007 10:15:30 -0500", formatter)) + .modificationDate(ZonedDateTime.parse("Tue, 13 Feb 2007 10:15:30 -0500", formatter)) + .readDate(ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter)).build(), + parse("attachment; creation-date=\"Mon, 12 Feb 2007 10:15:30 -0500\"; " + + "modification-date=\"Tue, 13 Feb 2007 10:15:30 -0500\"; " + + "read-date=\"Wed, 14 Feb 2007 10:15:30 -0500\"")); + } + + @Test + public void parseIgnoresInvalidDates() { + assertEquals( + builder("attachment") + .readDate(ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter)) + .build(), + parse("attachment; creation-date=\"-1\"; " + + "modification-date=\"-1\"; " + + "read-date=\"Wed, 14 Feb 2007 10:15:30 -0500\"")); } @Test(expected = IllegalArgumentException.class) public void parseEmpty() { - ContentDisposition.parse(""); + parse(""); } @Test(expected = IllegalArgumentException.class) public void parseNoType() { - ContentDisposition.parse(";"); + parse(";"); } @Test(expected = IllegalArgumentException.class) public void parseInvalidParameter() { - ContentDisposition.parse("foo;bar"); + parse("foo;bar"); } - @Test - public void parseDates() { - ContentDisposition disposition = ContentDisposition - .parse("attachment; creation-date=\"Mon, 12 Feb 2007 10:15:30 -0500\"; " + - "modification-date=\"Tue, 13 Feb 2007 10:15:30 -0500\"; " + - "read-date=\"Wed, 14 Feb 2007 10:15:30 -0500\""); - DateTimeFormatter formatter = DateTimeFormatter.RFC_1123_DATE_TIME; - assertEquals(ContentDisposition.builder("attachment") - .creationDate(ZonedDateTime.parse("Mon, 12 Feb 2007 10:15:30 -0500", formatter)) - .modificationDate(ZonedDateTime.parse("Tue, 13 Feb 2007 10:15:30 -0500", formatter)) - .readDate(ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter)).build(), disposition); + private static ContentDisposition parse(String input) { + return ContentDisposition.parse(input); } + @Test - public void parseInvalidDates() { - ContentDisposition disposition = ContentDisposition - .parse("attachment; creation-date=\"-1\"; modification-date=\"-1\"; " + - "read-date=\"Wed, 14 Feb 2007 10:15:30 -0500\""); - DateTimeFormatter formatter = DateTimeFormatter.RFC_1123_DATE_TIME; - assertEquals(ContentDisposition.builder("attachment") - .readDate(ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter)).build(), disposition); + public void format() { + assertEquals("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123", + builder("form-data").name("foo").filename("foo.txt").size(123L).build().toString()); } @Test - public void headerValue() { - ContentDisposition disposition = ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build(); - assertEquals("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123", disposition.toString()); + public void formatWithEncodedFilename() { + assertEquals("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt", + builder("form-data").name("name").filename("中文.txt", StandardCharsets.UTF_8).build().toString()); } @Test - public void headerValueWithEncodedFilename() { - ContentDisposition disposition = ContentDisposition.builder("form-data") - .name("name").filename("中文.txt", StandardCharsets.UTF_8).build(); - assertEquals("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt", - disposition.toString()); + public void formatWithEncodedFilenameUsingUsAscii() { + assertEquals("form-data; name=\"name\"; filename=\"test.txt\"", + builder("form-data") + .name("name") + .filename("test.txt", StandardCharsets.US_ASCII) + .build() + .toString()); } - @Test // SPR-14547 - public void encodeHeaderFieldParam() { - Method encode = ReflectionUtils.findMethod(ContentDisposition.class, - "encodeHeaderFieldParam", String.class, Charset.class); - ReflectionUtils.makeAccessible(encode); + @Test // gh-24220 + public void formatWithFilenameWithQuotes() { - String result = (String)ReflectionUtils.invokeMethod(encode, null, "test.txt", - StandardCharsets.US_ASCII); - assertEquals("test.txt", result); + BiConsumer tester = (input, output) -> { - result = (String)ReflectionUtils.invokeMethod(encode, null, "中文.txt", StandardCharsets.UTF_8); - assertEquals("UTF-8''%E4%B8%AD%E6%96%87.txt", result); - } + assertEquals("form-data; filename=\"" + output + "\"", + builder("form-data").filename(input).build().toString()); - @Test(expected = IllegalArgumentException.class) - public void encodeHeaderFieldParamInvalidCharset() { - Method encode = ReflectionUtils.findMethod(ContentDisposition.class, - "encodeHeaderFieldParam", String.class, Charset.class); - ReflectionUtils.makeAccessible(encode); - ReflectionUtils.invokeMethod(encode, null, "test", StandardCharsets.UTF_16); - } + assertEquals("form-data; filename=\"" + output + "\"", + builder("form-data").filename(input, StandardCharsets.US_ASCII).build().toString()); + }; + + String filename = "\"foo.txt"; + tester.accept(filename, "\\" + filename); + + filename = "\\\"foo.txt"; + tester.accept(filename, filename); + + filename = "\\\\\"foo.txt"; + tester.accept(filename, "\\" + filename); + + filename = "\\\\\\\"foo.txt"; + tester.accept(filename, filename); - @Test // SPR-14408 - public void decodeHeaderFieldParam() { - Method decode = ReflectionUtils.findMethod(ContentDisposition.class, - "decodeHeaderFieldParam", String.class); - ReflectionUtils.makeAccessible(decode); + filename = "\\\\\\\\\"foo.txt"; + tester.accept(filename, "\\" + filename); - String result = (String)ReflectionUtils.invokeMethod(decode, null, "test.txt"); - assertEquals("test.txt", result); + tester.accept("\"\"foo.txt", "\\\"\\\"foo.txt"); + tester.accept("\"\"\"foo.txt", "\\\"\\\"\\\"foo.txt"); - result = (String)ReflectionUtils.invokeMethod(decode, null, "UTF-8''%E4%B8%AD%E6%96%87.txt"); - assertEquals("中文.txt", result); + tester.accept("foo.txt\\", "foo.txt"); + tester.accept("foo.txt\\\\", "foo.txt\\\\"); + tester.accept("foo.txt\\\\\\", "foo.txt\\\\"); } @Test(expected = IllegalArgumentException.class) - public void decodeHeaderFieldParamInvalidCharset() { - Method decode = ReflectionUtils.findMethod(ContentDisposition.class, - "decodeHeaderFieldParam", String.class); - ReflectionUtils.makeAccessible(decode); - ReflectionUtils.invokeMethod(decode, null, "UTF-16''test"); + public void formatWithEncodedFilenameUsingInvalidCharset() { + builder("form-data").name("name").filename("test.txt", StandardCharsets.UTF_16).build().toString(); } }