Escape quoted filename in HttpHeaders

This is primarily to align with similar changes applied to
ContentDisposition in 5.x.

Closes gh-24580

Author: rstoyanchev

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -672,7 +672,10 @@ public List<String> getConnection() {
 
 	/**
 	 * Set the {@code Content-Disposition} header when creating a
-	 * {@code "multipart/form-data"} request.
+	 * {@code "multipart/form-data"} request. The given filename is formatted
+	 * as a 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"}.
 	 * <p>Applications typically would not set this header directly but
 	 * rather prepare a {@code MultiValueMap<String, Object>}, containing an
 	 * Object or a {@link org.springframework.core.io.Resource} for each part,
@@ -686,7 +689,7 @@ public void setContentDispositionFormData(String name, String filename) {
 		builder.append(name).append('\"');
 		if (filename != null) {
 			builder.append("; filename=\"");
-			builder.append(filename).append('\"');
+			builder.append(escapeQuotationsInFilename(filename)).append('\"');
 		}
 		set(CONTENT_DISPOSITION, builder.toString());
 	}
@@ -707,20 +710,13 @@ public void setContentDispositionFormData(String name, String filename) {
 	 */
 	@Deprecated
 	public void setContentDispositionFormData(String name, String filename, Charset charset) {
-		Assert.notNull(name, "'name' must not be null");
-		StringBuilder builder = new StringBuilder("form-data; name=\"");
-		builder.append(name).append('\"');
-		if (filename != null) {
-			if (charset == null || charset.name().equals("US-ASCII")) {
-				builder.append("; filename=\"");
-				builder.append(filename).append('\"');
-			}
-			else {
-				builder.append("; filename*=");
-				builder.append(encodeHeaderFieldParam(filename, charset));
-			}
+		if (filename == null || charset == null || charset.name().equals("US-ASCII")) {
+			setContentDispositionFormData(name, filename);
+			return;
 		}
-		set(CONTENT_DISPOSITION, builder.toString());
+		Assert.notNull(name, "'name' must not be null");
+		String encodedFileName = encodeHeaderFieldParam(filename, charset);
+		set(CONTENT_DISPOSITION, "form-data; name=\"" + name + '\"' + "; filename*=" + encodedFileName);
 	}
 
 	/**
@@ -1324,19 +1320,34 @@ public static HttpHeaders readOnlyHttpHeaders(HttpHeaders headers) {
 		return new HttpHeaders(headers, true);
 	}
 
+	private static String escapeQuotationsInFilename(String filename) {
+		if (filename.indexOf('"') == -1 && 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 == '\\');
+		}
+		// Remove backslash at the end..
+		if (escaped) {
+			sb.deleteCharAt(sb.length() - 1);
+		}
+		return sb.toString();
+	}
+
 	/**
 	 * Encode the given header field param as describe in RFC 5987.
 	 * @param input the header field param
 	 * @param charset the charset of the header field param string
 	 * @return the encoded header field param
 	 * @see <a href="https://tools.ietf.org/html/rfc5987">RFC 5987</a>
 	 */
-	static String encodeHeaderFieldParam(String input, Charset charset) {
+	private static String encodeHeaderFieldParam(String input, Charset charset) {
 		Assert.notNull(input, "Input String should not be null");
 		Assert.notNull(charset, "Charset should not be null");
-		if (charset.name().equals("US-ASCII")) {
-			return input;
-		}
+		Assert.isTrue(!charset.name().equals("US-ASCII"), "ASCII does not require encoding");
 		Assert.isTrue(charset.name().equals("UTF-8") || charset.name().equals("ISO-8859-1"),
 				"Charset should be UTF-8 or ISO-8859-1");
 		byte[] source = input.getBytes(charset);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,9 +16,13 @@
 
 package org.springframework.http;
 
+import org.hamcrest.Matchers;
+import org.junit.Test;
+
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
@@ -28,12 +32,15 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.TimeZone;
+import java.util.function.BiConsumer;
 
-import org.hamcrest.Matchers;
-import org.junit.Test;
-
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Unit tests for {@link org.springframework.http.HttpHeaders}.
@@ -311,21 +318,70 @@ public void cacheControlAllValues() {
 		assertThat(headers.getCacheControl(), is("max-age=1000, public, s-maxage=1000"));
 	}
 
-	@SuppressWarnings("deprecation")
 	@Test
 	public void contentDisposition() {
 		headers.setContentDispositionFormData("name", null);
 		assertEquals("Invalid Content-Disposition header", "form-data; name=\"name\"",
 				headers.getFirst("Content-Disposition"));
 
-		headers.setContentDispositionFormData("name", "filename");
-		assertEquals("Invalid Content-Disposition header", "form-data; name=\"name\"; filename=\"filename\"",
+		headers.setContentDispositionFormData("name", "foo.txt");
+		assertEquals("Invalid Content-Disposition header", "form-data; name=\"name\"; filename=\"foo.txt\"",
 				headers.getFirst("Content-Disposition"));
+	}
 
-		headers.setContentDispositionFormData("name", "中文.txt", Charset.forName("UTF-8"));
+	@SuppressWarnings("deprecation")
+	@Test // SPR-14547
+	public void contentDispositionWithCharset() {
+
+		headers.setContentDispositionFormData("name", "foo.txt", StandardCharsets.US_ASCII);
+		assertEquals("Invalid Content-Disposition header", "form-data; name=\"name\"; filename=\"foo.txt\"",
+				headers.getFirst("Content-Disposition"));
+
+		headers.setContentDispositionFormData("name", "中文.txt", StandardCharsets.UTF_8);
 		assertEquals("Invalid Content-Disposition header",
 				"form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt",
 				headers.getFirst("Content-Disposition"));
+
+		try {
+			headers.setContentDispositionFormData("name", "foo.txt", StandardCharsets.UTF_16);
+			fail();
+		}
+		catch (IllegalArgumentException ex) {
+			// expected
+		}
+	}
+
+	@Test // gh-24580
+	public void contentDispositionWithFilenameWithQuotes() {
+		BiConsumer<String, String> tester = (filenameIn, filenameOut) -> {
+			headers.setContentDispositionFormData("name", filenameIn);
+			assertEquals("form-data; name=\"name\"; filename=\"" + filenameOut + "\"",
+					headers.getFirst("Content-Disposition"));
+		};
+
+		tester.accept("foo.txt", "foo.txt");
+
+		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");
+
+		tester.accept("foo.txt\\", "foo.txt");
+		tester.accept("foo.txt\\\\", "foo.txt\\\\");
+		tester.accept("foo.txt\\\\\\", "foo.txt\\\\");
 	}
 
 	@Test  // SPR-11917
@@ -409,19 +465,4 @@ public void accessControlRequestMethod() {
 		headers.setAccessControlRequestMethod(HttpMethod.POST);
 		assertEquals(HttpMethod.POST, headers.getAccessControlRequestMethod());
 	}
-
-	@Test  // SPR-14547
-	public void encodeHeaderFieldParam() {
-		String result = HttpHeaders.encodeHeaderFieldParam("test.txt", Charset.forName("US-ASCII"));
-		assertEquals("test.txt", result);
-
-		result = HttpHeaders.encodeHeaderFieldParam("中文.txt", Charset.forName("UTF-8"));
-		assertEquals("UTF-8''%E4%B8%AD%E6%96%87.txt", result);
-	}
-
-	@Test(expected = IllegalArgumentException.class)
-	public void encodeHeaderFieldParamInvalidCharset() {
-		HttpHeaders.encodeHeaderFieldParam("test", Charset.forName("UTF-16"));
-	}
-
 }