diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java index ce310a5b3f8e4..62a3f3fd915d0 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java +++ b/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 { private final Map formatToMediaType; private final Map typeWithSubtypeToMediaType; + private final Map> parametersMap; - public MediaTypeParser(T[] acceptedMediaTypes) { - this(acceptedMediaTypes, Map.of()); - } - - public MediaTypeParser(T[] acceptedMediaTypes, Map additionalMediaTypes) { - final int size = acceptedMediaTypes.length + additionalMediaTypes.size(); - Map formatMap = new HashMap<>(size); - Map typeMap = new HashMap<>(size); - for (T mediaType : acceptedMediaTypes) { - typeMap.put(mediaType.typeWithSubtype(), mediaType); - formatMap.put(mediaType.format(), mediaType); - } - for (Map.Entry 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 formatToMediaType, Map typeWithSubtypeToMediaType, + Map> parametersMap) { + this.formatToMediaType = Map.copyOf(formatToMediaType); + this.typeWithSubtypeToMediaType = Map.copyOf(typeWithSubtypeToMediaType); + this.parametersMap = Map.copyOf(parametersMap); } public T fromMediaType(String mediaType) { @@ -65,6 +50,7 @@ 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 */ @@ -75,9 +61,11 @@ 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 parameters = new HashMap<>(); for (int i = 1; i < split.length; i++) { @@ -86,7 +74,12 @@ public ParsedMediaType parseMediaType(String headerValue) { if (keyValueParam.length != 2 || hasSpaces(keyValueParam[0]) || hasSpaces(keyValueParam[1])) { return null; } - parameters.put(keyValueParam[0].toLowerCase(Locale.ROOT), keyValueParam[1].toLowerCase(Locale.ROOT)); + String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT); + String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT); + if (isValidParameter(typeWithSubtype, parameterName, parameterValue) == false) { + return null; + } + parameters.put(parameterName, parameterValue); } return new ParsedMediaType(xContentType, parameters); } @@ -96,6 +89,17 @@ public ParsedMediaType parseMediaType(String headerValue) { return null; } + private boolean isValidParameter(String typeWithSubtype, String parameterName, String parameterValue) { + if (parametersMap.containsKey(typeWithSubtype)) { + Map 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) { return s.trim().equals(s) == false; } @@ -120,4 +124,37 @@ public Map getParameters() { return parameters; } } + + public static class Builder { + private final Map formatToMediaType = new HashMap<>(); + private final Map typeWithSubtypeToMediaType = new HashMap<>(); + private final Map> parametersMap = new HashMap<>(); + + public Builder withMediaTypeAndParams(String alternativeMediaType, T mediaType, Map paramNameAndValueRegex) { + typeWithSubtypeToMediaType.put(alternativeMediaType.toLowerCase(Locale.ROOT), mediaType); + formatToMediaType.put(mediaType.format(), mediaType); + + Map parametersForMediaType = new HashMap<>(paramNameAndValueRegex.size()); + for (Map.Entry params : paramNameAndValueRegex.entrySet()) { + String parameterName = params.getKey().toLowerCase(Locale.ROOT); + String parameterRegex = params.getValue(); + Pattern pattern = Pattern.compile(parameterRegex, Pattern.CASE_INSENSITIVE); + parametersForMediaType.put(parameterName, pattern); + } + parametersMap.put(alternativeMediaType, parametersForMediaType); + + return this; + } + + public Builder copyFromMediaTypeParser(MediaTypeParser mediaTypeParser) { + formatToMediaType.putAll(mediaTypeParser.formatToMediaType); + typeWithSubtypeToMediaType.putAll(mediaTypeParser.typeWithSubtypeToMediaType); + parametersMap.putAll(mediaTypeParser.parametersMap); + return this; + } + + public MediaTypeParser build() { + return new MediaTypeParser<>(formatToMediaType, typeWithSubtypeToMediaType, parametersMap); + } + } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java index 5801a3cd76def..076a20bad006a 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.xcontent.smile.SmileXContent; import org.elasticsearch.common.xcontent.yaml.YamlXContent; +import java.util.Collections; import java.util.Map; /** @@ -113,9 +114,26 @@ public XContent xContent() { } }; - public static final MediaTypeParser mediaTypeParser = new MediaTypeParser<>(XContentType.values(), - Map.of("application/*", JSON, "application/x-ndjson", JSON)); - + private static final String COMPATIBLE_WITH_PARAMETER_NAME = "compatible-with"; + private static final String VERSION_PATTERN = "\\d+"; + public static final MediaTypeParser mediaTypeParser = new MediaTypeParser.Builder() + .withMediaTypeAndParams("application/smile", SMILE, Collections.emptyMap()) + .withMediaTypeAndParams("application/cbor", CBOR, Collections.emptyMap()) + .withMediaTypeAndParams("application/json", JSON, Map.of("charset", "UTF-8")) + .withMediaTypeAndParams("application/yaml", YAML, Map.of("charset", "UTF-8")) + .withMediaTypeAndParams("application/*", JSON, Map.of("charset", "UTF-8")) + .withMediaTypeAndParams("application/x-ndjson", JSON, Map.of("charset", "UTF-8")) + .withMediaTypeAndParams("application/vnd.elasticsearch+json", JSON, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .withMediaTypeAndParams("application/vnd.elasticsearch+smile", SMILE, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .withMediaTypeAndParams("application/vnd.elasticsearch+yaml", YAML, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .withMediaTypeAndParams("application/vnd.elasticsearch+cbor", CBOR, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .withMediaTypeAndParams("application/vnd.elasticsearch+x-ndjson", JSON, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .build(); /** * Accepts a format string, which is most of the time is equivalent to {@link XContentType#subtype()} @@ -137,13 +155,23 @@ 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.ParsedMediaType parsedMediaType = mediaTypeParser.parseMediaType(mediaType); + if (parsedMediaType != null) { + String version = parsedMediaType + .getParameters() + .get(COMPATIBLE_WITH_PARAMETER_NAME); + return version != null ? Byte.parseByte(version) : null; + } + return null; + } + public int index() { return index; } diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java index 6514c7ec73306..08ca08a3d2240 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java @@ -29,29 +29,34 @@ import static org.hamcrest.Matchers.nullValue; public class MediaTypeParserTests extends ESTestCase { - MediaTypeParser mediaTypeParser = XContentType.mediaTypeParser; + + MediaTypeParser mediaTypeParser = new MediaTypeParser.Builder() + .withMediaTypeAndParams("application/vnd.elasticsearch+json", + XContentType.JSON, Map.of("compatible-with", "\\d+", + "charset", "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), @@ -59,10 +64,11 @@ public void testWhiteSpaceInTypeSubtype() { } public void testInvalidParameters() { - String mediaType = "application/json"; + String mediaType = "application/vnd.elasticsearch+json"; + assertThat(mediaTypeParser.parseMediaType(mediaType + "; charset=unknown") , + is(nullValue())); assertThat(mediaTypeParser.parseMediaType(mediaType + "; keyvalueNoEqualsSign"), is(nullValue())); - assertThat(mediaTypeParser.parseMediaType(mediaType + "; key = value"), is(nullValue())); assertThat(mediaTypeParser.parseMediaType(mediaType + "; key=") , diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index 26793eaf2dcdd..cd3330a1e18b3 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/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(); diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index c5bf1efa2cdb1..6f5aa618ae4d0 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/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 INCLUDE_FILTER = f -> f.charAt(0) != '-'; private static final Predicate EXCLUDE_FILTER = INCLUDE_FILTER.negate(); @@ -98,8 +101,9 @@ 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 (Strings.hasText(format)) { + responseContentType = XContentType.fromFormat(format); + } if (responseContentType == null) { responseContentType = XContentType.fromMediaType(acceptHeader); } diff --git a/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java b/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java index 3babe43474266..859eddc9a613a 100644 --- a/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java +++ b/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java @@ -50,7 +50,7 @@ public class BytesRestResponse extends RestResponse { private final RestStatus status; private final BytesReference content; - private final String contentType; + private final String responseMediaType; /** * Creates a new response based on {@link XContentBuilder}. @@ -69,24 +69,24 @@ public BytesRestResponse(RestStatus status, String content) { /** * Creates a new plain text response. */ - public BytesRestResponse(RestStatus status, String contentType, String content) { - this(status, contentType, new BytesArray(content)); + public BytesRestResponse(RestStatus status, String responseMediaType, String content) { + this(status, responseMediaType, new BytesArray(content)); } /** * Creates a binary response. */ - public BytesRestResponse(RestStatus status, String contentType, byte[] content) { - this(status, contentType, new BytesArray(content)); + public BytesRestResponse(RestStatus status, String responseMediaType, byte[] content) { + this(status, responseMediaType, new BytesArray(content)); } /** * Creates a binary response. */ - public BytesRestResponse(RestStatus status, String contentType, BytesReference content) { + public BytesRestResponse(RestStatus status, String responseMediaType, BytesReference content) { this.status = status; this.content = content; - this.contentType = contentType; + this.responseMediaType = responseMediaType; } public BytesRestResponse(RestChannel channel, Exception e) throws IOException { @@ -109,7 +109,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th try (XContentBuilder builder = channel.newErrorBuilder()) { build(builder, params, status, channel.detailedErrorsEnabled(), e); this.content = BytesReference.bytes(builder); - this.contentType = builder.contentType().mediaType(); + this.responseMediaType = builder.contentType().mediaType(); } if (e instanceof ElasticsearchException) { copyHeaders(((ElasticsearchException) e)); @@ -118,7 +118,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th @Override public String contentType() { - return this.contentType; + return this.responseMediaType; } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java index fa4bd586a4723..bbb746baafe92 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java @@ -51,14 +51,14 @@ public class RestTable { public static RestResponse buildResponse(Table table, RestChannel channel) throws Exception { RestRequest request = channel.request(); - XContentType xContentType = getxContentType(request); + XContentType xContentType = getXContentType(request); if (xContentType != null) { return buildXContentBuilder(table, channel); } return buildTextPlainResponse(table, channel); } - private static XContentType getxContentType(RestRequest request) { + private static XContentType getXContentType(RestRequest request) { if (request.hasParam("format")) { return XContentType.fromFormat(request.param("format")); } diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java index d6f9fb5e9ad5e..fa937f84d615c 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java @@ -23,6 +23,7 @@ import java.util.Locale; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; public class XContentTypeTests extends ESTestCase { @@ -33,6 +34,7 @@ public void testFromJson() throws Exception { assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType)); assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType)); assertThat(XContentType.fromMediaType(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType)); + assertThat(XContentType.fromMediaType(mediaType + "; charset=utf-8"), equalTo(expectedXContentType)); } public void testFromNdJson() throws Exception { @@ -93,4 +95,65 @@ public void testFromRubbish() throws Exception { assertThat(XContentType.fromMediaType("text/plain"), nullValue()); assertThat(XContentType.fromMediaType("gobbly;goop"), nullValue()); } + + public void testVersionedMediaType() { + String version = String.valueOf(Math.abs(randomByte())); + assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+json;compatible-with=" + version), + equalTo(XContentType.JSON)); + assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+cbor;compatible-with=" + version), + equalTo(XContentType.CBOR)); + assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+smile;compatible-with=" + version), + equalTo(XContentType.SMILE)); + assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+yaml;compatible-with=" + version), + equalTo(XContentType.YAML)); + assertThat(XContentType.fromMediaType("application/json"), + equalTo(XContentType.JSON)); + assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+x-ndjson;compatible-with=" + version), + equalTo(XContentType.JSON)); + + + assertThat(XContentType.fromMediaType("APPLICATION/VND.ELASTICSEARCH+JSON;COMPATIBLE-WITH=" + version), + equalTo(XContentType.JSON)); + assertThat(XContentType.fromMediaType("APPLICATION/JSON"), + equalTo(XContentType.JSON)); + } + + public void testVersionParsing() { + byte version = (byte) Math.abs(randomByte()); + assertThat(XContentType.parseVersion("application/vnd.elasticsearch+json;compatible-with=" + version), + equalTo(version)); + assertThat(XContentType.parseVersion("application/vnd.elasticsearch+cbor;compatible-with=" + version), + equalTo(version)); + assertThat(XContentType.parseVersion("application/vnd.elasticsearch+smile;compatible-with=" + version), + equalTo(version)); + assertThat(XContentType.parseVersion("application/vnd.elasticsearch+x-ndjson;compatible-with=" + version), + equalTo(version)); + assertThat(XContentType.parseVersion("application/json"), + nullValue()); + + + assertThat(XContentType.parseVersion("APPLICATION/VND.ELASTICSEARCH+JSON;COMPATIBLE-WITH=" + version), + equalTo(version)); + assertThat(XContentType.parseVersion("APPLICATION/JSON"), + nullValue()); + + assertThat(XContentType.parseVersion("application/json;compatible-with=" + version + ".0"), + is(nullValue())); + } + + public void testUnrecognizedParameter() { + assertThat(XContentType.parseVersion("application/json; sth=123"), + is(nullValue())); } + + public void testMediaTypeWithoutESSubtype() { + String version = String.valueOf(Math.abs(randomByte())); + assertThat(XContentType.fromMediaType("application/json;compatible-with=" + version), nullValue()); + } + + public void testAnchoring() { + String version = String.valueOf(Math.abs(randomByte())); + assertThat(XContentType.fromMediaType("sth_application/json;compatible-with=" + version + ".0"), nullValue()); + assertThat(XContentType.fromMediaType("sth_application/json;compatible-with=" + version + "_sth"), nullValue()); + assertThat(XContentType.fromMediaType("application/json;compatible-with=" + version + "_sth"), nullValue()); + } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java index 4ade958b60e85..763e460170cf0 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.plugin; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -19,7 +20,6 @@ import org.elasticsearch.xpack.sql.action.SqlQueryAction; import org.elasticsearch.xpack.sql.action.SqlQueryRequest; import org.elasticsearch.xpack.sql.action.SqlQueryResponse; -import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; import java.io.IOException; @@ -31,11 +31,11 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_DELIMITER; -import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_FORMAT; public class RestSqlQueryAction extends BaseRestHandler { - TextFormat textFormat; + private final SqlMediaTypeParser sqlMediaTypeParser = new SqlMediaTypeParser(); + MediaType responseMediaType; @Override public List routes() { @@ -52,53 +52,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli sqlRequest = SqlQueryRequest.fromXContent(parser); } - /* - * Since we support {@link TextFormat} and - * {@link XContent} outputs we can't use {@link RestToXContentListener} - * like everything else. We want to stick as closely as possible to - * Elasticsearch's defaults though, while still layering in ways to - * control the output more easily. - * - * First we find the string that the user used to specify the response - * format. If there is a {@code format} parameter we use that. If there - * isn't but there is a {@code Accept} header then we use that. If there - * isn't then we use the {@code Content-Type} header which is required. - */ - String accept; - - if (Mode.isDedicatedClient(sqlRequest.requestInfo().mode()) - && (sqlRequest.binaryCommunication() == null || sqlRequest.binaryCommunication())) { - // enforce CBOR response for drivers and CLI (unless instructed differently through the config param) - accept = XContentType.CBOR.name(); - } else { - accept = request.param(URL_PARAM_FORMAT); - } - if (accept == null) { - accept = request.header("Accept"); - if ("*/*".equals(accept)) { - // */* means "I don't care" which we should treat like not specifying the header - accept = null; - } - } - if (accept == null) { - accept = request.header("Content-Type"); - } - assert accept != null : "The Content-Type header is required"; - - /* - * Second, we pick the actual content type to use by first parsing the - * string from the previous step as an {@linkplain XContent} value. If - * that doesn't parse we parse it as a {@linkplain TextFormat} value. If - * that doesn't parse it'll throw an {@link IllegalArgumentException} - * which we turn into a 400 error. - */ - XContentType xContentType = getXContentType(accept); - textFormat = xContentType == null ? TextFormat.fromMediaTypeOrFormat(accept) : null; - - if (xContentType == null && sqlRequest.columnar()) { - throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with " - + "txt, csv or tsv formats"); - } + responseMediaType = sqlMediaTypeParser.getMediaType(request, sqlRequest); long startNanos = System.nanoTime(); return channel -> client.execute(SqlQueryAction.INSTANCE, sqlRequest, new RestResponseListener(channel) { @@ -107,16 +61,16 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception { RestResponse restResponse; // XContent branch - if (xContentType != null) { - XContentBuilder builder = channel.newBuilder(request.getXContentType(), xContentType, true); + if (responseMediaType != null && responseMediaType instanceof XContentType) { + XContentType type = (XContentType) responseMediaType; + XContentBuilder builder = channel.newBuilder(request.getXContentType(), type, true); response.toXContent(builder, request); restResponse = new BytesRestResponse(RestStatus.OK, builder); - } - // TextFormat - else { - final String data = textFormat.format(request, response); + } else { // TextFormat + TextFormat type = (TextFormat)responseMediaType; + final String data = type.format(request, response); - restResponse = new BytesRestResponse(RestStatus.OK, textFormat.contentType(request), + restResponse = new BytesRestResponse(RestStatus.OK, type.contentType(request), data.getBytes(StandardCharsets.UTF_8)); if (response.hasCursor()) { @@ -130,17 +84,12 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception { }); } - private XContentType getXContentType(String accept) { - if (accept == null) { - return XContentType.JSON; - } - XContentType xContentType = XContentType.fromFormat(accept); - return xContentType != null ? xContentType : XContentType.fromMediaType(accept); - } + + @Override protected Set responseParams() { - return textFormat == TextFormat.CSV ? Collections.singleton(URL_PARAM_DELIMITER) : Collections.emptySet(); + return responseMediaType == TextFormat.CSV ? Collections.singleton(URL_PARAM_DELIMITER) : Collections.emptySet(); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java new file mode 100644 index 0000000000000..189dc137b654c --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.plugin; + +import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.MediaTypeParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.xpack.sql.action.SqlQueryRequest; +import org.elasticsearch.xpack.sql.proto.Mode; + +import java.util.Map; + +import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_FORMAT; + +public class SqlMediaTypeParser { + private static final MediaTypeParser parser = new MediaTypeParser.Builder<>() + .copyFromMediaTypeParser(XContentType.mediaTypeParser) + .withMediaTypeAndParams(TextFormat.PLAIN_TEXT.typeWithSubtype(), TextFormat.PLAIN_TEXT, + Map.of("header", "present|absent", "charset", "utf-8")) + .withMediaTypeAndParams(TextFormat.CSV.typeWithSubtype(), TextFormat.CSV, + Map.of("header", "present|absent", "charset", "utf-8", + "delimiter", ".+"))// more detailed parsing is in TextFormat.CSV#delimiter + .withMediaTypeAndParams(TextFormat.TSV.typeWithSubtype(), TextFormat.TSV, + Map.of("header", "present|absent", "charset", "utf-8")) + .build(); + + /* + * Since we support {@link TextFormat} and + * {@link XContent} outputs we can't use {@link RestToXContentListener} + * like everything else. We want to stick as closely as possible to + * Elasticsearch's defaults though, while still layering in ways to + * control the output more easily. + * + * First we find the string that the user used to specify the response + * format. If there is a {@code format} parameter we use that. If there + * isn't but there is a {@code Accept} header then we use that. If there + * isn't then we use the {@code Content-Type} header which is required. + */ + public MediaType getMediaType(RestRequest request, SqlQueryRequest sqlRequest) { + + if (Mode.isDedicatedClient(sqlRequest.requestInfo().mode()) + && (sqlRequest.binaryCommunication() == null || sqlRequest.binaryCommunication())) { + // enforce CBOR response for drivers and CLI (unless instructed differently through the config param) + return XContentType.CBOR; + } else if (request.hasParam(URL_PARAM_FORMAT)) { + return validateColumnarRequest(sqlRequest.columnar(), parser.fromFormat(request.param(URL_PARAM_FORMAT))); + } + if (request.getHeaders().containsKey("Accept")) { + String accept = request.header("Accept"); + // */* means "I don't care" which we should treat like not specifying the header + if ("*/*".equals(accept) == false) { + return validateColumnarRequest(sqlRequest.columnar(), parser.fromMediaType(accept)); + } + } + + String contentType = request.header("Content-Type"); + assert contentType != null : "The Content-Type header is required"; + return validateColumnarRequest(sqlRequest.columnar(), parser.fromMediaType(contentType)); + } + + private static MediaType validateColumnarRequest(boolean requestIsColumnar, MediaType fromMediaType) { + if(requestIsColumnar && fromMediaType instanceof TextFormat){ + throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with " + + "txt, csv or tsv formats"); + } + return fromMediaType; + } + +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java index 1f50c5fa7ea28..d397d2316959c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java @@ -8,7 +8,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.MediaType; -import org.elasticsearch.common.xcontent.MediaTypeParser; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.xpack.ql.util.StringUtils; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; @@ -294,8 +293,6 @@ public String subtype() { private static final String PARAM_HEADER_ABSENT = "absent"; private static final String PARAM_HEADER_PRESENT = "present"; - private static final MediaTypeParser parser = new MediaTypeParser<>(TextFormat.values()); - String format(RestRequest request, SqlQueryResponse response) { StringBuilder sb = new StringBuilder(); @@ -316,18 +313,6 @@ boolean hasHeader(RestRequest request) { return true; } - static TextFormat fromMediaTypeOrFormat(String accept) { - TextFormat textFormat = parser.fromFormat(accept); - if (textFormat != null) { - return textFormat; - } - textFormat = parser.fromMediaType(accept); - if (textFormat != null) { - return textFormat; - } - throw new IllegalArgumentException("invalid format [" + accept + "]"); - } - /** * Formal IANA mime type. */ diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParserTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParserTests.java new file mode 100644 index 0000000000000..0459b777b15f9 --- /dev/null +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParserTests.java @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.plugin; + +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.xpack.sql.action.SqlQueryRequest; +import org.elasticsearch.xpack.sql.proto.Mode; +import org.elasticsearch.xpack.sql.proto.RequestInfo; + +import java.util.Collections; +import java.util.Map; + +import static org.elasticsearch.xpack.sql.plugin.TextFormat.CSV; +import static org.elasticsearch.xpack.sql.plugin.TextFormat.PLAIN_TEXT; +import static org.elasticsearch.xpack.sql.plugin.TextFormat.TSV; +import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLIENT_IDS; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; + +public class SqlMediaTypeParserTests extends ESTestCase { + SqlMediaTypeParser parser = new SqlMediaTypeParser(); + + public void testPlainTextDetection() { + MediaType text = parser.getMediaType(reqWithAccept("text/plain"), createTestInstance(false, Mode.PLAIN, false)); + assertThat(text, is(PLAIN_TEXT)); + } + + public void testCsvDetection() { + MediaType text = parser.getMediaType(reqWithAccept("text/csv"), createTestInstance(false, Mode.PLAIN, false)); + assertThat(text, is(CSV)); + + text = parser.getMediaType(reqWithAccept("text/csv; delimiter=x"), createTestInstance(false, Mode.PLAIN, false)); + assertThat(text, is(CSV)); + } + + public void testTsvDetection() { + MediaType text = parser.getMediaType(reqWithAccept("text/tab-separated-values"), createTestInstance(false, Mode.PLAIN, false)); + assertThat(text, is(TSV)); + } + + public void testMediaTypeDetectionWithParameters() { + assertThat(parser.getMediaType(reqWithAccept("text/plain; charset=utf-8"), + createTestInstance(false, Mode.PLAIN, false)), is(PLAIN_TEXT)); + assertThat(parser.getMediaType(reqWithAccept("text/plain; header=present"), + createTestInstance(false, Mode.PLAIN, false)), is(PLAIN_TEXT)); + assertThat(parser.getMediaType(reqWithAccept("text/plain; charset=utf-8; header=present"), + createTestInstance(false, Mode.PLAIN, false)), is(PLAIN_TEXT)); + + assertThat(parser.getMediaType(reqWithAccept("text/csv; charset=utf-8"), + createTestInstance(false, Mode.PLAIN, false)), is(CSV)); + assertThat(parser.getMediaType(reqWithAccept("text/csv; header=present"), + createTestInstance(false, Mode.PLAIN, false)), is(CSV)); + assertThat(parser.getMediaType(reqWithAccept("text/csv; charset=utf-8; header=present"), + createTestInstance(false, Mode.PLAIN, false)), is(CSV)); + + assertThat(parser.getMediaType(reqWithAccept("text/tab-separated-values; charset=utf-8"), + createTestInstance(false, Mode.PLAIN, false)), is(TSV)); + assertThat(parser.getMediaType(reqWithAccept("text/tab-separated-values; header=present"), + createTestInstance(false, Mode.PLAIN, false)), is(TSV)); + assertThat(parser.getMediaType(reqWithAccept("text/tab-separated-values; charset=utf-8; header=present"), + createTestInstance(false, Mode.PLAIN, false)), is(TSV)); + } + + public void testInvalidFormat() { + MediaType mediaType = parser.getMediaType(reqWithAccept("text/garbage"), createTestInstance(false, Mode.PLAIN, false)); + assertThat(mediaType, is(nullValue())); + } + + private static RestRequest reqWithAccept(String acceptHeader) { + + return new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withHeaders(Map.of("Content-Type", Collections.singletonList("application/json"), + "Accept", Collections.singletonList(acceptHeader))) + .build(); + } + + protected SqlQueryRequest createTestInstance(boolean binaryCommunication, Mode mode, boolean columnar) { + return new SqlQueryRequest(randomAlphaOfLength(10), Collections.emptyList(), null, + randomZone(), between(1, Integer.MAX_VALUE), TimeValue.parseTimeValue(randomTimeValue(), null, "test"), + TimeValue.parseTimeValue(randomTimeValue(), null, "test"), columnar, randomAlphaOfLength(10), + new RequestInfo(mode, randomFrom(randomFrom(CLIENT_IDS), randomAlphaOfLengthBetween(10, 20))), + randomBoolean(), randomBoolean()).binaryCommunication(binaryCommunication); + } +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java index a53fd8f94ad59..09d916af5cbc2 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java @@ -26,30 +26,9 @@ import static java.util.Collections.singletonMap; import static org.elasticsearch.xpack.sql.plugin.TextFormat.CSV; import static org.elasticsearch.xpack.sql.plugin.TextFormat.TSV; -import static org.hamcrest.CoreMatchers.is; public class TextFormatTests extends ESTestCase { - public void testPlainTextDetection() { - TextFormat text = TextFormat.fromMediaTypeOrFormat("text/plain"); - assertThat(text, is(TextFormat.PLAIN_TEXT)); - } - - public void testCsvDetection() { - TextFormat text = TextFormat.fromMediaTypeOrFormat("text/csv"); - assertThat(text, is(CSV)); - } - - public void testTsvDetection() { - TextFormat text = TextFormat.fromMediaTypeOrFormat("text/tab-separated-values"); - assertThat(text, is(TSV)); - } - - public void testInvalidFormat() { - Exception e = expectThrows(IllegalArgumentException.class, () -> TextFormat.fromMediaTypeOrFormat("text/garbage")); - assertEquals("invalid format [text/garbage]", e.getMessage()); - } - public void testCsvContentType() { assertEquals("text/csv; charset=utf-8; header=present", CSV.contentType(req())); }