Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support double-quoted values in HttpHeaders::getValuesAsList #29785

Closed
Frederick888 opened this issue Jan 9, 2023 · 2 comments
Closed

Support double-quoted values in HttpHeaders::getValuesAsList #29785

Frederick888 opened this issue Jan 9, 2023 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@Frederick888
Copy link
Contributor

Frederick888 commented Jan 9, 2023

Affects: 6.0.3


According to https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-8, the need of having , commas (and " double-quotes) in header values introduces the need of double-quoting header values, so that a comma in a header value can be distinguished from a comma delimiter for list-based fields.

However for example HttpHeaders.getValuesAsList(), which was introduced for #18797:

@RestController
class DemoController {
	@GetMapping("/headers")
	List<String> getHeaders(@RequestHeader HttpHeaders headers) {
		return headers.getValuesAsList("Example-Dates");
	}
}

If I send the example header from the RFC to it:

$ curl http://127.0.0.1:8080/headers -H 'Example-Dates: "Sat, 04 May 1996", "Wed, 14 Sep 2005"'
["\"Sat","04 May 1996\"","\"Wed","14 Sep 2005\""]

I think this should return ["Sat, 04 May 1996", "Wed, 14 Sep 2005"] instead.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 9, 2023
@Frederick888 Frederick888 changed the title Double-quoted header values are not unquoted and then treated as a single member Double-quoted header values are not unquoted and then treated as single members Jan 9, 2023
@Frederick888 Frederick888 changed the title Double-quoted header values are not unquoted and then treated as single members Double-quoted header values are not unquoted/unescaped and then treated as single members Jan 9, 2023
@Frederick888
Copy link
Contributor Author

Frederick888 commented Jan 12, 2023

It's getting late here and I somehow slapped together a patch for this half-asleep.

It's very, very messy... but seems to tackle all the weird cases I could come up with. In retrospect it's very similar to CSV files, so there may be some methods that I could've reused?

diff --git a/spring-core/src/main/java/org/springframework/util/StringUtils.java b/spring-core/src/main/java/org/springframework/util/StringUtils.java
index f2eb263234..64e38dcab6 100644
--- a/spring-core/src/main/java/org/springframework/util/StringUtils.java
+++ b/spring-core/src/main/java/org/springframework/util/StringUtils.java
@@ -75,8 +75,12 @@ public abstract class StringUtils {
 	private static final String CURRENT_PATH = ".";
 
 	private static final char EXTENSION_SEPARATOR = '.';
 
+	private static final String SPACE_LIKE_CHARACTERS = " \t";
+
+	private static final String LINE_BREAK_LIKE_CHARACTERS = "\r\n";
+
 
 	//---------------------------------------------------------------------
 	// General convenience methods for working with Strings
 	//---------------------------------------------------------------------
@@ -1237,8 +1241,84 @@ public abstract class StringUtils {
 		}
 		return toStringArray(result);
 	}
 
+	public static String[] tokenizeQuotedToStringArray(@Nullable String str, String delimiters, boolean trimTokens,
+			boolean ignoreEmptyTokens) {
+		if (str == null) {
+			return EMPTY_STRING_ARRAY;
+		}
+		if (str.indexOf('\0') > -1) {
+			throw new IllegalArgumentException("Invalid character NUL");
+		}
+
+		List<String> tokens = new ArrayList<>();
+		boolean quoting = false, escaping = false, quoteClosed = false;
+		StringBuilder sb = new StringBuilder(str.length());
+		for (int i = 0; i < str.length(); ++i) {
+			char c = str.charAt(i);
+			if (escaping) {
+				if (LINE_BREAK_LIKE_CHARACTERS.indexOf(c) > -1) {
+					throw new IllegalArgumentException("Invalid character line feed or carriage return");
+				}
+				sb.append(c);
+				escaping = false;
+				continue;
+			}
+			if (c == '"') {
+				if (sb.isEmpty()) {
+					quoting = true;
+				} else if (quoting) {
+					quoting = false;
+					quoteClosed = true;
+				} else {
+					sb.append(c);
+				}
+				continue;
+			}
+			if (quoting && c == '\\') {
+				escaping = true;
+				continue;
+			}
+			if (!quoting && (sb.isEmpty() || quoteClosed)
+					&& (SPACE_LIKE_CHARACTERS.indexOf(c) > -1 || LINE_BREAK_LIKE_CHARACTERS.indexOf(c) > -1)) {
+				continue;
+			}
+			if (!quoting && delimiters.indexOf(c) > -1) {
+				String token = (trimTokens || !quoteClosed) ? sb.toString().trim() : sb.toString();
+				if (!ignoreEmptyTokens || !token.isEmpty()) {
+					tokens.add(token);
+				}
+				sb.setLength(0);
+				quoteClosed = false;
+
+				if (i == str.length() - 1 && !ignoreEmptyTokens) {
+					tokens.add("");
+				}
+
+				continue;
+			}
+			if (quoteClosed) {
+				throw new IllegalArgumentException("Partially quoted token");
+			}
+			if (LINE_BREAK_LIKE_CHARACTERS.indexOf(c) > -1) {
+				throw new IllegalArgumentException("Invalid character line feed or carriage return");
+			}
+			sb.append(c);
+		}
+		if (!sb.isEmpty()) {
+			if (quoting || escaping) {
+				throw new IllegalArgumentException("Incomplete quoted token");
+			}
+			String token = (trimTokens || !quoteClosed) ? sb.toString().trim() : sb.toString();
+			if (!ignoreEmptyTokens || !token.isEmpty()) {
+				tokens.add(token);
+			}
+		}
+
+		return toStringArray(tokens);
+	}
+
 	/**
 	 * Convert a comma delimited list (e.g., a row from a CSV file) into an
 	 * array of strings.
 	 * @param str the input {@code String} (potentially {@code null} or empty)
diff --git a/spring-core/src/test/java/org/springframework/util/StringUtilsTests.java b/spring-core/src/test/java/org/springframework/util/StringUtilsTests.java
index 0893412146..076d9e2953 100644
--- a/spring-core/src/test/java/org/springframework/util/StringUtilsTests.java
+++ b/spring-core/src/test/java/org/springframework/util/StringUtilsTests.java
@@ -507,8 +507,42 @@ class StringUtilsTests {
 		assertThat(sa).hasSize(3);
 		assertThat(sa[0].equals("a") && sa[1].equals("b ") && sa[2].equals("c")).as("components are correct").isTrue();
 	}
 
+	@Test
+	void tokenizeQuotedToStringArrayWithQuotedDates() {
+		String[] sa = StringUtils.tokenizeQuotedToStringArray("\"Sat, 04 May 1996\", \"Wed, 14 Sep 2005\"", ",", true, false);
+		assertThat(sa).hasSize(2);
+		assertThat(sa[0].equals("Sat, 04 May 1996") && sa[1].equals("Wed, 14 Sep 2005")).as("components are correct").isTrue();
+	}
+
+	@Test
+	void tokenizeQuotedToStringArrayWithUnquotedWords() {
+		String[] sa = StringUtils.tokenizeQuotedToStringArray("Foo, Bar,\tHello,", ",", true, false);
+		assertThat(sa).hasSize(4);
+		assertThat(sa[0].equals("Foo")
+				&& sa[1].equals("Bar")
+				&& sa[2].equals("Hello")
+				&& sa[3].equals("")).as("components are correct").isTrue();
+	}
+
+	@Test
+	void tokenizeQuotedToStringArrayWithUnquotedQuotes() {
+		String[] sa = StringUtils.tokenizeQuotedToStringArray(" Hello \"World\\!\", \"\tHello, world! \"" , ",", false, false);
+		assertThat(sa).hasSize(2);
+		assertThat(sa[0].equals("Hello \"World\\!\"")
+				&& sa[1].equals("\tHello, world! ")).as("components are correct").isTrue();
+	}
+
+	@Test
+	void tokenizeQuotedToStringArrayWithEscaping() {
+		String[] sa = StringUtils.tokenizeQuotedToStringArray("Foo,\r\n\" Bar \", \"Hello, \\\"!\", \" \",", ",", true, true);
+		assertThat(sa).hasSize(3);
+		assertThat(sa[0].equals("Foo")
+				&& sa[1].equals("Bar")
+				&& sa[2].equals("Hello, \"!")).as("components are correct").isTrue();
+	}
+
 	@Test
 	void commaDelimitedListToStringArrayWithNullProducesEmptyArray() {
 		String[] sa = StringUtils.commaDelimitedListToStringArray(null);
 		assertThat(sa != null).as("String array isn't null with null input").isTrue();
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 cd5662fb73..174354dd54 100644
--- a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java
+++ b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java
@@ -1548,9 +1548,9 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
 		if (values != null) {
 			List<String> result = new ArrayList<>();
 			for (String value : values) {
 				if (value != null) {
-					Collections.addAll(result, StringUtils.tokenizeToStringArray(value, ","));
+					Collections.addAll(result, StringUtils.tokenizeQuotedToStringArray(value, ",", false, true));
 				}
 			}
 			return result;
 		}

@poutsma poutsma self-assigned this Jan 19, 2023
@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 19, 2023
@poutsma poutsma added this to the 6.0.5 milestone Jan 20, 2023
@poutsma poutsma changed the title Double-quoted header values are not unquoted/unescaped and then treated as single members Support double-quoted values in HttpHeaders::getValuesAsList Jan 20, 2023
@poutsma
Copy link
Contributor

poutsma commented Jan 20, 2023

@Frederick888 Thanks for your patch, after some refactoring I resolved the issue in 207c99e1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants