From 4cc91e46b210b4e4e7ed182f93994511391b54ed Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Tue, 6 Sep 2022 15:47:25 +0200 Subject: [PATCH] Decode quoted pairs in ContentDisposition This commit makes sure that quoted pairs, as used in Content-Disposition header file names (i.e. \" and \\), are properly decoded, whereas before they were stored as is. Closes gh-28837 --- .../http/ContentDisposition.java | 31 +++++++--- .../http/ContentDispositionTests.java | 62 +++++++++++-------- 2 files changed, 56 insertions(+), 37 deletions(-) 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 587ee40179..eb5cd93167 100644 --- a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java +++ b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java @@ -259,7 +259,7 @@ public final class ContentDisposition { if (this.filename != null) { if (this.charset == null || StandardCharsets.US_ASCII.equals(this.charset)) { sb.append("; filename=\""); - sb.append(escapeQuotationsInFilename(this.filename)).append('\"'); + sb.append(encodeQuotedPairs(this.filename)).append('\"'); } else { sb.append("; filename*="); @@ -404,6 +404,9 @@ public final class ContentDisposition { } } } + else if (value.indexOf('\\') != -1) { + filename = decodeQuotedPairs(value); + } else { filename = value; } @@ -560,25 +563,33 @@ public final class ContentDisposition { return StreamUtils.copyToString(baos, charset); } - private static String escapeQuotationsInFilename(String filename) { + private static String encodeQuotedPairs(String filename) { if (filename.indexOf('"') == -1 && filename.indexOf('\\') == -1) { return filename; } - boolean escaped = false; StringBuilder sb = new StringBuilder(); for (int i = 0; i < filename.length() ; i++) { char c = filename.charAt(i); - if (!escaped && c == '"') { - sb.append("\\\""); + if (c == '"' || c == '\\') { + sb.append('\\'); + } + sb.append(c); + } + return sb.toString(); + } + + private static String decodeQuotedPairs(String filename) { + StringBuilder sb = new StringBuilder(); + int length = filename.length(); + for (int i = 0; i < length; i++) { + char c = filename.charAt(i); + if (filename.charAt(i) == '\\' && i + 1 < length) { + i++; + sb.append(filename.charAt(i)); } else { sb.append(c); } - escaped = (!escaped && c == '\\'); - } - // Remove backslash at the end. - if (escaped) { - sb.deleteCharAt(sb.length() - 1); } return sb.toString(); } 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 9c6610550c..2164f8dbde 100644 --- a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java +++ b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java @@ -149,27 +149,25 @@ class ContentDispositionTests { .isThrownBy(() -> parse("form-data; name=\"name\"; filename*=UTF-8''%A.txt")); } - @Test // gh-23077 - @SuppressWarnings("deprecation") - void parseWithEscapedQuote() { - BiConsumer tester = (description, filename) -> - assertThat(parse("form-data; name=\"file\"; filename=\"" + filename + "\"; size=123")) - .as(description) - .isEqualTo(ContentDisposition.formData().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 + void parseBackslash() { + String s = "form-data; name=\"foo\"; filename=\"foo\\\\bar \\\"baz\\\" qux \\\\\\\" quux.txt\""; + ContentDisposition cd = ContentDisposition.parse( + s); + assertThat(cd.getName()).isEqualTo("foo"); + assertThat(cd.getFilename()).isEqualTo("foo\\bar \"baz\" qux \\\" quux.txt"); + assertThat(cd.toString()).isEqualTo(s); } + @Test + void parseBackslashInLastPosition() { + ContentDisposition cd = ContentDisposition.parse("form-data; name=\"foo\"; filename=\"bar\\\""); + assertThat(cd.getName()).isEqualTo("foo"); + assertThat(cd.getFilename()).isEqualTo("bar\\"); + assertThat(cd.toString()).isEqualTo("form-data; name=\"foo\"; filename=\"bar\\\\\""); + } + + @Test @SuppressWarnings("deprecation") void parseWithExtraSemicolons() { @@ -281,26 +279,26 @@ class ContentDispositionTests { }; String filename = "\"foo.txt"; - tester.accept(filename, "\\" + filename); + tester.accept(filename, "\\\"foo.txt"); filename = "\\\"foo.txt"; - tester.accept(filename, filename); + tester.accept(filename, "\\\\\\\"foo.txt"); filename = "\\\\\"foo.txt"; - tester.accept(filename, "\\" + filename); + tester.accept(filename, "\\\\\\\\\\\"foo.txt"); filename = "\\\\\\\"foo.txt"; - tester.accept(filename, filename); + tester.accept(filename, "\\\\\\\\\\\\\\\"foo.txt"); filename = "\\\\\\\\\"foo.txt"; - tester.accept(filename, "\\" + filename); + tester.accept(filename, "\\\\\\\\\\\\\\\\\\\"foo.txt"); tester.accept("\"\"foo.txt", "\\\"\\\"foo.txt"); tester.accept("\"\"\"foo.txt", "\\\"\\\"\\\"foo.txt"); - tester.accept("foo.txt\\", "foo.txt"); - tester.accept("foo.txt\\\\", "foo.txt\\\\"); - tester.accept("foo.txt\\\\\\", "foo.txt\\\\"); + tester.accept("foo.txt\\", "foo.txt\\\\"); + tester.accept("foo.txt\\\\", "foo.txt\\\\\\\\"); + tester.accept("foo.txt\\\\\\", "foo.txt\\\\\\\\\\\\"); } @Test @@ -313,4 +311,14 @@ class ContentDispositionTests { .toString()); } + @Test + void parseFormatted() { + ContentDisposition cd = ContentDisposition.builder("form-data") + .name("foo") + .filename("foo\\bar \"baz\" qux \\\" quux.txt").build(); + ContentDisposition parsed = ContentDisposition.parse(cd.toString()); + assertThat(parsed).isEqualTo(cd); + assertThat(parsed.toString()).isEqualTo(cd.toString()); + } + }