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
This commit is contained in:
Arjen Poutsma 2022-09-06 15:47:25 +02:00
parent 9cfe79186d
commit 4cc91e46b2
2 changed files with 56 additions and 37 deletions

View File

@ -259,7 +259,7 @@ public final class ContentDisposition {
if (this.filename != null) { if (this.filename != null) {
if (this.charset == null || StandardCharsets.US_ASCII.equals(this.charset)) { if (this.charset == null || StandardCharsets.US_ASCII.equals(this.charset)) {
sb.append("; filename=\""); sb.append("; filename=\"");
sb.append(escapeQuotationsInFilename(this.filename)).append('\"'); sb.append(encodeQuotedPairs(this.filename)).append('\"');
} }
else { else {
sb.append("; filename*="); sb.append("; filename*=");
@ -404,6 +404,9 @@ public final class ContentDisposition {
} }
} }
} }
else if (value.indexOf('\\') != -1) {
filename = decodeQuotedPairs(value);
}
else { else {
filename = value; filename = value;
} }
@ -560,25 +563,33 @@ public final class ContentDisposition {
return StreamUtils.copyToString(baos, charset); return StreamUtils.copyToString(baos, charset);
} }
private static String escapeQuotationsInFilename(String filename) { private static String encodeQuotedPairs(String filename) {
if (filename.indexOf('"') == -1 && filename.indexOf('\\') == -1) { if (filename.indexOf('"') == -1 && filename.indexOf('\\') == -1) {
return filename; return filename;
} }
boolean escaped = false;
StringBuilder sb = new StringBuilder(); StringBuilder sb = new StringBuilder();
for (int i = 0; i < filename.length() ; i++) { for (int i = 0; i < filename.length() ; i++) {
char c = filename.charAt(i); char c = filename.charAt(i);
if (!escaped && c == '"') { if (c == '"' || c == '\\') {
sb.append("\\\""); 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 { else {
sb.append(c); sb.append(c);
} }
escaped = (!escaped && c == '\\');
}
// Remove backslash at the end.
if (escaped) {
sb.deleteCharAt(sb.length() - 1);
} }
return sb.toString(); return sb.toString();
} }

View File

@ -149,27 +149,25 @@ class ContentDispositionTests {
.isThrownBy(() -> parse("form-data; name=\"name\"; filename*=UTF-8''%A.txt")); .isThrownBy(() -> parse("form-data; name=\"name\"; filename*=UTF-8''%A.txt"));
} }
@Test // gh-23077 @Test
@SuppressWarnings("deprecation") void parseBackslash() {
void parseWithEscapedQuote() { String s = "form-data; name=\"foo\"; filename=\"foo\\\\bar \\\"baz\\\" qux \\\\\\\" quux.txt\"";
BiConsumer<String, String> tester = (description, filename) -> ContentDisposition cd = ContentDisposition.parse(
assertThat(parse("form-data; name=\"file\"; filename=\"" + filename + "\"; size=123")) s);
.as(description) assertThat(cd.getName()).isEqualTo("foo");
.isEqualTo(ContentDisposition.formData().name("file").filename(filename).size(123L).build()); assertThat(cd.getFilename()).isEqualTo("foo\\bar \"baz\" qux \\\" quux.txt");
assertThat(cd.toString()).isEqualTo(s);
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 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 @Test
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
void parseWithExtraSemicolons() { void parseWithExtraSemicolons() {
@ -281,26 +279,26 @@ class ContentDispositionTests {
}; };
String filename = "\"foo.txt"; String filename = "\"foo.txt";
tester.accept(filename, "\\" + filename); tester.accept(filename, "\\\"foo.txt");
filename = "\\\"foo.txt"; filename = "\\\"foo.txt";
tester.accept(filename, filename); tester.accept(filename, "\\\\\\\"foo.txt");
filename = "\\\\\"foo.txt"; filename = "\\\\\"foo.txt";
tester.accept(filename, "\\" + filename); tester.accept(filename, "\\\\\\\\\\\"foo.txt");
filename = "\\\\\\\"foo.txt"; filename = "\\\\\\\"foo.txt";
tester.accept(filename, filename); tester.accept(filename, "\\\\\\\\\\\\\\\"foo.txt");
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\\\\\\\\");
tester.accept("foo.txt\\\\\\", "foo.txt\\\\"); tester.accept("foo.txt\\\\\\", "foo.txt\\\\\\\\\\\\");
} }
@Test @Test
@ -313,4 +311,14 @@ class ContentDispositionTests {
.toString()); .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());
}
} }