Fail _search request with trailing tokens (#29428)

This change validates that the `_search` request does not have trailing
tokens after the main object and fails the request with a parsing exception otherwise.

Closes #28995

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

@@ -1127,7 +1127,7 @@ public class RequestTests extends ESTestCase {
 
         List<SearchRequest> requests = new ArrayList<>();
         CheckedBiConsumer<SearchRequest, XContentParser, IOException> consumer = (searchRequest, p) -> {
-            SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(p);
+            SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(p, false);
             if (searchSourceBuilder.equals(new SearchSourceBuilder()) == false) {
                 searchRequest.source(searchSourceBuilder);
             }

docs/reference/migration/migrate_7_0/search.asciidoc

@@ -73,3 +73,8 @@ Executing a Regexp Query with a long regex string may degrade search performance
 To safeguard against this, the maximum length of regex that can be used in a
 Regexp Query request has been limited to 1000. This default maximum can be changed
 for a particular index with the index setting `index.max_regex_length`.
+
+==== Invalid `_search` request body
+
+Search requests with extra content after the main object will no longer be accepted
+by the `_search` endpoint. A parsing exception will be thrown instead.

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java

@@ -112,7 +112,7 @@ public class TransportSearchTemplateAction extends HandledTransportAction<Search
         try (XContentParser parser = XContentFactory.xContent(XContentType.JSON)
                 .createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, source)) {
             SearchSourceBuilder builder = SearchSourceBuilder.searchSource();
-            builder.parseXContent(parser);
+            builder.parseXContent(parser, true);
             builder.explain(searchTemplateRequest.isExplain());
             builder.profile(searchTemplateRequest.isProfile());
             searchRequest.source(builder);

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java

@@ -223,7 +223,7 @@ public class RatedRequest implements Writeable, ToXContentObject {
             return RatedDocument.fromXContent(p);
         }, RATINGS_FIELD);
         PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) ->
-                SearchSourceBuilder.fromXContent(p), REQUEST_FIELD);
+                SearchSourceBuilder.fromXContent(p, false), REQUEST_FIELD);
         PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), PARAMS_FIELD);
         PARSER.declareStringArray(RatedRequest::addSummaryFields, FIELDS_FIELD);
         PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), TEMPLATE_ID_FIELD);

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java

@@ -107,7 +107,7 @@ public class TransportRankEvalAction extends HandledTransportAction<RankEvalRequ
                 String resolvedRequest = templateScript.newInstance(params).execute();
                 try (XContentParser subParser = createParser(namedXContentRegistry,
                     LoggingDeprecationHandler.INSTANCE, new BytesArray(resolvedRequest), XContentType.JSON)) {
-                    ratedSearchSource = SearchSourceBuilder.fromXContent(subParser);
+                    ratedSearchSource = SearchSourceBuilder.fromXContent(subParser, false);
                 } catch (IOException e) {
                     // if we fail parsing, put the exception into the errors map and continue
                     errors.put(ratedRequest.getId(), e);

modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java

@@ -77,7 +77,7 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
             try (InputStream stream = BytesReference.bytes(builder).streamInput();
                  XContentParser innerParser = parser.contentType().xContent()
                      .createParser(parser.getXContentRegistry(), parser.getDeprecationHandler(), stream)) {
-                request.getSearchRequest().source().parseXContent(innerParser);
+                request.getSearchRequest().source().parseXContent(innerParser, false);
             }
         };
 

server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java

@@ -94,7 +94,7 @@ public class RestMultiSearchAction extends BaseRestHandler {
 
 
         parseMultiLineRequest(restRequest, multiRequest.indicesOptions(), allowExplicitIndex, (searchRequest, parser) -> {
-            searchRequest.source(SearchSourceBuilder.fromXContent(parser));
+            searchRequest.source(SearchSourceBuilder.fromXContent(parser, false));
             multiRequest.add(searchRequest);
         });
         List<SearchRequest> requests = multiRequest.requests();

server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

@@ -109,7 +109,7 @@ public class RestSearchAction extends BaseRestHandler {
         }
         searchRequest.indices(Strings.splitStringByCommaToArray(request.param("index")));
         if (requestContentParser != null) {
-            searchRequest.source().parseXContent(requestContentParser);
+            searchRequest.source().parseXContent(requestContentParser, true);
         }
 
         final int batchedReduceSize = request.paramAsInt("batched_reduce_size", searchRequest.getBatchedReduceSize());
@@ -128,7 +128,7 @@ public class RestSearchAction extends BaseRestHandler {
             // only set if we have the parameter passed to override the cluster-level default
             searchRequest.allowPartialSearchResults(request.paramAsBoolean("allow_partial_search_results", null));
         }
-        
+
         // do not allow 'query_and_fetch' or 'dfs_query_and_fetch' search types
         // from the REST layer. these modes are an internal optimization and should
         // not be specified explicitly by the user.

server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

@@ -111,8 +111,12 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R
     public static final ParseField ALL_FIELDS_FIELDS = new ParseField("all_fields");
 
     public static SearchSourceBuilder fromXContent(XContentParser parser) throws IOException {
+        return fromXContent(parser, true);
+    }
+
+    public static SearchSourceBuilder fromXContent(XContentParser parser, boolean checkTrailingTokens) throws IOException {
         SearchSourceBuilder builder = new SearchSourceBuilder();
-        builder.parseXContent(parser);
+        builder.parseXContent(parser, checkTrailingTokens);
         return builder;
     }
 
@@ -951,12 +955,19 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R
         return rewrittenBuilder;
     }
 
+    public void parseXContent(XContentParser parser) throws IOException {
+        parseXContent(parser, true);
+    }
+
     /**
      * Parse some xContent into this SearchSourceBuilder, overwriting any values specified in the xContent. Use this if you need to set up
-     * different defaults than a regular SearchSourceBuilder would have and use
-     * {@link #fromXContent(XContentParser)} if you have normal defaults.
+     * different defaults than a regular SearchSourceBuilder would have and use {@link #fromXContent(XContentParser, boolean)} if you have
+     * normal defaults.
+     *
+     * @param parser The xContent parser.
+     * @param checkTrailingTokens If true throws a parsing exception when extra tokens are found after the main object.
      */
-    public void parseXContent(XContentParser parser) throws IOException {
+    public void parseXContent(XContentParser parser, boolean checkTrailingTokens) throws IOException {
         XContentParser.Token token = parser.currentToken();
         String currentFieldName = null;
         if (token != XContentParser.Token.START_OBJECT && (token = parser.nextToken()) != XContentParser.Token.START_OBJECT) {
@@ -1106,6 +1117,12 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R
                         parser.getTokenLocation());
             }
         }
+        if (checkTrailingTokens) {
+            token = parser.nextToken();
+            if (token != null) {
+                throw new ParsingException(parser.getTokenLocation(), "Unexpected token [" + token + "] found after the main object.");
+            }
+        }
     }
 
     @Override

server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java

@@ -165,7 +165,7 @@ public class MultiSearchRequestTests extends ESTestCase {
                         new MultiSearchResponse.Item(null, new IllegalStateException("baaaaaazzzz"))
                 }, tookInMillis);
 
-        assertEquals("{\"took\":" 
+        assertEquals("{\"took\":"
                         + tookInMillis
                         + ",\"responses\":["
                         + "{"
@@ -225,7 +225,7 @@ public class MultiSearchRequestTests extends ESTestCase {
             byte[] originalBytes = MultiSearchRequest.writeMultiLineFormat(originalRequest, xContentType.xContent());
             MultiSearchRequest parsedRequest = new MultiSearchRequest();
             CheckedBiConsumer<SearchRequest, XContentParser, IOException> consumer = (r, p) -> {
-                SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(p);
+                SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(p, false);
                 if (searchSourceBuilder.equals(new SearchSourceBuilder()) == false) {
                     r.source(searchSourceBuilder);
                 }
@@ -273,7 +273,7 @@ public class MultiSearchRequestTests extends ESTestCase {
             if (randomBoolean()) {
                 searchRequest.allowPartialSearchResults(true);
             }
-            
+
             // scroll is not supported in the current msearch api, so unset it:
             searchRequest.scroll((Scroll) null);
 

server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.builder;
 
+import com.fasterxml.jackson.core.JsonParseException;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesReference;
@@ -67,6 +68,18 @@ public class SearchSourceBuilderTests extends AbstractSearchTestCase {
         assertParseSearchSource(testSearchSourceBuilder, createParser(builder));
     }
 
+    public void testFromXContentInvalid() throws IOException {
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{}}")) {
+            JsonParseException exc = expectThrows(JsonParseException.class, () -> SearchSourceBuilder.fromXContent(parser));
+            assertThat(exc.getMessage(), containsString("Unexpected close marker"));
+        }
+
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{}{}")) {
+            ParsingException exc = expectThrows(ParsingException.class, () -> SearchSourceBuilder.fromXContent(parser));
+            assertThat(exc.getDetailedMessage(), containsString("found after the main object"));
+        }
+    }
+
     private static void assertParseSearchSource(SearchSourceBuilder testBuilder, XContentParser parser) throws IOException {
         if (randomBoolean()) {
             parser.nextToken(); // sometimes we move it on the START_OBJECT to