Escape quotes in filename

Closes gh-24220
This commit is contained in:
Rossen Stoyanchev 2019-12-19 12:23:12 +00:00
parent 44da775134
commit 41f40c6c22
2 changed files with 80 additions and 21 deletions

View File

@ -458,7 +458,11 @@ public final class ContentDisposition {
Builder name(String name); 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); Builder filename(String filename);
@ -539,10 +543,24 @@ public final class ContentDisposition {
@Override @Override
public Builder filename(String filename) { public Builder filename(String filename) {
this.filename = filename; Assert.hasText(filename, "No filename");
this.filename = escapeQuotationMarks(filename);
return this; 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 @Override
public Builder filename(String filename, Charset charset) { public Builder filename(String filename, Charset charset) {
this.filename = filename; this.filename = filename;

View File

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