Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow parsing Content-Type and Accept headers with version #61427

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
7514d87
Allow parsing Content-Type and Accept headers with version
pgomulka Aug 21, 2020
921bc6f
code review follow up
pgomulka Sep 1, 2020
ee280ba
unused imports
pgomulka Sep 1, 2020
433de74
revert unrelated test
pgomulka Sep 2, 2020
51467e0
remove */* and method visibility
pgomulka Sep 2, 2020
ccaadbd
typos
pgomulka Sep 3, 2020
de13d4b
Merge branch 'master' into compat/content-type-header-with-version
pgomulka Sep 17, 2020
d9ee420
exted parsing with parameters
pgomulka Sep 17, 2020
41150a6
import
pgomulka Sep 17, 2020
2fc8f86
fix test
pgomulka Sep 17, 2020
734031a
scope parser
pgomulka Sep 18, 2020
c04af65
x-ndjson to be versioned
pgomulka Sep 18, 2020
474eb29
Merge branch 'master' into compat/content-type-header-with-version
pgomulka Sep 18, 2020
5c30813
Apply suggestions from code review
pgomulka Sep 18, 2020
7b760ba
precompile pattern
pgomulka Sep 18, 2020
5853561
import
pgomulka Sep 21, 2020
b6ebd63
removal of unused method and pattern in method
pgomulka Sep 21, 2020
ff6c94f
throwing exception when failing to parse
pgomulka Sep 22, 2020
b969f2b
checkstyle
pgomulka Sep 22, 2020
0684e58
test fixes
pgomulka Sep 23, 2020
b91d472
*/* header
pgomulka Sep 23, 2020
9edc49f
catch
pgomulka Sep 23, 2020
c5d8166
hacks around text/plain
pgomulka Sep 23, 2020
ad673b6
sout remove
pgomulka Sep 23, 2020
1ae6a60
precommit
pgomulka Sep 23, 2020
1ae63c6
fix nullpointer
pgomulka Sep 24, 2020
ee1e5a0
fix yml test
pgomulka Sep 24, 2020
9f4b6c3
fix yml test
pgomulka Sep 24, 2020
bd4dee1
throwing exception when failing to parse
pgomulka Sep 22, 2020
ce42f1b
Revert "throwing exception when failing to parse"
pgomulka Sep 24, 2020
0f8ae7b
Merge branch 'revert_strict_header_parsing' into compat/content-type-…
pgomulka Sep 24, 2020
c4f02ad
import
pgomulka Sep 28, 2020
176302e
fix storing response time in sql
pgomulka Sep 28, 2020
eab471c
return null on errors
pgomulka Sep 28, 2020
e128fd7
sqlmedia type parsing test - do not throw exceptions
pgomulka Sep 28, 2020
66435dd
remove todoes
pgomulka Sep 28, 2020
08f1395
rename argument
pgomulka Sep 28, 2020
3f3f0d5
review follow up
pgomulka Sep 29, 2020
427c391
remove pattern from signature
pgomulka Sep 30, 2020
e031605
fix regex
pgomulka Sep 30, 2020
4134d92
Apply suggestions from code review
pgomulka Oct 2, 2020
a976f5b
Apply suggestions from code review
pgomulka Oct 5, 2020
3e0eec0
relax delimiter validation
pgomulka Oct 5, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

import java.util.Locale;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* The content type of {@link org.elasticsearch.common.xcontent.XContent}.
Expand Down Expand Up @@ -114,13 +116,35 @@ public XContent xContent() {
}
};

/**
* A regexp to allow parsing media types. It covers two use cases.
* 1. Media type with a version - requires a custom vnd.elasticserach subtype and a compatible-with parameter
jaymode marked this conversation as resolved.
Show resolved Hide resolved
* i.e. application/vnd.elasticsearch+json;compatible-with
* 2. Media type without a version - for users not using compatible API i.e. application/json
*/
private static final Pattern COMPATIBLE_API_HEADER_PATTERN = Pattern.compile(
//type
"^(application|text)/" +
// custom subtype and a version: vnd.elasticsearch+json;compatible-with=7
"((vnd\\.elasticsearch\\+([^;\\s]+)(\\s*;\\s*compatible-with=(\\d+)))" +
"|([^;\\s]+))" + //subtype: json,yaml,etc some of these are defined in x-pack so can't be enumerated
"(?:\\s*;\\s*(charset=UTF-8)?)?$",
jaymode marked this conversation as resolved.
Show resolved Hide resolved
Pattern.CASE_INSENSITIVE);


/*Pattern.compile(
jaymode marked this conversation as resolved.
Show resolved Hide resolved
"^(application|text)/(vnd\\.elasticsearch\\+)?([^;]+)(\\s*;\\s*compatible-with=(\\d+))?$",
Pattern.CASE_INSENSITIVE);
*/
/**
* Accepts either a format string, which is equivalent to {@link XContentType#shortName()} or a media type that optionally has
* parameters and attempts to match the value to an {@link XContentType}. The comparisons are done in lower case format and this method
* also supports a wildcard accept for {@code application/*}. This method can be used to parse the {@code Accept} HTTP header or a
* format query string parameter. This method will return {@code null} if no match is found
*/
public static XContentType fromMediaTypeOrFormat(String mediaType) {
public static XContentType fromMediaTypeOrFormat(String mediaTypeHeaderValue) {
String mediaType = parseMediaType(mediaTypeHeaderValue);

if (mediaType == null) {
return null;
}
Expand All @@ -130,7 +154,7 @@ public static XContentType fromMediaTypeOrFormat(String mediaType) {
}
}
final String lowercaseMediaType = mediaType.toLowerCase(Locale.ROOT);
if (lowercaseMediaType.startsWith("application/*")) {
if (lowercaseMediaType.startsWith("application/*") || lowercaseMediaType.equals("*/*")) {
jaymode marked this conversation as resolved.
Show resolved Hide resolved
return JSON;
}

Expand All @@ -142,7 +166,9 @@ public static XContentType fromMediaTypeOrFormat(String mediaType) {
* The provided media type should not include any parameters. This method is suitable for parsing part of the {@code Content-Type}
* HTTP header. This method will return {@code null} if no match is found
*/
public static XContentType fromMediaType(String mediaType) {
public static XContentType fromMediaType(String mediaTypeHeaderValue) {
String mediaType = parseMediaType(mediaTypeHeaderValue);

final String lowercaseMediaType = Objects.requireNonNull(mediaType, "mediaType cannot be null").toLowerCase(Locale.ROOT);
for (XContentType type : values()) {
if (type.mediaTypeWithoutParameters().equals(lowercaseMediaType)) {
Expand All @@ -157,6 +183,31 @@ public static XContentType fromMediaType(String mediaType) {
return null;
}

// package private for testing
static String parseMediaType(String mediaType) {
if (mediaType != null) {
Matcher matcher = COMPATIBLE_API_HEADER_PATTERN.matcher(mediaType);
if (matcher.find()) {
if(matcher.group(2).toLowerCase(Locale.ROOT).startsWith("vnd.elasticsearch")){
return (matcher.group(1) + "/" + matcher.group(4)).toLowerCase(Locale.ROOT);
}
return (matcher.group(1) + "/" + matcher.group(7)).toLowerCase(Locale.ROOT);
}
}

return null;
}

// package private for testing
static String parseVersion(String mediaType){
if(mediaType != null){
Matcher matcher = COMPATIBLE_API_HEADER_PATTERN.matcher(mediaType);
if (matcher.find() && matcher.group(2).toLowerCase(Locale.ROOT).startsWith("vnd.elasticsearch")) {
return matcher.group(6);
}
}
return null;
}
private static boolean isSameMediaTypeOrFormatAs(String stringType, XContentType type) {
return type.mediaTypeWithoutParameters().equalsIgnoreCase(stringType) ||
stringType.toLowerCase(Locale.ROOT).startsWith(type.mediaTypeWithoutParameters().toLowerCase(Locale.ROOT) + ";") ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
pgomulka marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -84,4 +85,75 @@ public void testFromRubbish() throws Exception {
assertThat(XContentType.fromMediaTypeOrFormat("text/plain"), nullValue());
assertThat(XContentType.fromMediaTypeOrFormat("gobbly;goop"), nullValue());
}

public void testMediaType() {
pgomulka marked this conversation as resolved.
Show resolved Hide resolved
String version = String.valueOf(Math.abs(randomByte()));
assertThat(XContentType.parseMediaType("application/vnd.elasticsearch+json;compatible-with=" + version),
equalTo("application/json"));
assertThat(XContentType.parseMediaType("application/vnd.elasticsearch+cbor;compatible-with=" + version),
equalTo("application/cbor"));
assertThat(XContentType.parseMediaType("application/vnd.elasticsearch+smile;compatible-with=" + version),
equalTo("application/smile"));
assertThat(XContentType.parseMediaType("application/vnd.elasticsearch+yaml;compatible-with=" + version),
equalTo("application/yaml"));
assertThat(XContentType.parseMediaType("application/json"),
equalTo("application/json"));


assertThat(XContentType.parseMediaType("APPLICATION/VND.ELASTICSEARCH+JSON;COMPATIBLE-WITH=" + version),
equalTo("application/json"));
assertThat(XContentType.parseMediaType("APPLICATION/JSON"),
equalTo("application/json"));
}

public void testVersionParsing() {
String version = String.valueOf(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/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"),
pgomulka marked this conversation as resolved.
Show resolved Hide resolved
is(nullValue()));
}

public void testMediaTypeWithoutESSubtype() {
String version = String.valueOf(Math.abs(randomByte()));
assertThat(XContentType.fromMediaTypeOrFormat("application/json;compatible-with=" + version), nullValue());
}

public void testAnchoring(){
pgomulka marked this conversation as resolved.
Show resolved Hide resolved
String version = String.valueOf(Math.abs(randomByte()));
assertThat(XContentType.fromMediaTypeOrFormat("sth_application/json;compatible-with=" + version+".0"), nullValue());
assertThat(XContentType.fromMediaTypeOrFormat("sth_application/json;compatible-with=" + version+"_sth"), nullValue());
//incorrect parameter not validated at the moment, just ignored
assertThat(XContentType.fromMediaTypeOrFormat("application/json;compatible-with=" + version+"_sth"), nullValue());
}

public void testVersionParsingOnText() {
String version = String.valueOf(Math.abs(randomByte()));
assertThat(XContentType.parseVersion("text/vnd.elasticsearch+csv;compatible-with=" + version),
equalTo(version));
assertThat(XContentType.parseVersion("text/vnd.elasticsearch+text;compatible-with=" + version),
equalTo(version));
assertThat(XContentType.parseVersion("text/vnd.elasticsearch+tab-separated-values;compatible-with=" + version),
equalTo(version));
assertThat(XContentType.parseVersion("text/csv"),
nullValue());

assertThat(XContentType.parseVersion("TEXT/VND.ELASTICSEARCH+CSV;COMPATIBLE-WITH=" + version),
equalTo(version));
assertThat(XContentType.parseVersion("TEXT/csv"),
nullValue());
}
}