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 115aea08d5aaa65876639d7130b7463125c899b6..37d912005751698374f02e30d9dd5add270ba15b 100644 --- a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java +++ b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java @@ -42,6 +42,9 @@ import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME; */ 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; @@ -205,7 +208,7 @@ public final class ContentDisposition { } else { sb.append("; filename*="); - sb.append(encodeHeaderFieldParam(this.filename, this.charset)); + sb.append(encodeFilename(this.filename, this.charset)); } } if (this.size != null) { @@ -271,15 +274,23 @@ public final class 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('\''))); - 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)); + 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; @@ -357,24 +368,17 @@ public final class ContentDisposition { } /** - * Decode the given header field param as describe in RFC 5987. + * Decode the given header field param as described 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 filename + * @param charset the charset for the filename * @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)); - 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 +387,18 @@ public final class ContentDisposition { 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); @@ -409,7 +418,7 @@ public final class ContentDisposition { * @return the encoded header field param * @see RFC 5987 */ - private static String encodeHeaderFieldParam(String input, Charset charset) { + private static String encodeFilename(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)) { 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 13acce90a02913058855ff38809ea8c9b116111d..a647871b0e6b2d7e03af66e319fac9a823f8027e 100644 --- a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java +++ b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java @@ -16,186 +16,196 @@ 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 org.junit.jupiter.api.Test; -import org.springframework.util.ReflectionUtils; - import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Unit tests for {@link ContentDisposition} - * * @author Sebastien Deleuze + * @author Rossen Stoyanchev */ public class ContentDispositionTests { + private static DateTimeFormatter formatter = DateTimeFormatter.RFC_1123_DATE_TIME; - @Test - public void parseTest() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123"); - assertThat(disposition).isEqualTo(ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build()); - } @Test public void parse() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123"); - assertThat(disposition).isEqualTo(ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build()); + assertThat(parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123")) + .isEqualTo(ContentDisposition.builder("form-data") + .name("foo") + .filename("foo.txt") + .size(123L) + .build()); } @Test - public void parseType() { - ContentDisposition disposition = ContentDisposition.parse("form-data"); - assertThat(disposition).isEqualTo(ContentDisposition.builder("form-data").build()); + public void parseFilenameUnquoted() { + assertThat(parse("form-data; filename=unquoted")) + .isEqualTo(ContentDisposition.builder("form-data") + .filename("unquoted") + .build()); + } + + @Test // SPR-16091 + public void parseFilenameWithSemicolon() { + assertThat(parse("attachment; filename=\"filename with ; semicolon.txt\"")) + .isEqualTo(ContentDisposition.builder("attachment") + .filename("filename with ; semicolon.txt") + .build()); } @Test - public void parseUnquotedFilename() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; filename=unquoted"); - assertThat(disposition).isEqualTo(ContentDisposition.builder("form-data").filename("unquoted").build()); + public void parseEncodedFilename() { + assertThat(parse("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt")) + .isEqualTo(ContentDisposition.builder("form-data") + .name("name") + .filename("中文.txt", StandardCharsets.UTF_8) + .build()); } - @Test // SPR-16091 - public void parseFilenameWithSemicolon() { - ContentDisposition disposition = ContentDisposition - .parse("attachment; filename=\"filename with ; semicolon.txt\""); - assertThat(disposition).isEqualTo(ContentDisposition.builder("attachment") - .filename("filename with ; semicolon.txt").build()); + @Test + public void parseEncodedFilenameWithoutCharset() { + assertThat(parse("form-data; name=\"name\"; filename*=test.txt")) + .isEqualTo(ContentDisposition.builder("form-data") + .name("name") + .filename("test.txt") + .build()); } @Test - public void parseAndIgnoreEmptyParts() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"foo\";; ; filename=\"foo.txt\"; size=123"); - assertThat(disposition).isEqualTo(ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build()); + public void parseEncodedFilenameWithInvalidCharset() { + assertThatIllegalArgumentException() + .isThrownBy(() -> parse("form-data; name=\"name\"; filename*=UTF-16''test.txt")); } @Test - public void parseEncodedFilename() { - ContentDisposition disposition = ContentDisposition - .parse("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt"); - assertThat(disposition).isEqualTo(ContentDisposition.builder("form-data").name("name") - .filename("中文.txt", StandardCharsets.UTF_8).build()); + public void parseEncodedFilenameWithInvalidName() { + assertThatIllegalArgumentException() + .isThrownBy(() -> parse("form-data; name=\"name\"; filename*=UTF-8''%A")); + + assertThatIllegalArgumentException() + .isThrownBy(() -> parse("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"); - assertThat(ContentDisposition.builder("form-data").name("file") - .filename("\\\"The Twilight Zone\\\".txt").size(123L).build()).isEqualTo(disposition); + assertThat(parse("form-data; name=\"file\"; filename=\"\\\"The Twilight Zone\\\".txt\"; size=123")) + .isEqualTo(ContentDisposition.builder("form-data") + .name("file") + .filename("\\\"The Twilight Zone\\\".txt") + .size(123L) + .build()); } @Test - public void parseEmpty() { - assertThatIllegalArgumentException().isThrownBy(() -> - ContentDisposition.parse("")); + public void parseWithExtraSemicolons() { + assertThat(parse("form-data; name=\"foo\";; ; filename=\"foo.txt\"; size=123")) + .isEqualTo(ContentDisposition.builder("form-data") + .name("foo") + .filename("foo.txt") + .size(123L) + .build()); } @Test - public void parseNoType() { - assertThatIllegalArgumentException().isThrownBy(() -> - ContentDisposition.parse(";")); + public void parseDates() { + ZonedDateTime creationTime = ZonedDateTime.parse("Mon, 12 Feb 2007 10:15:30 -0500", formatter); + ZonedDateTime modificationTime = ZonedDateTime.parse("Tue, 13 Feb 2007 10:15:30 -0500", formatter); + ZonedDateTime readTime = ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter); + + assertThat( + parse("attachment; " + + "creation-date=\"" + creationTime.format(formatter) + "\"; " + + "modification-date=\"" + modificationTime.format(formatter) + "\"; " + + "read-date=\"" + readTime.format(formatter) + "\"")).isEqualTo( + ContentDisposition.builder("attachment") + .creationDate(creationTime) + .modificationDate(modificationTime) + .readDate(readTime) + .build()); } @Test - public void parseInvalidParameter() { - assertThatIllegalArgumentException().isThrownBy(() -> - ContentDisposition.parse("foo;bar")); - } + public void parseIgnoresInvalidDates() { + ZonedDateTime readTime = ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter); - @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; - assertThat(disposition).isEqualTo(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()); + assertThat( + parse("attachment; " + + "creation-date=\"-1\"; " + + "modification-date=\"-1\"; " + + "read-date=\"" + readTime.format(formatter) + "\"")).isEqualTo( + ContentDisposition.builder("attachment") + .readDate(readTime) + .build()); } @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; - assertThat(disposition).isEqualTo(ContentDisposition.builder("attachment") - .readDate(ZonedDateTime.parse("Wed, 14 Feb 2007 10:15:30 -0500", formatter)).build()); + public void parseEmpty() { + assertThatIllegalArgumentException().isThrownBy(() -> parse("")); } @Test - public void headerValue() { - ContentDisposition disposition = ContentDisposition.builder("form-data") - .name("foo").filename("foo.txt").size(123L).build(); - assertThat(disposition.toString()).isEqualTo("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123"); + public void parseNoType() { + assertThatIllegalArgumentException().isThrownBy(() -> parse(";")); } @Test - public void headerValueWithEncodedFilename() { - ContentDisposition disposition = ContentDisposition.builder("form-data") - .name("name").filename("中文.txt", StandardCharsets.UTF_8).build(); - assertThat(disposition.toString()).isEqualTo("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt"); + public void parseInvalidParameter() { + assertThatIllegalArgumentException().isThrownBy(() -> parse("foo;bar")); } - @Test // SPR-14547 - public void encodeHeaderFieldParam() { - Method encode = ReflectionUtils.findMethod(ContentDisposition.class, - "encodeHeaderFieldParam", String.class, Charset.class); - ReflectionUtils.makeAccessible(encode); + private static ContentDisposition parse(String input) { + return ContentDisposition.parse(input); + } - String result = (String)ReflectionUtils.invokeMethod(encode, null, "test.txt", - StandardCharsets.US_ASCII); - assertThat(result).isEqualTo("test.txt"); - result = (String)ReflectionUtils.invokeMethod(encode, null, "中文.txt", StandardCharsets.UTF_8); - assertThat(result).isEqualTo("UTF-8''%E4%B8%AD%E6%96%87.txt"); + @Test + public void format() { + assertThat( + ContentDisposition.builder("form-data") + .name("foo") + .filename("foo.txt") + .size(123L) + .build().toString()) + .isEqualTo("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123"); } @Test - public void encodeHeaderFieldParamInvalidCharset() { - Method encode = ReflectionUtils.findMethod(ContentDisposition.class, - "encodeHeaderFieldParam", String.class, Charset.class); - ReflectionUtils.makeAccessible(encode); - assertThatIllegalArgumentException().isThrownBy(() -> - ReflectionUtils.invokeMethod(encode, null, "test", StandardCharsets.UTF_16)); + public void formatWithEncodedFilename() { + assertThat( + ContentDisposition.builder("form-data") + .name("name") + .filename("中文.txt", StandardCharsets.UTF_8) + .build().toString()) + .isEqualTo("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt"); } - @Test // SPR-14408 - public void decodeHeaderFieldParam() { - Method decode = ReflectionUtils.findMethod(ContentDisposition.class, - "decodeHeaderFieldParam", String.class); - ReflectionUtils.makeAccessible(decode); - - String result = (String)ReflectionUtils.invokeMethod(decode, null, "test.txt"); - assertThat(result).isEqualTo("test.txt"); - - result = (String)ReflectionUtils.invokeMethod(decode, null, "UTF-8''%E4%B8%AD%E6%96%87.txt"); - assertThat(result).isEqualTo("中文.txt"); + @Test + public void formatWithEncodedFilenameUsingUsAscii() { + assertThat( + ContentDisposition.builder("form-data") + .name("name") + .filename("test.txt", StandardCharsets.US_ASCII) + .build() + .toString()) + .isEqualTo("form-data; name=\"name\"; filename=\"test.txt\""); } @Test - public void decodeHeaderFieldParamInvalidCharset() { - Method decode = ReflectionUtils.findMethod(ContentDisposition.class, - "decodeHeaderFieldParam", String.class); - ReflectionUtils.makeAccessible(decode); + public void formatWithEncodedFilenameUsingInvalidCharset() { assertThatIllegalArgumentException().isThrownBy(() -> - ReflectionUtils.invokeMethod(decode, null, "UTF-16''test")); + ContentDisposition.builder("form-data") + .name("name") + .filename("test.txt", StandardCharsets.UTF_16) + .build() + .toString()); } }