Explorar o código

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
Przemyslaw Gomulka %!s(int64=5) %!d(string=hai) anos
pai
achega
710500a50a

+ 14 - 8
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java

@@ -63,6 +63,7 @@ public class ParsedMediaType {
 
 
     /**
     /**
      * Parses a header value into it's parts.
      * 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
      * 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
      * Do not rely on nulls
      *
      *
@@ -90,16 +91,14 @@ public class ParsedMediaType {
                     if (paramsAsString.isEmpty()) {
                     if (paramsAsString.isEmpty()) {
                         continue;
                         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 + "]");
                         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),
                 return new ParsedMediaType(splitMediaType[0].trim().toLowerCase(Locale.ROOT),
                     splitMediaType[1].trim().toLowerCase(Locale.ROOT), parameters);
                     splitMediaType[1].trim().toLowerCase(Locale.ROOT), parameters);
@@ -108,6 +107,13 @@ public class ParsedMediaType {
         return null;
         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.
      * Resolves this instance to a MediaType instance defined in given MediaTypeRegistry.
      * Performs validation against parameters.
      * Performs validation against parameters.

+ 12 - 8
libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ParsedMediaTypeTests.java

@@ -81,14 +81,6 @@ public class ParsedMediaTypeTests extends ESTestCase {
             ParsedMediaType.parseMediaType(mediaType + "; compatible-with=123;charset=UTF-8").getParameters());
             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() {
     public void testEmptyParams() {
         String mediaType = "application/foo";
         String mediaType = "application/foo";
         ParsedMediaType parsedMediaType = ParsedMediaType.parseMediaType(mediaType + randomFrom("", " ", ";", ";;", ";;;"));
         ParsedMediaType parsedMediaType = ParsedMediaType.parseMediaType(mediaType + randomFrom("", " ", ";", ";;", ";;;"));
@@ -105,6 +97,18 @@ public class ParsedMediaTypeTests extends ESTestCase {
         exception = expectThrows(IllegalArgumentException.class,
         exception = expectThrows(IllegalArgumentException.class,
             () -> ParsedMediaType.parseMediaType(mediaType + "; char=set=unknown"));
             () -> ParsedMediaType.parseMediaType(mediaType + "; char=set=unknown"));
         assertThat(exception.getMessage(), equalTo("invalid parameters for header [application/foo; 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() {
     public void testDefaultAcceptHeader() {