Do not allow spaces within MediaType's parameters (#64650)

Per https://tools.ietf.org/html/rfc7231#section-3.1.1.1 MediaType can have spaces around parameters pair, but do not allow spaces within parameter. That means that parameter pair forbids spaces around = follow up after #64406 (comment) relates #51816
elastic · Nov 6, 2020 · 710500a · 710500a
1 parent 47da916
commit 710500a
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 16 deletions.
diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java
@@ -63,6 +63,7 @@ public Map<String, String> getParameters() {
 
     /**
      * Parses a header value into it's parts.
+     * follows https://tools.ietf.org/html/rfc7231#section-3.1.1.1 but allows only single media type and do not support quality factors
      * Note: parsing can return null, but it will throw exceptions once https://github.com/elastic/elasticsearch/issues/63080 is done
      * Do not rely on nulls
      *
@@ -90,16 +91,14 @@ public static ParsedMediaType parseMediaType(String headerValue) {
                     if (paramsAsString.isEmpty()) {
                         continue;
                     }
-                    // intentionally allowing to have spaces around `=`
-                    // https://tools.ietf.org/html/rfc7231#section-3.1.1.1 disallows this
-                    String[] keyValueParam = elements[i].trim().split("=");
-                    if (keyValueParam.length == 2) {
-                        String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT).trim();
-                        String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT).trim();
-                        parameters.put(parameterName, parameterValue);
-                    } else {
+                    //spaces are allowed between parameters, but not between '=' sign
+                    String[] keyValueParam = paramsAsString.split("=");
+                    if (keyValueParam.length != 2 || hasTrailingSpace(keyValueParam[0]) || hasLeadingSpace(keyValueParam[1])) {
                         throw new IllegalArgumentException("invalid parameters for header [" + headerValue + "]");
                     }
+                    String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT).trim();
+                    String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT).trim();
+                    parameters.put(parameterName, parameterValue);
                 }
                 return new ParsedMediaType(splitMediaType[0].trim().toLowerCase(Locale.ROOT),
                     splitMediaType[1].trim().toLowerCase(Locale.ROOT), parameters);
@@ -108,6 +107,13 @@ public static ParsedMediaType parseMediaType(String headerValue) {
         return null;
     }
 
+    private static boolean hasTrailingSpace(String s) {
+        return s.length() == 0 || Character.isWhitespace(s.charAt(s.length()-1));
+    }
+
+    private static boolean hasLeadingSpace(String s) {
+        return s.length() == 0 || Character.isWhitespace(s.charAt(0));
+    }
     /**
      * Resolves this instance to a MediaType instance defined in given MediaTypeRegistry.
      * Performs validation against parameters.

diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ParsedMediaTypeTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ParsedMediaTypeTests.java
@@ -81,14 +81,6 @@ public void testWithParameters() {
             ParsedMediaType.parseMediaType(mediaType + "; compatible-with=123;charset=UTF-8").getParameters());
     }
 
-    public void testWhiteSpaces() {
-        //be lenient with white space since it can be really hard to troubleshoot
-        String mediaType = "  application/foo  ";
-        ParsedMediaType parsedMediaType = ParsedMediaType.parseMediaType(mediaType + "    ;  compatible-with =  123  ;  charset=UTF-8");
-        assertEquals("application/foo", parsedMediaType.mediaTypeWithoutParameters());
-        assertEquals((Map.of("charset", "utf-8", "compatible-with", "123")), parsedMediaType.getParameters());
-    }
-
     public void testEmptyParams() {
         String mediaType = "application/foo";
         ParsedMediaType parsedMediaType = ParsedMediaType.parseMediaType(mediaType + randomFrom("", " ", ";", ";;", ";;;"));
@@ -105,6 +97,18 @@ public void testMalformedParameters() {
         exception = expectThrows(IllegalArgumentException.class,
             () -> ParsedMediaType.parseMediaType(mediaType + "; char=set=unknown"));
         assertThat(exception.getMessage(), equalTo("invalid parameters for header [application/foo; char=set=unknown]"));
+
+        // do not allow white space in parameters between `=`
+        exception = expectThrows(IllegalArgumentException.class,
+            () -> ParsedMediaType.parseMediaType(mediaType + "    ;  compatible-with =  123  ;  charset=UTF-8"));
+        assertThat(exception.getMessage(),
+            equalTo("invalid parameters for header [application/foo    ;  compatible-with =  123  ;  charset=UTF-8]"));
+
+        expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k =y"));
+        expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k= y"));
+        expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k = y"));
+        expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";= y"));
+        expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k="));
     }
 
     public void testDefaultAcceptHeader() {