Skip to content

Commit

Permalink
Allow parsing Content-Type and Accept headers with version (#61427)
Browse files Browse the repository at this point in the history 
Content-Type and Accept headers can be populated with a versioned form of media types
like application/vnd.elasticsearch+json;compatible-with=7
when previously it was simple application/json or (cbor, yaml..) - this is still supported.
Extending MediaTypeParser to validate the parameters.

relates  #51816
  • Loading branch information
@pgomulka
pgomulka authored Oct 5, 2020
1 parent 1eac692 commit d04edcd
Show file tree
Hide file tree
Showing 13 changed files with 372 additions and 154 deletions.
85 changes: 61 additions & 24 deletions libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
*/
Expand All @@ -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<String, String> parameters = new HashMap<>();
for (int i = 1; i < split.length; i++) {
Expand All @@ -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);
}
Expand All @@ -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<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) {
return s.trim().equals(s) == false;
}
Expand All @@ -120,4 +124,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, String> 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, String> 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<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);
}
}
}
36 changes: 32 additions & 4 deletions libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -113,9 +114,26 @@ public XContent xContent() {
}
};

public static final MediaTypeParser<XContentType> 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<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", "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()}
Expand All @@ -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<XContentType>.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;
}
Expand Down
28 changes: 17 additions & 11 deletions libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,40 +29,46 @@
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", "\\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),
is(nullValue()));
}

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=") ,
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/ElasticsearchException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 6 additions & 2 deletions server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

Expand Down Expand Up @@ -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);
}
Expand Down
18 changes: 9 additions & 9 deletions server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand All @@ -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 {
Expand All @@ -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));
Expand All @@ -118,7 +118,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th

@Override
public String contentType() {
return this.contentType;
return this.responseMediaType;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
Expand Down
Loading

0 comments on commit d04edcd

Please sign in to comment.