From f4b05dc2e730ca667daf8861c6eb2d9a6b83d534 Mon Sep 17 00:00:00 2001 From: Dimitrios Liapis Date: Wed, 7 Nov 2018 00:03:20 +0100 Subject: [PATCH 1/2] MediaType parsing supports comma inside quotes Issue: SPR-17459 --- .../springframework/util/MimeTypeUtils.java | 23 ++++++-- .../springframework/util/MimeTypeTests.java | 51 ++++++++++++++++++ .../org/springframework/http/MediaType.java | 23 ++++++-- .../springframework/http/MediaTypeTests.java | 53 ++++++++++++++++++- 4 files changed, 139 insertions(+), 11 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java index d6581241a3..987d1e8ca6 100644 --- a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java +++ b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java @@ -37,6 +37,7 @@ import org.springframework.util.MimeType.SpecificityComparator; * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Dimitrios Liapis * @since 4.0 */ public abstract class MimeTypeUtils { @@ -257,12 +258,24 @@ public abstract class MimeTypeUtils { if (!StringUtils.hasLength(mimeTypes)) { return Collections.emptyList(); } - String[] tokens = StringUtils.tokenizeToStringArray(mimeTypes, ","); - List result = new ArrayList<>(tokens.length); - for (String token : tokens) { - result.add(parseMimeType(token)); + boolean isQuoted = false; + int nextBeginIndex = 0; + List tokens = new ArrayList<>(); + for(int i = 0; i < mimeTypes.length() - 1; i++) { + //tokenizing on commas that are not within double quotes + if(mimeTypes.charAt(i) == ',' && !isQuoted) { + tokens.add(parseMimeType(mimeTypes.substring(nextBeginIndex,i))); + nextBeginIndex = i + 1; + //ignoring escaped double quote within double quotes + } else if(isQuoted && mimeTypes.charAt(i) == '"' && mimeTypes.charAt(i-1) == '\\') { + continue; + } else if(mimeTypes.charAt(i) == '"') { + isQuoted = !isQuoted; + } } - return result; + //either the last part of the tokenization or the original string + tokens.add(parseMimeType(mimeTypes.substring(nextBeginIndex))); + return tokens; } /** diff --git a/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java b/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java index 904f230820..bf84523a82 100644 --- a/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java +++ b/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java @@ -36,6 +36,7 @@ import static org.junit.Assert.*; * @author Arjen Poutsma * @author Juergen Hoeller * @author Sam Brannen + * @author Dimitrios Liapis */ public class MimeTypeTests { @@ -276,6 +277,56 @@ public class MimeTypeTests { assertEquals("Invalid amount of mime types", 0, mimeTypes.size()); } + // SPR-17459 + @Test + public void parseMimeTypesWithOddNumberOfDoubleQuotedCommas() { + String s = "foo/bar;param=\",\""; + List mimeTypes = MimeTypeUtils.parseMimeTypes(s); + assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); + assertEquals("Comma should be part of the mime type", s, mimeTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMimeTypesWithEvenNumberOfDoubleQuotedCommas() { + String s = "foo/bar;param=\"s,a,\""; + List mimeTypes = MimeTypeUtils.parseMimeTypes(s); + assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); + assertEquals("Comma should be part of the mime type", s, mimeTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMimeTypesWithAndWithoutDoubleQuotedCommas() { + String s = "foo/bar;param=\"s,\", text/x-c"; + List mimeTypes = MimeTypeUtils.parseMimeTypes(s); + assertEquals("Invalid amount of mime types", 2, mimeTypes.size()); + assertEquals("Comma should be part of the mime type", "foo/bar;param=\"s,\"", mimeTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMimeTypesIgnoreEscapedDoubleQuoteWithinDoubleQuotes() { + String s = "foo/bar;param=\"a\\\"b,c\""; + List mimeTypes = MimeTypeUtils.parseMimeTypes(s); + assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); + assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mimeTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMimeTypesIgnoreEscapedBackslash() { + String s = "foo/bar;param=\"\\\\\""; + List mimeTypes = MimeTypeUtils.parseMimeTypes(s); + assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); + assertEquals("Escaped backslash should be ignored when considering comma tokenization", s, mimeTypes.get(0).toString()); + + s = "foo/bar;param=\"\\,\\\""; + mimeTypes = MimeTypeUtils.parseMimeTypes(s); + assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); + assertEquals("Escaped backslash should be ignored when considering comma tokenization", s, mimeTypes.get(0).toString()); + } + @Test public void compareTo() { MimeType audioBasic = new MimeType("audio", "basic"); diff --git a/spring-web/src/main/java/org/springframework/http/MediaType.java b/spring-web/src/main/java/org/springframework/http/MediaType.java index d7962f3929..047b1e5db2 100644 --- a/spring-web/src/main/java/org/springframework/http/MediaType.java +++ b/spring-web/src/main/java/org/springframework/http/MediaType.java @@ -44,6 +44,7 @@ import org.springframework.util.StringUtils; * @author Rossen Stoyanchev * @author Sebastien Deleuze * @author Kazuki Shimizu + * @author Dimitrios Liapis * @since 3.0 * @see * HTTP 1.1: Semantics and Content, section 3.1.1.1 @@ -552,12 +553,24 @@ public class MediaType extends MimeType implements Serializable { if (!StringUtils.hasLength(mediaTypes)) { return Collections.emptyList(); } - String[] tokens = StringUtils.tokenizeToStringArray(mediaTypes, ","); - List result = new ArrayList<>(tokens.length); - for (String token : tokens) { - result.add(parseMediaType(token)); + boolean isQuoted = false; + int nextBeginIndex = 0; + List tokens = new ArrayList<>(); + for(int i = 0; i < mediaTypes.length() - 1; i++) { + //tokenizing on commas that are not within double quotes + if(mediaTypes.charAt(i) == ',' && !isQuoted) { + tokens.add(parseMediaType(mediaTypes.substring(nextBeginIndex, i))); + nextBeginIndex = i + 1; + //ignoring escaped double quote within double quotes + } else if(isQuoted && mediaTypes.charAt(i) == '"' && mediaTypes.charAt(i-1) == '\\') { + continue; + } else if(mediaTypes.charAt(i) == '"') { + isQuoted = !isQuoted; + } } - return result; + //either the last part of the tokenization or the original string + tokens.add(parseMediaType(mediaTypes.substring(nextBeginIndex))); + return tokens; } /** diff --git a/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java b/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java index fcee3943c0..e537c1b497 100644 --- a/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java +++ b/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -32,6 +32,7 @@ import static org.junit.Assert.*; /** * @author Arjen Poutsma * @author Juergen Hoeller + * @author Dimitrios Liapis */ public class MediaTypeTests { @@ -143,6 +144,56 @@ public class MediaTypeTests { assertEquals("Invalid amount of media types", 0, mediaTypes.size()); } + // SPR-17459 + @Test + public void parseMediaTypesWithOddNumberOfDoubleQuotedCommas() { + String s = "foo/bar;param=\",\""; + List mediaTypes = MediaType.parseMediaTypes(s); + assertEquals("Invalid amount of media types", 1, mediaTypes.size()); + assertEquals("Comma should be part of the media type", s, mediaTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMediaTypesWithEvenNumberOfDoubleQuotedCommas() { + String s = "foo/bar;param=\"s,a,\""; + List mediaTypes = MediaType.parseMediaTypes(s); + assertEquals("Invalid amount of media types", 1, mediaTypes.size()); + assertEquals("Comma should be part of the media type", s, mediaTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMediaTypesWithAndWithoutDoubleQuotedCommas() { + String s = "foo/bar;param=\"s,\", text/x-c"; + List mediaTypes = MediaType.parseMediaTypes(s); + assertEquals("Invalid amount of media types", 2, mediaTypes.size()); + assertEquals("Comma should be part of the media type", "foo/bar;param=\"s,\"", mediaTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMediaTypesIgnoreEscapedDoubleQuoteWithinDoubleQuotes() { + String s = "foo/bar;param=\"a\\\"b,c\""; + List mediaTypes = MediaType.parseMediaTypes(s); + assertEquals("Invalid amount of media types", 1, mediaTypes.size()); + assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mediaTypes.get(0).toString()); + } + + // SPR-17459 + @Test + public void parseMediaTypesIgnoreEscapedBackslash() { + String s = "foo/bar;param=\"\\\\\""; + List mediaTypes = MediaType.parseMediaTypes(s); + assertEquals("Invalid amount of media types", 1, mediaTypes.size()); + assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mediaTypes.get(0).toString()); + + s = "foo/bar;param=\"\\,\\\""; + mediaTypes = MediaType.parseMediaTypes(s); + assertEquals("Invalid amount of media types", 1, mediaTypes.size()); + assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mediaTypes.get(0).toString()); + } + @Test public void compareTo() { MediaType audioBasic = new MediaType("audio", "basic"); From ba3fef3e8a55c56523deb0ba436fee6c74c4d60a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 13 Nov 2018 22:43:29 -0500 Subject: [PATCH 2/2] Refactor media types parsing improvements Issue: SPR-17459 --- .../springframework/util/MimeTypeUtils.java | 56 +++++++++++----- .../springframework/util/MimeTypeTests.java | 65 +++++-------------- .../org/springframework/http/MediaType.java | 23 +------ .../springframework/http/MediaTypeTests.java | 53 +-------------- 4 files changed, 60 insertions(+), 137 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java index 987d1e8ca6..d09ee8c4a2 100644 --- a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java +++ b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java @@ -28,6 +28,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Random; +import java.util.stream.Collectors; import org.springframework.lang.Nullable; import org.springframework.util.MimeType.SpecificityComparator; @@ -249,32 +250,53 @@ public abstract class MimeTypeUtils { } /** - * Parse the given, comma-separated string into a list of {@code MimeType} objects. + * Parse the comma-separated string into a list of {@code MimeType} objects. * @param mimeTypes the string to parse * @return the list of mime types - * @throws IllegalArgumentException if the string cannot be parsed + * @throws InvalidMimeTypeException if the string cannot be parsed */ public static List parseMimeTypes(String mimeTypes) { if (!StringUtils.hasLength(mimeTypes)) { return Collections.emptyList(); } - boolean isQuoted = false; - int nextBeginIndex = 0; - List tokens = new ArrayList<>(); - for(int i = 0; i < mimeTypes.length() - 1; i++) { - //tokenizing on commas that are not within double quotes - if(mimeTypes.charAt(i) == ',' && !isQuoted) { - tokens.add(parseMimeType(mimeTypes.substring(nextBeginIndex,i))); - nextBeginIndex = i + 1; - //ignoring escaped double quote within double quotes - } else if(isQuoted && mimeTypes.charAt(i) == '"' && mimeTypes.charAt(i-1) == '\\') { - continue; - } else if(mimeTypes.charAt(i) == '"') { - isQuoted = !isQuoted; + return tokenize(mimeTypes).stream() + .map(MimeTypeUtils::parseMimeType).collect(Collectors.toList()); + } + + /** + * Tokenize the given comma-separated string of {@code MimeType} objects + * into a {@code List}. Unlike simple tokenization by ",", this + * method takes into account quoted parameters. + * @param mimeTypes the string to tokenize + * @return the list of tokens + * @since 5.1.3 + */ + public static List tokenize(String mimeTypes) { + if (!StringUtils.hasLength(mimeTypes)) { + return Collections.emptyList(); + } + List tokens = new ArrayList<>(); + boolean inQuotes = false; + int startIndex = 0; + int i = 0; + while (i < mimeTypes.length()) { + switch (mimeTypes.charAt(i)) { + case '"': + inQuotes = !inQuotes; + break; + case ',': + if (!inQuotes) { + tokens.add(mimeTypes.substring(startIndex, i)); + startIndex = i + 1; + } + break; + case '\\': + i++; + break; } + i++; } - //either the last part of the tokenization or the original string - tokens.add(parseMimeType(mimeTypes.substring(nextBeginIndex))); + tokens.add(mimeTypes.substring(startIndex)); return tokens; } diff --git a/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java b/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java index bf84523a82..07e3b54358 100644 --- a/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java +++ b/spring-core/src/test/java/org/springframework/util/MimeTypeTests.java @@ -277,54 +277,23 @@ public class MimeTypeTests { assertEquals("Invalid amount of mime types", 0, mimeTypes.size()); } - // SPR-17459 - @Test - public void parseMimeTypesWithOddNumberOfDoubleQuotedCommas() { - String s = "foo/bar;param=\",\""; - List mimeTypes = MimeTypeUtils.parseMimeTypes(s); - assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); - assertEquals("Comma should be part of the mime type", s, mimeTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMimeTypesWithEvenNumberOfDoubleQuotedCommas() { - String s = "foo/bar;param=\"s,a,\""; - List mimeTypes = MimeTypeUtils.parseMimeTypes(s); - assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); - assertEquals("Comma should be part of the mime type", s, mimeTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMimeTypesWithAndWithoutDoubleQuotedCommas() { - String s = "foo/bar;param=\"s,\", text/x-c"; - List mimeTypes = MimeTypeUtils.parseMimeTypes(s); - assertEquals("Invalid amount of mime types", 2, mimeTypes.size()); - assertEquals("Comma should be part of the mime type", "foo/bar;param=\"s,\"", mimeTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMimeTypesIgnoreEscapedDoubleQuoteWithinDoubleQuotes() { - String s = "foo/bar;param=\"a\\\"b,c\""; - List mimeTypes = MimeTypeUtils.parseMimeTypes(s); - assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); - assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mimeTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMimeTypesIgnoreEscapedBackslash() { - String s = "foo/bar;param=\"\\\\\""; - List mimeTypes = MimeTypeUtils.parseMimeTypes(s); - assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); - assertEquals("Escaped backslash should be ignored when considering comma tokenization", s, mimeTypes.get(0).toString()); - - s = "foo/bar;param=\"\\,\\\""; - mimeTypes = MimeTypeUtils.parseMimeTypes(s); - assertEquals("Invalid amount of mime types", 1, mimeTypes.size()); - assertEquals("Escaped backslash should be ignored when considering comma tokenization", s, mimeTypes.get(0).toString()); + @Test // SPR-17459 + public void parseMimeTypesWithQuotedParameters() { + testWithQuotedParameters("foo/bar;param=\",\""); + testWithQuotedParameters("foo/bar;param=\"s,a,\""); + testWithQuotedParameters("foo/bar;param=\"s,\"", "text/x-c"); + testWithQuotedParameters("foo/bar;param=\"a\\\"b,c\""); + testWithQuotedParameters("foo/bar;param=\"\\\\\""); + testWithQuotedParameters("foo/bar;param=\"\\,\\\""); + } + + private void testWithQuotedParameters(String... mimeTypes) { + String s = String.join(",", mimeTypes); + List actual = MimeTypeUtils.parseMimeTypes(s); + assertEquals(mimeTypes.length, actual.size()); + for (int i=0; i < mimeTypes.length; i++) { + assertEquals(mimeTypes[i], actual.get(i).toString()); + } } @Test diff --git a/spring-web/src/main/java/org/springframework/http/MediaType.java b/spring-web/src/main/java/org/springframework/http/MediaType.java index 047b1e5db2..3c6bfea981 100644 --- a/spring-web/src/main/java/org/springframework/http/MediaType.java +++ b/spring-web/src/main/java/org/springframework/http/MediaType.java @@ -44,7 +44,6 @@ import org.springframework.util.StringUtils; * @author Rossen Stoyanchev * @author Sebastien Deleuze * @author Kazuki Shimizu - * @author Dimitrios Liapis * @since 3.0 * @see * HTTP 1.1: Semantics and Content, section 3.1.1.1 @@ -543,7 +542,7 @@ public class MediaType extends MimeType implements Serializable { } /** - * Parse the given comma-separated string into a list of {@code MediaType} objects. + * Parse the comma-separated string into a list of {@code MediaType} objects. *

This method can be used to parse an Accept or Content-Type header. * @param mediaTypes the string to parse * @return the list of media types @@ -553,24 +552,8 @@ public class MediaType extends MimeType implements Serializable { if (!StringUtils.hasLength(mediaTypes)) { return Collections.emptyList(); } - boolean isQuoted = false; - int nextBeginIndex = 0; - List tokens = new ArrayList<>(); - for(int i = 0; i < mediaTypes.length() - 1; i++) { - //tokenizing on commas that are not within double quotes - if(mediaTypes.charAt(i) == ',' && !isQuoted) { - tokens.add(parseMediaType(mediaTypes.substring(nextBeginIndex, i))); - nextBeginIndex = i + 1; - //ignoring escaped double quote within double quotes - } else if(isQuoted && mediaTypes.charAt(i) == '"' && mediaTypes.charAt(i-1) == '\\') { - continue; - } else if(mediaTypes.charAt(i) == '"') { - isQuoted = !isQuoted; - } - } - //either the last part of the tokenization or the original string - tokens.add(parseMediaType(mediaTypes.substring(nextBeginIndex))); - return tokens; + return MimeTypeUtils.tokenize(mediaTypes).stream() + .map(MediaType::parseMediaType).collect(Collectors.toList()); } /** diff --git a/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java b/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java index e537c1b497..fcee3943c0 100644 --- a/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java +++ b/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2016 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. @@ -32,7 +32,6 @@ import static org.junit.Assert.*; /** * @author Arjen Poutsma * @author Juergen Hoeller - * @author Dimitrios Liapis */ public class MediaTypeTests { @@ -144,56 +143,6 @@ public class MediaTypeTests { assertEquals("Invalid amount of media types", 0, mediaTypes.size()); } - // SPR-17459 - @Test - public void parseMediaTypesWithOddNumberOfDoubleQuotedCommas() { - String s = "foo/bar;param=\",\""; - List mediaTypes = MediaType.parseMediaTypes(s); - assertEquals("Invalid amount of media types", 1, mediaTypes.size()); - assertEquals("Comma should be part of the media type", s, mediaTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMediaTypesWithEvenNumberOfDoubleQuotedCommas() { - String s = "foo/bar;param=\"s,a,\""; - List mediaTypes = MediaType.parseMediaTypes(s); - assertEquals("Invalid amount of media types", 1, mediaTypes.size()); - assertEquals("Comma should be part of the media type", s, mediaTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMediaTypesWithAndWithoutDoubleQuotedCommas() { - String s = "foo/bar;param=\"s,\", text/x-c"; - List mediaTypes = MediaType.parseMediaTypes(s); - assertEquals("Invalid amount of media types", 2, mediaTypes.size()); - assertEquals("Comma should be part of the media type", "foo/bar;param=\"s,\"", mediaTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMediaTypesIgnoreEscapedDoubleQuoteWithinDoubleQuotes() { - String s = "foo/bar;param=\"a\\\"b,c\""; - List mediaTypes = MediaType.parseMediaTypes(s); - assertEquals("Invalid amount of media types", 1, mediaTypes.size()); - assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mediaTypes.get(0).toString()); - } - - // SPR-17459 - @Test - public void parseMediaTypesIgnoreEscapedBackslash() { - String s = "foo/bar;param=\"\\\\\""; - List mediaTypes = MediaType.parseMediaTypes(s); - assertEquals("Invalid amount of media types", 1, mediaTypes.size()); - assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mediaTypes.get(0).toString()); - - s = "foo/bar;param=\"\\,\\\""; - mediaTypes = MediaType.parseMediaTypes(s); - assertEquals("Invalid amount of media types", 1, mediaTypes.size()); - assertEquals("Escaped quote within quotes should be ignored when considering comma tokenization", s, mediaTypes.get(0).toString()); - } - @Test public void compareTo() { MediaType audioBasic = new MediaType("audio", "basic");