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 ac77d45c7e65..a9e574148c54 100644 --- a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java +++ b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java @@ -458,7 +458,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); @@ -539,10 +543,24 @@ public Builder name(String name) { @Override public Builder filename(String filename) { - this.filename = filename; + Assert.hasText(filename, "No filename"); + this.filename = escapeQuotationMarks(filename); return this; } + private static String escapeQuotationMarks(String filename) { + if (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 == '\\'); + } + return sb.toString(); + } + @Override public Builder filename(String filename, Charset charset) { this.filename = filename; 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 26b5ae57eab8..caf6e398e77a 100644 --- a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java +++ b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java @@ -19,11 +19,13 @@ import java.nio.charset.StandardCharsets; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; +import java.util.function.BiConsumer; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.springframework.http.ContentDisposition.builder; /** * Unit tests for {@link ContentDisposition} @@ -38,7 +40,7 @@ public class ContentDispositionTests { @Test public void parse() { assertThat(parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123")) - .isEqualTo(ContentDisposition.builder("form-data") + .isEqualTo(builder("form-data") .name("foo") .filename("foo.txt") .size(123L) @@ -48,7 +50,7 @@ public void parse() { @Test public void parseFilenameUnquoted() { assertThat(parse("form-data; filename=unquoted")) - .isEqualTo(ContentDisposition.builder("form-data") + .isEqualTo(builder("form-data") .filename("unquoted") .build()); } @@ -56,7 +58,7 @@ public void parseFilenameUnquoted() { @Test // SPR-16091 public void parseFilenameWithSemicolon() { assertThat(parse("attachment; filename=\"filename with ; semicolon.txt\"")) - .isEqualTo(ContentDisposition.builder("attachment") + .isEqualTo(builder("attachment") .filename("filename with ; semicolon.txt") .build()); } @@ -64,7 +66,7 @@ public void parseFilenameWithSemicolon() { @Test public void parseEncodedFilename() { assertThat(parse("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt")) - .isEqualTo(ContentDisposition.builder("form-data") + .isEqualTo(builder("form-data") .name("name") .filename("中文.txt", StandardCharsets.UTF_8) .build()); @@ -73,7 +75,7 @@ public void parseEncodedFilename() { @Test // gh-24112 public void parseEncodedFilenameWithPaddedCharset() { assertThat(parse("attachment; filename*= UTF-8''some-file.zip")) - .isEqualTo(ContentDisposition.builder("attachment") + .isEqualTo(builder("attachment") .filename("some-file.zip", StandardCharsets.UTF_8) .build()); } @@ -81,7 +83,7 @@ public void parseEncodedFilenameWithPaddedCharset() { @Test public void parseEncodedFilenameWithoutCharset() { assertThat(parse("form-data; name=\"name\"; filename*=test.txt")) - .isEqualTo(ContentDisposition.builder("form-data") + .isEqualTo(builder("form-data") .name("name") .filename("test.txt") .build()); @@ -104,18 +106,30 @@ public void parseEncodedFilenameWithInvalidName() { @Test // gh-23077 public void parseWithEscapedQuote() { - 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()); + + BiConsumer tester = (description, filename) -> { + assertThat(parse("form-data; name=\"file\"; filename=\"" + filename + "\"; size=123")) + .as(description) + .isEqualTo(builder("form-data").name("file").filename(filename).size(123L).build()); + }; + + 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() { assertThat(parse("form-data; name=\"foo\";; ; filename=\"foo.txt\"; size=123")) - .isEqualTo(ContentDisposition.builder("form-data") + .isEqualTo(builder("form-data") .name("foo") .filename("foo.txt") .size(123L) @@ -133,7 +147,7 @@ public void parseDates() { "creation-date=\"" + creationTime.format(formatter) + "\"; " + "modification-date=\"" + modificationTime.format(formatter) + "\"; " + "read-date=\"" + readTime.format(formatter) + "\"")).isEqualTo( - ContentDisposition.builder("attachment") + builder("attachment") .creationDate(creationTime) .modificationDate(modificationTime) .readDate(readTime) @@ -149,7 +163,7 @@ public void parseIgnoresInvalidDates() { "creation-date=\"-1\"; " + "modification-date=\"-1\"; " + "read-date=\"" + readTime.format(formatter) + "\"")).isEqualTo( - ContentDisposition.builder("attachment") + builder("attachment") .readDate(readTime) .build()); } @@ -177,7 +191,7 @@ private static ContentDisposition parse(String input) { @Test public void format() { assertThat( - ContentDisposition.builder("form-data") + builder("form-data") .name("foo") .filename("foo.txt") .size(123L) @@ -188,7 +202,7 @@ public void format() { @Test public void formatWithEncodedFilename() { assertThat( - ContentDisposition.builder("form-data") + builder("form-data") .name("name") .filename("中文.txt", StandardCharsets.UTF_8) .build().toString()) @@ -198,7 +212,7 @@ public void formatWithEncodedFilename() { @Test public void formatWithEncodedFilenameUsingUsAscii() { assertThat( - ContentDisposition.builder("form-data") + builder("form-data") .name("name") .filename("test.txt", StandardCharsets.US_ASCII) .build() @@ -206,10 +220,37 @@ public void formatWithEncodedFilenameUsingUsAscii() { .isEqualTo("form-data; name=\"name\"; filename=\"test.txt\""); } + @Test // gh-24220 + public void formatWithFilenameWithQuotes() { + + BiConsumer tester = (input, output) -> { + assertThat(builder("form-data").filename(input).build().toString()) + .isEqualTo("form-data; filename=\"" + output + "\""); + }; + + 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); + + filename = "\\\\\\\\\"foo.txt"; + tester.accept(filename, "\\" + filename); + + tester.accept("\"\"foo.txt", "\\\"\\\"foo.txt"); + tester.accept("\"\"\"foo.txt", "\\\"\\\"\\\"foo.txt"); + } + @Test public void formatWithEncodedFilenameUsingInvalidCharset() { assertThatIllegalArgumentException().isThrownBy(() -> - ContentDisposition.builder("form-data") + builder("form-data") .name("name") .filename("test.txt", StandardCharsets.UTF_16) .build()