Skip to content

Commit 530cec4

Browse files
jakelandisjakelandis
committed
Revert "Allow parsing Content-Type and Accept headers with version (elastic#61427)"
This reverts commit d04edcd
1 parent 65ceee8 commit 530cec4

File tree

13 files changed

+154
-372
lines changed

13 files changed

+154
-372
lines changed

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

Lines changed: 24 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,33 @@
2222
import java.util.HashMap;
2323
import java.util.Locale;
2424
import java.util.Map;
25-
import java.util.regex.Pattern;
2625

2726
public class MediaTypeParser<T extends MediaType> {
2827
private final Map<String, T> formatToMediaType;
2928
private final Map<String, T> typeWithSubtypeToMediaType;
30-
private final Map<String, Map<String, Pattern>> parametersMap;
3129

32-
public MediaTypeParser(Map<String, T> formatToMediaType, Map<String, T> typeWithSubtypeToMediaType,
33-
Map<String, Map<String, Pattern>> parametersMap) {
34-
this.formatToMediaType = Map.copyOf(formatToMediaType);
35-
this.typeWithSubtypeToMediaType = Map.copyOf(typeWithSubtypeToMediaType);
36-
this.parametersMap = Map.copyOf(parametersMap);
30+
public MediaTypeParser(T[] acceptedMediaTypes) {
31+
this(acceptedMediaTypes, Map.of());
32+
}
33+
34+
public MediaTypeParser(T[] acceptedMediaTypes, Map<String, T> additionalMediaTypes) {
35+
final int size = acceptedMediaTypes.length + additionalMediaTypes.size();
36+
Map<String, T> formatMap = new HashMap<>(size);
37+
Map<String, T> typeMap = new HashMap<>(size);
38+
for (T mediaType : acceptedMediaTypes) {
39+
typeMap.put(mediaType.typeWithSubtype(), mediaType);
40+
formatMap.put(mediaType.format(), mediaType);
41+
}
42+
for (Map.Entry<String, T> entry : additionalMediaTypes.entrySet()) {
43+
String typeWithSubtype = entry.getKey();
44+
T mediaType = entry.getValue();
45+
46+
typeMap.put(typeWithSubtype.toLowerCase(Locale.ROOT), mediaType);
47+
formatMap.put(mediaType.format(), mediaType);
48+
}
49+
50+
this.formatToMediaType = Map.copyOf(formatMap);
51+
this.typeWithSubtypeToMediaType = Map.copyOf(typeMap);
3752
}
3853

3954
public T fromMediaType(String mediaType) {
@@ -50,7 +65,6 @@ public T fromFormat(String format) {
5065

5166
/**
5267
* parsing media type that follows https://tools.ietf.org/html/rfc7231#section-3.1.1.1
53-
*
5468
* @param headerValue a header value from Accept or Content-Type
5569
* @return a parsed media-type
5670
*/
@@ -61,11 +75,9 @@ public ParsedMediaType parseMediaType(String headerValue) {
6175
String[] typeSubtype = split[0].trim().toLowerCase(Locale.ROOT)
6276
.split("/");
6377
if (typeSubtype.length == 2) {
64-
6578
String type = typeSubtype[0];
6679
String subtype = typeSubtype[1];
67-
String typeWithSubtype = type + "/" + subtype;
68-
T xContentType = typeWithSubtypeToMediaType.get(typeWithSubtype);
80+
T xContentType = typeWithSubtypeToMediaType.get(type + "/" + subtype);
6981
if (xContentType != null) {
7082
Map<String, String> parameters = new HashMap<>();
7183
for (int i = 1; i < split.length; i++) {
@@ -74,12 +86,7 @@ public ParsedMediaType parseMediaType(String headerValue) {
7486
if (keyValueParam.length != 2 || hasSpaces(keyValueParam[0]) || hasSpaces(keyValueParam[1])) {
7587
return null;
7688
}
77-
String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT);
78-
String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT);
79-
if (isValidParameter(typeWithSubtype, parameterName, parameterValue) == false) {
80-
return null;
81-
}
82-
parameters.put(parameterName, parameterValue);
89+
parameters.put(keyValueParam[0].toLowerCase(Locale.ROOT), keyValueParam[1].toLowerCase(Locale.ROOT));
8390
}
8491
return new ParsedMediaType(xContentType, parameters);
8592
}
@@ -89,17 +96,6 @@ public ParsedMediaType parseMediaType(String headerValue) {
8996
return null;
9097
}
9198

92-
private boolean isValidParameter(String typeWithSubtype, String parameterName, String parameterValue) {
93-
if (parametersMap.containsKey(typeWithSubtype)) {
94-
Map<String, Pattern> parameters = parametersMap.get(typeWithSubtype);
95-
if (parameters.containsKey(parameterName)) {
96-
Pattern regex = parameters.get(parameterName);
97-
return regex.matcher(parameterValue).matches();
98-
}
99-
}
100-
return false;
101-
}
102-
10399
private boolean hasSpaces(String s) {
104100
return s.trim().equals(s) == false;
105101
}
@@ -124,37 +120,4 @@ public Map<String, String> getParameters() {
124120
return parameters;
125121
}
126122
}
127-
128-
public static class Builder<T extends MediaType> {
129-
private final Map<String, T> formatToMediaType = new HashMap<>();
130-
private final Map<String, T> typeWithSubtypeToMediaType = new HashMap<>();
131-
private final Map<String, Map<String, Pattern>> parametersMap = new HashMap<>();
132-
133-
public Builder<T> withMediaTypeAndParams(String alternativeMediaType, T mediaType, Map<String, String> paramNameAndValueRegex) {
134-
typeWithSubtypeToMediaType.put(alternativeMediaType.toLowerCase(Locale.ROOT), mediaType);
135-
formatToMediaType.put(mediaType.format(), mediaType);
136-
137-
Map<String, Pattern> parametersForMediaType = new HashMap<>(paramNameAndValueRegex.size());
138-
for (Map.Entry<String, String> params : paramNameAndValueRegex.entrySet()) {
139-
String parameterName = params.getKey().toLowerCase(Locale.ROOT);
140-
String parameterRegex = params.getValue();
141-
Pattern pattern = Pattern.compile(parameterRegex, Pattern.CASE_INSENSITIVE);
142-
parametersForMediaType.put(parameterName, pattern);
143-
}
144-
parametersMap.put(alternativeMediaType, parametersForMediaType);
145-
146-
return this;
147-
}
148-
149-
public Builder<T> copyFromMediaTypeParser(MediaTypeParser<? extends T> mediaTypeParser) {
150-
formatToMediaType.putAll(mediaTypeParser.formatToMediaType);
151-
typeWithSubtypeToMediaType.putAll(mediaTypeParser.typeWithSubtypeToMediaType);
152-
parametersMap.putAll(mediaTypeParser.parametersMap);
153-
return this;
154-
}
155-
156-
public MediaTypeParser<T> build() {
157-
return new MediaTypeParser<>(formatToMediaType, typeWithSubtypeToMediaType, parametersMap);
158-
}
159-
}
160123
}

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

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.xcontent.smile.SmileXContent;
2525
import org.elasticsearch.common.xcontent.yaml.YamlXContent;
2626

27-
import java.util.Collections;
2827
import java.util.Map;
2928

3029
/**
@@ -114,26 +113,9 @@ public XContent xContent() {
114113
}
115114
};
116115

117-
private static final String COMPATIBLE_WITH_PARAMETER_NAME = "compatible-with";
118-
private static final String VERSION_PATTERN = "\\d+";
119-
public static final MediaTypeParser<XContentType> mediaTypeParser = new MediaTypeParser.Builder<XContentType>()
120-
.withMediaTypeAndParams("application/smile", SMILE, Collections.emptyMap())
121-
.withMediaTypeAndParams("application/cbor", CBOR, Collections.emptyMap())
122-
.withMediaTypeAndParams("application/json", JSON, Map.of("charset", "UTF-8"))
123-
.withMediaTypeAndParams("application/yaml", YAML, Map.of("charset", "UTF-8"))
124-
.withMediaTypeAndParams("application/*", JSON, Map.of("charset", "UTF-8"))
125-
.withMediaTypeAndParams("application/x-ndjson", JSON, Map.of("charset", "UTF-8"))
126-
.withMediaTypeAndParams("application/vnd.elasticsearch+json", JSON,
127-
Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8"))
128-
.withMediaTypeAndParams("application/vnd.elasticsearch+smile", SMILE,
129-
Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8"))
130-
.withMediaTypeAndParams("application/vnd.elasticsearch+yaml", YAML,
131-
Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8"))
132-
.withMediaTypeAndParams("application/vnd.elasticsearch+cbor", CBOR,
133-
Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8"))
134-
.withMediaTypeAndParams("application/vnd.elasticsearch+x-ndjson", JSON,
135-
Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8"))
136-
.build();
116+
public static final MediaTypeParser<XContentType> mediaTypeParser = new MediaTypeParser<>(XContentType.values(),
117+
Map.of("application/*", JSON, "application/x-ndjson", JSON));
118+
137119

138120
/**
139121
* Accepts a format string, which is most of the time is equivalent to {@link XContentType#subtype()}
@@ -155,23 +137,13 @@ public static XContentType fromMediaType(String mediaTypeHeaderValue) {
155137
return mediaTypeParser.fromMediaType(mediaTypeHeaderValue);
156138
}
157139

140+
158141
private int index;
159142

160143
XContentType(int index) {
161144
this.index = index;
162145
}
163146

164-
public static Byte parseVersion(String mediaType) {
165-
MediaTypeParser<XContentType>.ParsedMediaType parsedMediaType = mediaTypeParser.parseMediaType(mediaType);
166-
if (parsedMediaType != null) {
167-
String version = parsedMediaType
168-
.getParameters()
169-
.get(COMPATIBLE_WITH_PARAMETER_NAME);
170-
return version != null ? Byte.parseByte(version) : null;
171-
}
172-
return null;
173-
}
174-
175147
public int index() {
176148
return index;
177149
}

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

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,46 +29,40 @@
2929
import static org.hamcrest.Matchers.nullValue;
3030

3131
public class MediaTypeParserTests extends ESTestCase {
32-
33-
MediaTypeParser<XContentType> mediaTypeParser = new MediaTypeParser.Builder<XContentType>()
34-
.withMediaTypeAndParams("application/vnd.elasticsearch+json",
35-
XContentType.JSON, Map.of("compatible-with", "\\d+",
36-
"charset", "UTF-8"))
37-
.build();
32+
MediaTypeParser<XContentType> mediaTypeParser = XContentType.mediaTypeParser;
3833

3934
public void testJsonWithParameters() throws Exception {
40-
String mediaType = "application/vnd.elasticsearch+json";
35+
String mediaType = "application/json";
4136
assertThat(mediaTypeParser.parseMediaType(mediaType).getParameters(),
4237
equalTo(Collections.emptyMap()));
4338
assertThat(mediaTypeParser.parseMediaType(mediaType + ";").getParameters(),
4439
equalTo(Collections.emptyMap()));
4540
assertThat(mediaTypeParser.parseMediaType(mediaType + "; charset=UTF-8").getParameters(),
4641
equalTo(Map.of("charset", "utf-8")));
47-
assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123;charset=UTF-8").getParameters(),
48-
equalTo(Map.of("charset", "utf-8", "compatible-with", "123")));
42+
assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123;charset=UTF-8").getParameters(),
43+
equalTo(Map.of("charset", "utf-8", "custom", "123")));
4944
}
5045

5146
public void testWhiteSpaceInTypeSubtype() {
52-
String mediaType = " application/vnd.elasticsearch+json ";
47+
String mediaType = " application/json ";
5348
assertThat(mediaTypeParser.parseMediaType(mediaType).getMediaType(),
5449
equalTo(XContentType.JSON));
5550

56-
assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123; charset=UTF-8").getParameters(),
57-
equalTo(Map.of("charset", "utf-8", "compatible-with", "123")));
58-
assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123;\n charset=UTF-8").getParameters(),
59-
equalTo(Map.of("charset", "utf-8", "compatible-with", "123")));
51+
assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123; charset=UTF-8").getParameters(),
52+
equalTo(Map.of("charset", "utf-8", "custom", "123")));
53+
assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123;\n charset=UTF-8").getParameters(),
54+
equalTo(Map.of("charset", "utf-8", "custom", "123")));
6055

6156
mediaType = " application / json ";
6257
assertThat(mediaTypeParser.parseMediaType(mediaType),
6358
is(nullValue()));
6459
}
6560

6661
public void testInvalidParameters() {
67-
String mediaType = "application/vnd.elasticsearch+json";
68-
assertThat(mediaTypeParser.parseMediaType(mediaType + "; charset=unknown") ,
69-
is(nullValue()));
62+
String mediaType = "application/json";
7063
assertThat(mediaTypeParser.parseMediaType(mediaType + "; keyvalueNoEqualsSign"),
7164
is(nullValue()));
65+
7266
assertThat(mediaTypeParser.parseMediaType(mediaType + "; key = value"),
7367
is(nullValue()));
7468
assertThat(mediaTypeParser.parseMediaType(mediaType + "; key=") ,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ public static ElasticsearchException[] guessRootCauses(Throwable t) {
631631
* parsing exception because that is generally the most interesting
632632
* exception to return to the user. If that exception is caused by
633633
* an ElasticsearchException we'd like to keep unwrapping because
634-
* ElasticsearchExceptions tend to contain useful information for
634+
* ElasticserachExceptions tend to contain useful information for
635635
* the user.
636636
*/
637637
Throwable cause = ex.getCause();

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
*/
1919
package org.elasticsearch.rest;
2020

21-
import org.apache.logging.log4j.LogManager;
22-
import org.apache.logging.log4j.Logger;
2321
import org.elasticsearch.common.Nullable;
2422
import org.elasticsearch.common.Strings;
2523
import org.elasticsearch.common.io.Streams;
@@ -38,7 +36,6 @@
3836

3937
public abstract class AbstractRestChannel implements RestChannel {
4038

41-
private static final Logger logger = LogManager.getLogger(AbstractRestChannel.class);
4239
private static final Predicate<String> INCLUDE_FILTER = f -> f.charAt(0) != '-';
4340
private static final Predicate<String> EXCLUDE_FILTER = INCLUDE_FILTER.negate();
4441

@@ -101,9 +98,8 @@ public XContentBuilder newBuilder(@Nullable XContentType requestContentType, boo
10198
public XContentBuilder newBuilder(@Nullable XContentType requestContentType, @Nullable XContentType responseContentType,
10299
boolean useFiltering) throws IOException {
103100
if (responseContentType == null) {
104-
if (Strings.hasText(format)) {
105-
responseContentType = XContentType.fromFormat(format);
106-
}
101+
//TODO PG shoudld format vs acceptHeader be always the same, do we allow overriding?
102+
responseContentType = XContentType.fromFormat(format);
107103
if (responseContentType == null) {
108104
responseContentType = XContentType.fromMediaType(acceptHeader);
109105
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class BytesRestResponse extends RestResponse {
5050

5151
private final RestStatus status;
5252
private final BytesReference content;
53-
private final String responseMediaType;
53+
private final String contentType;
5454

5555
/**
5656
* Creates a new response based on {@link XContentBuilder}.
@@ -69,24 +69,24 @@ public BytesRestResponse(RestStatus status, String content) {
6969
/**
7070
* Creates a new plain text response.
7171
*/
72-
public BytesRestResponse(RestStatus status, String responseMediaType, String content) {
73-
this(status, responseMediaType, new BytesArray(content));
72+
public BytesRestResponse(RestStatus status, String contentType, String content) {
73+
this(status, contentType, new BytesArray(content));
7474
}
7575

7676
/**
7777
* Creates a binary response.
7878
*/
79-
public BytesRestResponse(RestStatus status, String responseMediaType, byte[] content) {
80-
this(status, responseMediaType, new BytesArray(content));
79+
public BytesRestResponse(RestStatus status, String contentType, byte[] content) {
80+
this(status, contentType, new BytesArray(content));
8181
}
8282

8383
/**
8484
* Creates a binary response.
8585
*/
86-
public BytesRestResponse(RestStatus status, String responseMediaType, BytesReference content) {
86+
public BytesRestResponse(RestStatus status, String contentType, BytesReference content) {
8787
this.status = status;
8888
this.content = content;
89-
this.responseMediaType = responseMediaType;
89+
this.contentType = contentType;
9090
}
9191

9292
public BytesRestResponse(RestChannel channel, Exception e) throws IOException {
@@ -109,7 +109,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th
109109
try (XContentBuilder builder = channel.newErrorBuilder()) {
110110
build(builder, params, status, channel.detailedErrorsEnabled(), e);
111111
this.content = BytesReference.bytes(builder);
112-
this.responseMediaType = builder.contentType().mediaType();
112+
this.contentType = builder.contentType().mediaType();
113113
}
114114
if (e instanceof ElasticsearchException) {
115115
copyHeaders(((ElasticsearchException) e));
@@ -118,7 +118,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th
118118

119119
@Override
120120
public String contentType() {
121-
return this.responseMediaType;
121+
return this.contentType;
122122
}
123123

124124
@Override

‎server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,14 @@ public class RestTable {
5151

5252
public static RestResponse buildResponse(Table table, RestChannel channel) throws Exception {
5353
RestRequest request = channel.request();
54-
XContentType xContentType = getXContentType(request);
54+
XContentType xContentType = getxContentType(request);
5555
if (xContentType != null) {
5656
return buildXContentBuilder(table, channel);
5757
}
5858
return buildTextPlainResponse(table, channel);
5959
}
6060

61-
private static XContentType getXContentType(RestRequest request) {
61+
private static XContentType getxContentType(RestRequest request) {
6262
if (request.hasParam("format")) {
6363
return XContentType.fromFormat(request.param("format"));
6464
}

0 commit comments

Comments
 (0)