From 85c5a0c1a943f4485f3d4248d54c7b11cab176b3 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 21 Apr 2020 08:28:42 +0100 Subject: [PATCH] Escape quoted filename in HttpHeaders This is primarily to align with similar changes applied to ContentDisposition in 5.x. Closes gh-24580 --- .../org/springframework/http/HttpHeaders.java | 51 +++++++---- .../http/HttpHeadersTests.java | 91 ++++++++++++++----- 2 files changed, 97 insertions(+), 45 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java index 0cde42e4a7..1559d1d771 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java +++ b/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 class HttpHeaders implements MultiValueMap, Serializable /** * 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"}. *

Applications typically would not set this header directly but * rather prepare a {@code MultiValueMap}, containing an * Object or a {@link org.springframework.core.io.Resource} for each part, @@ -686,7 +689,7 @@ public class HttpHeaders implements MultiValueMap, Serializable 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 class HttpHeaders implements MultiValueMap, Serializable */ @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,6 +1320,23 @@ public class HttpHeaders implements MultiValueMap, Serializable 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 @@ -1331,12 +1344,10 @@ public class HttpHeaders implements MultiValueMap, Serializable * @return the encoded header field param * @see RFC 5987 */ - 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); diff --git a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java index 8c64779ef8..870956dc7b 100644 --- a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java +++ b/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.GregorianCalendar; 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 class HttpHeadersTests { 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")); + } + + @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", Charset.forName("UTF-8")); + 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 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 class HttpHeadersTests { 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")); - } - }