Allow parsing Content-Type and Accept headers with version

buildSrc/src/main/java/org/elasticsearch/gradle/http/WaitForHttpResource.java

@@ -145,6 +145,7 @@ HttpURLConnection buildConnection(SSLContext ssl) throws IOException {
         configureSslContext(connection, ssl);
         configureBasicAuth(connection);
         connection.setRequestMethod("GET");
+        connection.setRequestProperty("Accept", "text/plain; charset=UTF-8");
         return connection;
     }
 

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

@@ -1900,9 +1900,9 @@ protected final <Resp> Resp parseEntity(final HttpEntity entity,
             throw new IllegalStateException("Elasticsearch didn't return the [Content-Type] header, unable to parse response body");
         }
         XContentType xContentType = XContentType.fromMediaType(entity.getContentType().getValue());
-        if (xContentType == null) {
-            throw new IllegalStateException("Unsupported Content-Type: " + entity.getContentType().getValue());
-        }
+//        if (xContentType == null) {
+//            throw new IllegalStateException("Unsupported Content-Type: " + entity.getContentType().getValue());
+//        }
         try (XContentParser parser = xContentType.xContent().createParser(registry, DEPRECATION_HANDLER, entity.getContent())) {
             return entityParser.apply(parser);
         }

client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java

@@ -302,8 +302,9 @@ public void testParseEntity() throws IOException {
         }
         {
             NStringEntity entity = new NStringEntity("", ContentType.APPLICATION_SVG_XML);
-            IllegalStateException ise = expectThrows(IllegalStateException.class, () -> restHighLevelClient.parseEntity(entity, null));
-            assertEquals("Unsupported Content-Type: " + entity.getContentType().getValue(), ise.getMessage());
+            IllegalArgumentException ise = expectThrows(IllegalArgumentException.class,
+                () -> restHighLevelClient.parseEntity(entity, null));
+            assertEquals("unrecognized media type: " + entity.getContentType().getValue(), ise.getMessage());
         }
         {
             CheckedFunction<XContentParser, String, IOException> entityParser = parser -> {

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java

@@ -22,33 +22,18 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 public class MediaTypeParser<T extends MediaType> {
     private final Map<String, T> formatToMediaType;
     private final Map<String, T> typeWithSubtypeToMediaType;
+    private final Map<String, Map<String, Pattern>> parametersMap;
 
-    public MediaTypeParser(T[] acceptedMediaTypes) {
-        this(acceptedMediaTypes, Map.of());
-    }
-
-    public MediaTypeParser(T[] acceptedMediaTypes, Map<String, T> additionalMediaTypes) {
-        final int size = acceptedMediaTypes.length + additionalMediaTypes.size();
-        Map<String, T> formatMap = new HashMap<>(size);
-        Map<String, T> typeMap = new HashMap<>(size);
-        for (T mediaType : acceptedMediaTypes) {
-            typeMap.put(mediaType.typeWithSubtype(), mediaType);
-            formatMap.put(mediaType.format(), mediaType);
-        }
-        for (Map.Entry<String, T> entry : additionalMediaTypes.entrySet()) {
-            String typeWithSubtype = entry.getKey();
-            T mediaType = entry.getValue();
-
-            typeMap.put(typeWithSubtype.toLowerCase(Locale.ROOT), mediaType);
-            formatMap.put(mediaType.format(), mediaType);
-        }
-
-        this.formatToMediaType = Map.copyOf(formatMap);
-        this.typeWithSubtypeToMediaType = Map.copyOf(typeMap);
+    public MediaTypeParser(Map<String, T> formatToMediaType, Map<String, T> typeWithSubtypeToMediaType,
+                           Map<String, Map<String, Pattern>> parametersMap) {
+        this.formatToMediaType = Map.copyOf(formatToMediaType);
+        this.typeWithSubtypeToMediaType = Map.copyOf(typeWithSubtypeToMediaType);
+        this.parametersMap = Map.copyOf(parametersMap);
     }
 
     public T fromMediaType(String mediaType) {
@@ -65,8 +50,10 @@ public T fromFormat(String format) {
 
     /**
      * parsing media type that follows https://tools.ietf.org/html/rfc7231#section-3.1.1.1
+     *
      * @param headerValue a header value from Accept or Content-Type
      * @return a parsed media-type
+     * //todo pg should this all be maybe based on a regex?
      */
     public ParsedMediaType parseMediaType(String headerValue) {
         if (headerValue != null) {
@@ -75,25 +62,44 @@ public ParsedMediaType parseMediaType(String headerValue) {
             String[] typeSubtype = split[0].trim().toLowerCase(Locale.ROOT)
                 .split("/");
             if (typeSubtype.length == 2) {
+
                 String type = typeSubtype[0];
                 String subtype = typeSubtype[1];
-                T xContentType = typeWithSubtypeToMediaType.get(type + "/" + subtype);
+                String typeWithSubtype = type + "/" + subtype;
+                T xContentType = typeWithSubtypeToMediaType.get(typeWithSubtype);
                 if (xContentType != null) {
                     Map<String, String> parameters = new HashMap<>();
                     for (int i = 1; i < split.length; i++) {
                         //spaces are allowed between parameters, but not between '=' sign
                         String[] keyValueParam = split[i].trim().split("=");
                         if (keyValueParam.length != 2 || hasSpaces(keyValueParam[0]) || hasSpaces(keyValueParam[1])) {
-                            return null;
+                            throw new IllegalArgumentException("Spaces are not allowed between parameters. " + headerValue);
+                        }
+                        String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT);
+                        String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT);
+                        if (isValidParameter(typeWithSubtype, parameterName, parameterValue) == false) {
+                            throw new IllegalArgumentException("unrecognized parameter "
+                                + parameterName + " " + parameterValue + " headerValue=" + headerValue);
                         }
-                        parameters.put(keyValueParam[0].toLowerCase(Locale.ROOT), keyValueParam[1].toLowerCase(Locale.ROOT));
+                        parameters.put(parameterName, parameterValue);
                     }
                     return new ParsedMediaType(xContentType, parameters);
                 }
             }
 
         }
-        return null;
+        throw new IllegalArgumentException("unrecognized media type: " + headerValue);
+    }
+
+    private boolean isValidParameter(String typeWithSubtype, String parameterName, String parameterValue) {
+        if (parametersMap.containsKey(typeWithSubtype)) {
+            Map<String, Pattern> parameters = parametersMap.get(typeWithSubtype);
+            if (parameters.containsKey(parameterName)) {
+                Pattern regex = parameters.get(parameterName);
+                return regex.matcher(parameterValue).matches();
+            }
+        }
+        return false;
     }
 
     private boolean hasSpaces(String s) {
@@ -120,4 +126,37 @@ public Map<String, String> getParameters() {
             return parameters;
         }
     }
+
+    public static class Builder<T extends MediaType> {
+        private final Map<String, T> formatToMediaType = new HashMap<>();
+        private final Map<String, T> typeWithSubtypeToMediaType = new HashMap<>();
+        private final Map<String, Map<String, Pattern>> parametersMap = new HashMap<>();
+
+        public Builder<T> withMediaTypeAndParams(String alternativeMediaType, T mediaType, Map<String, Pattern> paramNameAndValueRegex) {
+            typeWithSubtypeToMediaType.put(alternativeMediaType.toLowerCase(Locale.ROOT), mediaType);
+            formatToMediaType.put(mediaType.format(), mediaType);
+
+            Map<String, Pattern> parametersForMediaType = new HashMap<>(paramNameAndValueRegex.size());
+            for (Map.Entry<String, Pattern> params : paramNameAndValueRegex.entrySet()) {
+                String parameterName = params.getKey().toLowerCase(Locale.ROOT);
+                Pattern parameterRegex = params.getValue();
+                Pattern pattern = Pattern.compile(parameterRegex.pattern(), Pattern.CASE_INSENSITIVE);
+                parametersForMediaType.put(parameterName, pattern);
+            }
+            parametersMap.put(alternativeMediaType, parametersForMediaType);
+
+            return this;
+        }
+
+        public Builder<T> copyFromMediaTypeParser(MediaTypeParser<? extends T> mediaTypeParser) {
+            formatToMediaType.putAll(mediaTypeParser.formatToMediaType);
+            typeWithSubtypeToMediaType.putAll(mediaTypeParser.typeWithSubtypeToMediaType);
+            parametersMap.putAll(mediaTypeParser.parametersMap);
+            return this;
+        }
+
+        public MediaTypeParser<T> build() {
+            return new MediaTypeParser<>(formatToMediaType, typeWithSubtypeToMediaType, parametersMap);
+        }
+    }
 }

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java

@@ -24,7 +24,9 @@
 import org.elasticsearch.common.xcontent.smile.SmileXContent;
 import org.elasticsearch.common.xcontent.yaml.YamlXContent;
 
+import java.util.Collections;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 /**
  * The content type of {@link org.elasticsearch.common.xcontent.XContent}.
@@ -113,9 +115,24 @@ public XContent xContent() {
         }
     };
 
-    public static final MediaTypeParser<XContentType> mediaTypeParser = new MediaTypeParser<>(XContentType.values(),
-        Map.of("application/*", JSON, "application/x-ndjson", JSON));
-
+     public static final MediaTypeParser<XContentType> mediaTypeParser = new MediaTypeParser.Builder<XContentType>()
+        .withMediaTypeAndParams("application/smile", SMILE, Collections.emptyMap())
+        .withMediaTypeAndParams("application/cbor", CBOR, Collections.emptyMap())
+        .withMediaTypeAndParams("application/json", JSON, Map.of("charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/yaml", YAML, Map.of("charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/*", JSON, Map.of("charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/x-ndjson", JSON, Map.of("charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/vnd.elasticsearch+json", JSON,
+            Map.of("compatible-with", Pattern.compile("\\d+"),"charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/vnd.elasticsearch+smile", SMILE,
+            Map.of("compatible-with", Pattern.compile("\\d+"),"charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/vnd.elasticsearch+yaml", YAML,
+            Map.of("compatible-with", Pattern.compile("\\d+"),"charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/vnd.elasticsearch+cbor", CBOR,
+            Map.of("compatible-with", Pattern.compile("\\d+"),"charset", Pattern.compile("UTF-8")))
+        .withMediaTypeAndParams("application/vnd.elasticsearch+x-ndjson", JSON,
+            Map.of("compatible-with", Pattern.compile("\\d+"),"charset", Pattern.compile("UTF-8")))
+        .build();
 
     /**
      * Accepts a format string, which is most of the time is equivalent to {@link XContentType#subtype()}
@@ -131,19 +148,29 @@ public static XContentType fromFormat(String mediaType) {
      * Attempts to match the given media type with the known {@link XContentType} values. This match is done in a case-insensitive manner.
      * The provided media type can optionally has parameters.
      * This method is suitable for parsing of the {@code Content-Type} and {@code Accept} HTTP headers.
-     * This method will return {@code null} if no match is found
+     * This method will throw IllegalArgumentException if no match is found
      */
     public static XContentType fromMediaType(String mediaTypeHeaderValue) {
         return mediaTypeParser.fromMediaType(mediaTypeHeaderValue);
     }
 
-
     private int index;
 
     XContentType(int index) {
         this.index = index;
     }
 
+    public static Byte parseVersion(String mediaType) {
+        MediaTypeParser<XContentType>.ParsedMediaType parsedMediaType = mediaTypeParser.parseMediaType(mediaType);
+        if (parsedMediaType != null) {
+            String version = parsedMediaType
+                .getParameters()
+                .get("compatible-with");
+            return version != null ? Byte.parseByte(version) : null;
+        }
+        return null;
+    }
+
     public int index() {
         return index;
     }

libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java

@@ -23,49 +23,50 @@
 
 import java.util.Collections;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.nullValue;
 
 public class MediaTypeParserTests extends ESTestCase {
-    MediaTypeParser<XContentType> mediaTypeParser = XContentType.mediaTypeParser;
+
+    MediaTypeParser<XContentType> mediaTypeParser = new MediaTypeParser.Builder<XContentType>()
+        .withMediaTypeAndParams("application/vnd.elasticsearch+json",
+            XContentType.JSON, Map.of("compatible-with", Pattern.compile("\\d+"),
+            "charset", Pattern.compile("UTF-8")))
+        .build();
 
     public void testJsonWithParameters() throws Exception {
-        String mediaType = "application/json";
+        String mediaType = "application/vnd.elasticsearch+json";
         assertThat(mediaTypeParser.parseMediaType(mediaType).getParameters(),
             equalTo(Collections.emptyMap()));
         assertThat(mediaTypeParser.parseMediaType(mediaType + ";").getParameters(),
             equalTo(Collections.emptyMap()));
         assertThat(mediaTypeParser.parseMediaType(mediaType + "; charset=UTF-8").getParameters(),
             equalTo(Map.of("charset", "utf-8")));
-        assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123;charset=UTF-8").getParameters(),
-            equalTo(Map.of("charset", "utf-8", "custom", "123")));
+        assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123;charset=UTF-8").getParameters(),
+            equalTo(Map.of("charset", "utf-8", "compatible-with", "123")));
     }
 
     public void testWhiteSpaceInTypeSubtype() {
-        String mediaType = " application/json ";
+        String mediaType = " application/vnd.elasticsearch+json ";
         assertThat(mediaTypeParser.parseMediaType(mediaType).getMediaType(),
             equalTo(XContentType.JSON));
 
-        assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123; charset=UTF-8").getParameters(),
-            equalTo(Map.of("charset", "utf-8", "custom", "123")));
-        assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123;\n charset=UTF-8").getParameters(),
-            equalTo(Map.of("charset", "utf-8", "custom", "123")));
+        assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123; charset=UTF-8").getParameters(),
+            equalTo(Map.of("charset", "utf-8", "compatible-with", "123")));
+        assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123;\n charset=UTF-8").getParameters(),
+            equalTo(Map.of("charset", "utf-8", "compatible-with", "123")));
 
-        mediaType = " application / json ";
-        assertThat(mediaTypeParser.parseMediaType(mediaType),
-            is(nullValue()));
+       String  mediaTypeWithWhitespace = " application / json ";
+        expectThrows(IllegalArgumentException.class , () -> mediaTypeParser.parseMediaType(mediaTypeWithWhitespace));
     }
 
     public void testInvalidParameters() {
-        String mediaType = "application/json";
-        assertThat(mediaTypeParser.parseMediaType(mediaType + "; keyvalueNoEqualsSign"),
-            is(nullValue()));
+        String mediaType = "application/vnd.elasticsearch+json";
+        expectThrows(IllegalArgumentException.class , () -> mediaTypeParser.parseMediaType(mediaType + "; keyvalueNoEqualsSign"));
+
+        expectThrows(IllegalArgumentException.class , () -> mediaTypeParser.parseMediaType(mediaType + "; key = value"));
 
-        assertThat(mediaTypeParser.parseMediaType(mediaType + "; key = value"),
-            is(nullValue()));
-        assertThat(mediaTypeParser.parseMediaType(mediaType + "; key=") ,
-            is(nullValue()));
+        expectThrows(IllegalArgumentException.class , () -> mediaTypeParser.parseMediaType(mediaType + "; key="));
     }
 }

server/src/main/java/org/elasticsearch/ElasticsearchException.java

@@ -631,7 +631,7 @@ public static ElasticsearchException[] guessRootCauses(Throwable t) {
              * parsing exception because that is generally the most interesting
              * exception to return to the user. If that exception is caused by
              * an ElasticsearchException we'd like to keep unwrapping because
-             * ElasticserachExceptions tend to contain useful information for
+             * ElasticsearchExceptions tend to contain useful information for
              * the user.
              */
             Throwable cause = ex.getCause();

server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java

@@ -18,6 +18,8 @@
  */
 package org.elasticsearch.rest;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.Streams;
@@ -36,6 +38,7 @@
 
 public abstract class AbstractRestChannel implements RestChannel {
 
+    private static final Logger logger = LogManager.getLogger(AbstractRestChannel.class);
     private static final Predicate<String> INCLUDE_FILTER = f -> f.charAt(0) != '-';
     private static final Predicate<String> EXCLUDE_FILTER = INCLUDE_FILTER.negate();
 
@@ -98,10 +101,17 @@ public XContentBuilder newBuilder(@Nullable XContentType requestContentType, boo
     public XContentBuilder newBuilder(@Nullable XContentType requestContentType, @Nullable XContentType responseContentType,
             boolean useFiltering) throws IOException {
         if (responseContentType == null) {
-            //TODO PG shoudld format vs acceptHeader be always the same, do we allow overriding?
-            responseContentType = XContentType.fromFormat(format);
-            if (responseContentType == null) {
-                responseContentType = XContentType.fromMediaType(acceptHeader);
+            if (Strings.hasText(format)) {
+                responseContentType = XContentType.fromFormat(format);
+            } else if (Strings.hasText(acceptHeader) && acceptHeader.equals("*/*") == false
+                && acceptHeader.startsWith("text/plain")==false) { //TODO PG there are a lot of usages of text/plain as a response type.
+                // I feel it would fit into xcontenttype..
+                try {
+                    responseContentType = XContentType.fromMediaType(acceptHeader);
+                }catch (IllegalArgumentException e){
+                    //todo pg this is in a way controlling the flow by exceptions. to be discussed
+                    logger.debug("Unrecognized accept header",e);
+                }
             }
         }
         // try to determine the response content type from the media type or the format query string parameter, with the format parameter

server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java

@@ -83,6 +83,7 @@ public BytesRestResponse(RestStatus status, String contentType, byte[] content)
     /**
      * Creates a binary response.
      */
+    //todo pg this should probably be acceptType? (2nd param)
     public BytesRestResponse(RestStatus status, String contentType, BytesReference content) {
         this.status = status;
         this.content = content;