Skip to content

Commit

Permalink
Remove leniency in msearch parsing (elastic#61771)
Browse files Browse the repository at this point in the history
Extra characters after msearch request lines are currently ignored. This
commit adds a check for extra chars and throws an exception if found.

Fixes elastic#61771
  • Loading branch information
limotova committed Dec 9, 2023
1 parent f74d18d commit 936cb1a
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParseException;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentParserConfiguration;

Expand Down Expand Up @@ -243,6 +244,9 @@ public static void readMultiLineFormat(
XContentParser parser = xContent.createParser(parserConfig, stream)
) {
Map<String, Object> source = parser.map();
if (parser.nextToken() != null) {
throw new XContentParseException(parser.getTokenLocation(), "Unexpected token after end of object");
}
Object expandWildcards = null;
Object ignoreUnavailable = null;
Object ignoreThrottled = null;
Expand Down Expand Up @@ -304,6 +308,9 @@ public static void readMultiLineFormat(
BytesReference bytes = data.slice(from, nextMarker - from);
try (InputStream stream = bytes.streamInput(); XContentParser parser = xContent.createParser(parserConfig, stream)) {
consumer.accept(searchRequest, parser);
if (parser.nextToken() != null) {
throw new XContentParseException(parser.getTokenLocation(), "Unexpected token after end of object");
}
}
// move pointers
from = nextMarker + 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParseException;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xcontent.json.JsonXContent;
Expand Down Expand Up @@ -87,7 +88,7 @@ public void testSimpleAdd() throws Exception {

public void testFailWithUnknownKey() {
final String requestContent = """
{"index":"test", "ignore_unavailable" : true, "unknown_key" : "open,closed"}}
{"index":"test", "ignore_unavailable" : true, "unknown_key" : "open,closed"}
{"query" : {"match_all" :{}}}
""";
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent(
Expand All @@ -103,7 +104,7 @@ public void testFailWithUnknownKey() {

public void testSimpleAddWithCarriageReturn() throws Exception {
final String requestContent = """
{"index":"test", "ignore_unavailable" : true, "expand_wildcards" : "open,closed"}}
{"index":"test", "ignore_unavailable" : true, "expand_wildcards" : "open,closed"}
{"query" : {"match_all" :{}}}
""";
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent(
Expand All @@ -121,7 +122,7 @@ public void testSimpleAddWithCarriageReturn() throws Exception {

public void testDefaultIndicesOptions() throws IOException {
final String requestContent = """
{"index":"test", "expand_wildcards" : "open,closed"}}
{"index":"test", "expand_wildcards" : "open,closed"}
{"query" : {"match_all" :{}}}
""";
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent(
Expand Down Expand Up @@ -542,6 +543,41 @@ public void testEqualsAndHashcode() {
checkEqualsAndHashCode(createMultiSearchRequest(), MultiSearchRequestTests::copyRequest, MultiSearchRequestTests::mutate);
}

public void testFailOnExtraCharacters() throws IOException {
try {
parseMultiSearchRequestFromString("""
{"index": "test"}{{{{{extra chars that shouldn't be here
{ "query": {"match_all": {}}}
""", null);
fail("should have caught first line; extra open brackets");
} catch (XContentParseException e) {
assertEquals("[1:18] Unexpected token after end of object", e.getMessage());
}
try {
parseMultiSearchRequestFromString("""
{"index": "test"}
{ "query": {"match_all": {}}}{{{{even more chars
""", null);
fail("should have caught second line");
} catch (XContentParseException e) {
assertEquals("[1:30] Unexpected token after end of object", e.getMessage());
}
try {
parseMultiSearchRequestFromString("""
{}
{ "query": {"match_all": {}}}}}}different error message
""", null);
fail("should have caught second line; extra closing brackets");
} catch (XContentParseException e) {
assertEquals(
"[1:31] Unexpected close marker '}': expected ']' (for root starting at "
+ "[Source: (org.elasticsearch.common.io.stream.ByteBufferStreamInput); line: 1, column: 0])\n"
+ " at [Source: (org.elasticsearch.common.io.stream.ByteBufferStreamInput); line: 1, column: 31]",
e.getMessage()
);
}
}

private static MultiSearchRequest mutate(MultiSearchRequest searchRequest) throws IOException {
MultiSearchRequest mutation = copyRequest(searchRequest);
List<CheckedRunnable<IOException>> mutators = new ArrayList<>();
Expand Down

0 comments on commit 936cb1a

Please sign in to comment.