Skip to content

Commit 4cc91e4

Browse files
committed
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 spring-projectsgh-28837
1 parent 9cfe791 commit 4cc91e4

File tree

2 files changed

+55
-36
lines changed

2 files changed

+55
-36
lines changed

spring-web/src/main/java/org/springframework/http/ContentDisposition.java

+21-10
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ public String toString() {
259259
if (this.filename != null) {
260260
if (this.charset == null || StandardCharsets.US_ASCII.equals(this.charset)) {
261261
sb.append("; filename=\"");
262-
sb.append(escapeQuotationsInFilename(this.filename)).append('\"');
262+
sb.append(encodeQuotedPairs(this.filename)).append('\"');
263263
}
264264
else {
265265
sb.append("; filename*=");
@@ -404,6 +404,9 @@ else if (attribute.equals("filename") && (filename == null)) {
404404
}
405405
}
406406
}
407+
else if (value.indexOf('\\') != -1) {
408+
filename = decodeQuotedPairs(value);
409+
}
407410
else {
408411
filename = value;
409412
}
@@ -560,25 +563,33 @@ else if (b == '=' && index < value.length - 2) {
560563
return StreamUtils.copyToString(baos, charset);
561564
}
562565

563-
private static String escapeQuotationsInFilename(String filename) {
566+
private static String encodeQuotedPairs(String filename) {
564567
if (filename.indexOf('"') == -1 && filename.indexOf('\\') == -1) {
565568
return filename;
566569
}
567-
boolean escaped = false;
568570
StringBuilder sb = new StringBuilder();
569571
for (int i = 0; i < filename.length() ; i++) {
570572
char c = filename.charAt(i);
571-
if (!escaped && c == '"') {
572-
sb.append("\\\"");
573+
if (c == '"' || c == '\\') {
574+
sb.append('\\');
575+
}
576+
sb.append(c);
577+
}
578+
return sb.toString();
579+
}
580+
581+
private static String decodeQuotedPairs(String filename) {
582+
StringBuilder sb = new StringBuilder();
583+
int length = filename.length();
584+
for (int i = 0; i < length; i++) {
585+
char c = filename.charAt(i);
586+
if (filename.charAt(i) == '\\' && i + 1 < length) {
587+
i++;
588+
sb.append(filename.charAt(i));
573589
}
574590
else {
575591
sb.append(c);
576592
}
577-
escaped = (!escaped && c == '\\');
578-
}
579-
// Remove backslash at the end.
580-
if (escaped) {
581-
sb.deleteCharAt(sb.length() - 1);
582593
}
583594
return sb.toString();
584595
}

spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java

+34-26
Original file line numberDiff line numberDiff line change
@@ -149,27 +149,25 @@ void parseEncodedFilenameWithInvalidName() {
149149
.isThrownBy(() -> parse("form-data; name=\"name\"; filename*=UTF-8''%A.txt"));
150150
}
151151

152-
@Test // gh-23077
153-
@SuppressWarnings("deprecation")
154-
void parseWithEscapedQuote() {
155-
BiConsumer<String, String> tester = (description, filename) ->
156-
assertThat(parse("form-data; name=\"file\"; filename=\"" + filename + "\"; size=123"))
157-
.as(description)
158-
.isEqualTo(ContentDisposition.formData().name("file").filename(filename).size(123L).build());
159-
160-
tester.accept("Escaped quotes should be ignored",
161-
"\\\"The Twilight Zone\\\".txt");
162-
163-
tester.accept("Escaped quotes preceded by escaped backslashes should be ignored",
164-
"\\\\\\\"The Twilight Zone\\\\\\\".txt");
165-
166-
tester.accept("Escaped backslashes should not suppress quote",
167-
"The Twilight Zone \\\\");
152+
@Test
153+
void parseBackslash() {
154+
String s = "form-data; name=\"foo\"; filename=\"foo\\\\bar \\\"baz\\\" qux \\\\\\\" quux.txt\"";
155+
ContentDisposition cd = ContentDisposition.parse(
156+
s);
157+
assertThat(cd.getName()).isEqualTo("foo");
158+
assertThat(cd.getFilename()).isEqualTo("foo\\bar \"baz\" qux \\\" quux.txt");
159+
assertThat(cd.toString()).isEqualTo(s);
160+
}
168161

169-
tester.accept("Escaped backslashes should not suppress quote",
170-
"The Twilight Zone \\\\\\\\");
162+
@Test
163+
void parseBackslashInLastPosition() {
164+
ContentDisposition cd = ContentDisposition.parse("form-data; name=\"foo\"; filename=\"bar\\\"");
165+
assertThat(cd.getName()).isEqualTo("foo");
166+
assertThat(cd.getFilename()).isEqualTo("bar\\");
167+
assertThat(cd.toString()).isEqualTo("form-data; name=\"foo\"; filename=\"bar\\\\\"");
171168
}
172169

170+
173171
@Test
174172
@SuppressWarnings("deprecation")
175173
void parseWithExtraSemicolons() {
@@ -281,26 +279,26 @@ void formatWithFilenameWithQuotes() {
281279
};
282280

283281
String filename = "\"foo.txt";
284-
tester.accept(filename, "\\" + filename);
282+
tester.accept(filename, "\\\"foo.txt");
285283

286284
filename = "\\\"foo.txt";
287-
tester.accept(filename, filename);
285+
tester.accept(filename, "\\\\\\\"foo.txt");
288286

289287
filename = "\\\\\"foo.txt";
290-
tester.accept(filename, "\\" + filename);
288+
tester.accept(filename, "\\\\\\\\\\\"foo.txt");
291289

292290
filename = "\\\\\\\"foo.txt";
293-
tester.accept(filename, filename);
291+
tester.accept(filename, "\\\\\\\\\\\\\\\"foo.txt");
294292

295293
filename = "\\\\\\\\\"foo.txt";
296-
tester.accept(filename, "\\" + filename);
294+
tester.accept(filename, "\\\\\\\\\\\\\\\\\\\"foo.txt");
297295

298296
tester.accept("\"\"foo.txt", "\\\"\\\"foo.txt");
299297
tester.accept("\"\"\"foo.txt", "\\\"\\\"\\\"foo.txt");
300298

301-
tester.accept("foo.txt\\", "foo.txt");
302-
tester.accept("foo.txt\\\\", "foo.txt\\\\");
303-
tester.accept("foo.txt\\\\\\", "foo.txt\\\\");
299+
tester.accept("foo.txt\\", "foo.txt\\\\");
300+
tester.accept("foo.txt\\\\", "foo.txt\\\\\\\\");
301+
tester.accept("foo.txt\\\\\\", "foo.txt\\\\\\\\\\\\");
304302
}
305303

306304
@Test
@@ -313,4 +311,14 @@ void formatWithEncodedFilenameUsingInvalidCharset() {
313311
.toString());
314312
}
315313

314+
@Test
315+
void parseFormatted() {
316+
ContentDisposition cd = ContentDisposition.builder("form-data")
317+
.name("foo")
318+
.filename("foo\\bar \"baz\" qux \\\" quux.txt").build();
319+
ContentDisposition parsed = ContentDisposition.parse(cd.toString());
320+
assertThat(parsed).isEqualTo(cd);
321+
assertThat(parsed.toString()).isEqualTo(cd.toString());
322+
}
323+
316324
}

0 commit comments

Comments
 (0)