Skip to content

Commit 4f88f83

Browse files
original-brownbearcbuescher
authored andcommitted
Invalid JSON request body caused endless loop (#26680)
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
1 parent 2527bf0 commit 4f88f83

File tree

2 files changed

+37
-4
lines changed

2 files changed

+37
-4
lines changed

core/src/main/java/org/elasticsearch/rest/action/RestActions.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,15 @@ public RestResponse buildResponse(NodesResponse response, XContentBuilder builde
243243
private static QueryBuilder parseTopLevelQueryBuilder(XContentParser parser) {
244244
try {
245245
QueryBuilder queryBuilder = null;
246+
XContentParser.Token first = parser.nextToken();
247+
if (first == null) {
248+
return null;
249+
} else if (first != XContentParser.Token.START_OBJECT) {
250+
throw new ParsingException(
251+
parser.getTokenLocation(), "Expected [" + XContentParser.Token.START_OBJECT +
252+
"] but found [" + first + "]", parser.getTokenLocation()
253+
);
254+
}
246255
for (XContentParser.Token token = parser.nextToken(); token != XContentParser.Token.END_OBJECT; token = parser.nextToken()) {
247256
if (token == XContentParser.Token.FIELD_NAME) {
248257
String fieldName = parser.currentName();

core/src/test/java/org/elasticsearch/rest/action/RestActionsTests.java

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.elasticsearch.rest.action;
2121

22+
import com.fasterxml.jackson.core.io.JsonEOFException;
23+
import java.util.Arrays;
2224
import org.elasticsearch.common.ParsingException;
2325
import org.elasticsearch.common.settings.Settings;
2426
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@@ -59,10 +61,32 @@ public void testParseTopLevelBuilder() throws IOException {
5961
}
6062

6163
public void testParseTopLevelBuilderEmptyObject() throws IOException {
62-
String requestBody = "{}";
63-
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
64-
QueryBuilder query = RestActions.getQueryContent(parser);
65-
assertNull(query);
64+
for (String requestBody : Arrays.asList("{}", "")) {
65+
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
66+
QueryBuilder query = RestActions.getQueryContent(parser);
67+
assertNull(query);
68+
}
69+
}
70+
}
71+
72+
public void testParseTopLevelBuilderMalformedJson() throws IOException {
73+
for (String requestBody : Arrays.asList("\"\"", "\"someString\"", "\"{\"")) {
74+
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
75+
ParsingException exception =
76+
expectThrows(ParsingException.class, () -> RestActions.getQueryContent(parser));
77+
assertEquals("Expected [START_OBJECT] but found [VALUE_STRING]", exception.getMessage());
78+
}
79+
}
80+
}
81+
82+
public void testParseTopLevelBuilderIncompleteJson() throws IOException {
83+
for (String requestBody : Arrays.asList("{", "{ \"query\" :")) {
84+
try (XContentParser parser = createParser(JsonXContent.jsonXContent, requestBody)) {
85+
ParsingException exception =
86+
expectThrows(ParsingException.class, () -> RestActions.getQueryContent(parser));
87+
assertEquals("Failed to parse", exception.getMessage());
88+
assertEquals(JsonEOFException.class, exception.getRootCause().getClass());
89+
}
6690
}
6791
}
6892

0 commit comments

Comments
 (0)