Skip to content

Commit

Permalink
Invalid JSON request body caused endless loop (#26680)
Browse files Browse the repository at this point in the history
Request bodys that only consists of a String value can lead to endless loops in the
parser of several rest requests like e.g. `_count`. Up to 5.2 this seems to have been caught 
in the logic guessing the content type of the request, but since then it causes the node to
block. This change introduces checks for receiving a valid xContent object before starting the
parsing in RestActions#parseTopLevelQueryBuilder().

Closes #26083
  • Loading branch information
original-brownbear authored and cbuescher committed Sep 19, 2017
1 parent 024aee0 commit e38a9f1
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,15 @@ public RestResponse buildResponse(NodesResponse response, XContentBuilder builde
private static QueryBuilder parseTopLevelQueryBuilder(XContentParser parser) {
try {
QueryBuilder queryBuilder = null;
XContentParser.Token first = parser.nextToken();
if (first == null) {
return null;
} else if (first != XContentParser.Token.START_OBJECT) {
throw new ParsingException(
parser.getTokenLocation(), "Expected [" + XContentParser.Token.START_OBJECT +
"] but found [" + first + "]", parser.getTokenLocation()
);
}
for (XContentParser.Token token = parser.nextToken(); token != XContentParser.Token.END_OBJECT; token = parser.nextToken()) {
if (token == XContentParser.Token.FIELD_NAME) {
String fieldName = parser.currentName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.elasticsearch.rest.action;

import com.fasterxml.jackson.core.io.JsonEOFException;
import java.util.Arrays;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -59,10 +61,32 @@ public void testParseTopLevelBuilder() throws IOException {
}

public void testParseTopLevelBuilderEmptyObject() throws IOException {
String requestBody = "{}";
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
QueryBuilder query = RestActions.getQueryContent(parser);
assertNull(query);
for (String requestBody : Arrays.asList("{}", "")) {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
QueryBuilder query = RestActions.getQueryContent(parser);
assertNull(query);
}
}
}

public void testParseTopLevelBuilderMalformedJson() throws IOException {
for (String requestBody : Arrays.asList("\"\"", "\"someString\"", "\"{\"")) {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
ParsingException exception =
expectThrows(ParsingException.class, () -> RestActions.getQueryContent(parser));
assertEquals("Expected [START_OBJECT] but found [VALUE_STRING]", exception.getMessage());
}
}
}

public void testParseTopLevelBuilderIncompleteJson() throws IOException {
for (String requestBody : Arrays.asList("{", "{ \"query\" :")) {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
ParsingException exception =
expectThrows(ParsingException.class, () -> RestActions.getQueryContent(parser));
assertEquals("Failed to parse", exception.getMessage());
assertEquals(JsonEOFException.class, exception.getRootCause().getClass());
}
}
}

Expand Down

0 comments on commit e38a9f1

Please sign in to comment.