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.

Sign up for GitHub

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

Do not allow spaces within MediaType's parameters #64650

Merged
merged 6 commits into from
Nov 6, 2020
Merged

Do not allow spaces within MediaType's parameters #64650

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
52b53af
Do not allow spaces within MediaType's parameters
pgomulka Nov 5, 2020
80b3944
unnecessary if
pgomulka Nov 5, 2020
5298696
remove trimp approach
pgomulka Nov 5, 2020
fc62ad4
line formatting
pgomulka Nov 5, 2020
bdaeacd
Merge branch 'master' into compat/allign_paramter_parsing_to_rfc
pgomulka Nov 5, 2020
ae3a333
Merge branch 'master' into compat/allign_paramter_parsing_to_rfc
pgomulka Nov 6, 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
22 changes: 14 additions & 8 deletions libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public Map<String, String> getParameters() {

/**
* Parses a header value into it's parts.
* follows https://tools.ietf.org/html/rfc7231#section-3.1.1.1 but allows only single media type and do not support quality factors
* Note: parsing can return null, but it will throw exceptions once https://github.com/elastic/elasticsearch/issues/63080 is done
* Do not rely on nulls
*
Expand Down Expand Up @@ -90,16 +91,14 @@ public static ParsedMediaType parseMediaType(String headerValue) {
if (paramsAsString.isEmpty()) {
continue;
}
// intentionally allowing to have spaces around `=`
// https://tools.ietf.org/html/rfc7231#section-3.1.1.1 disallows this
String[] keyValueParam = elements[i].trim().split("=");
if (keyValueParam.length == 2) {
String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT).trim();
String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT).trim();
parameters.put(parameterName, parameterValue);
} else {
//spaces are allowed between parameters, but not between '=' sign
String[] keyValueParam = paramsAsString.split("=");
if (keyValueParam.length != 2 || hasTrailingSpace(keyValueParam[0]) || hasLeadingSpace(keyValueParam[1])) {
throw new IllegalArgumentException("invalid parameters for header [" + headerValue + "]");
}
String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT).trim();
String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT).trim();
parameters.put(parameterName, parameterValue);
}
return new ParsedMediaType(splitMediaType[0].trim().toLowerCase(Locale.ROOT),
splitMediaType[1].trim().toLowerCase(Locale.ROOT), parameters);
Expand All @@ -108,6 +107,13 @@ public static ParsedMediaType parseMediaType(String headerValue) {
return null;
}

private static boolean hasTrailingSpace(String s) {
return s.length() == 0 || Character.isWhitespace(s.charAt(s.length()-1));
}

private static boolean hasLeadingSpace(String s) {
return s.length() == 0 || Character.isWhitespace(s.charAt(0));
}
/**
* Resolves this instance to a MediaType instance defined in given MediaTypeRegistry.
* Performs validation against parameters.
Expand Down
20 changes: 12 additions & 8 deletions libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ParsedMediaTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,6 @@ public void testWithParameters() {
ParsedMediaType.parseMediaType(mediaType + "; compatible-with=123;charset=UTF-8").getParameters());
}

public void testWhiteSpaces() {
//be lenient with white space since it can be really hard to troubleshoot
String mediaType = " application/foo ";
ParsedMediaType parsedMediaType = ParsedMediaType.parseMediaType(mediaType + " ; compatible-with = 123 ; charset=UTF-8");
assertEquals("application/foo", parsedMediaType.mediaTypeWithoutParameters());
assertEquals((Map.of("charset", "utf-8", "compatible-with", "123")), parsedMediaType.getParameters());
}

public void testEmptyParams() {
String mediaType = "application/foo";
ParsedMediaType parsedMediaType = ParsedMediaType.parseMediaType(mediaType + randomFrom("", " ", ";", ";;", ";;;"));
Expand All @@ -105,6 +97,18 @@ public void testMalformedParameters() {
exception = expectThrows(IllegalArgumentException.class,
() -> ParsedMediaType.parseMediaType(mediaType + "; char=set=unknown"));
assertThat(exception.getMessage(), equalTo("invalid parameters for header [application/foo; char=set=unknown]"));

// do not allow white space in parameters between `=`
exception = expectThrows(IllegalArgumentException.class,
() -> ParsedMediaType.parseMediaType(mediaType + " ; compatible-with = 123 ; charset=UTF-8"));
assertThat(exception.getMessage(),
equalTo("invalid parameters for header [application/foo ; compatible-with = 123 ; charset=UTF-8]"));

expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k =y"));
expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k= y"));
expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k = y"));
expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";= y"));
expectThrows(IllegalArgumentException.class, () -> ParsedMediaType.parseMediaType(mediaType + ";k="));
}

public void testDefaultAcceptHeader() {
Expand Down