diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index e560540236c2f..4a63088c6d5ad 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -1155,7 +1155,7 @@ public void testMultiSearch() throws IOException { List requests = new ArrayList<>(); CheckedBiConsumer consumer = (searchRequest, p) -> { - SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(p); + SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(p, false); if (searchSourceBuilder.equals(new SearchSourceBuilder()) == false) { searchRequest.source(searchSourceBuilder); } diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 0d3770993b2ff..92c684d8a125c 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -70,3 +70,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. diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java index 50f63841231f8..360e332f2c32d 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java @@ -112,7 +112,7 @@ static SearchRequest convert(SearchTemplateRequest searchTemplateRequest, 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); diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java index 770c91e82a6a1..8f17c8203b7e8 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java @@ -223,7 +223,7 @@ public void addSummaryFields(List summaryFields) { 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); diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java index a076b93fbd30d..019ae274466ab 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java @@ -107,7 +107,7 @@ protected void doExecute(RankEvalRequest request, ActionListener { - searchRequest.source(SearchSourceBuilder.fromXContent(parser)); + searchRequest.source(SearchSourceBuilder.fromXContent(parser, false)); multiRequest.add(searchRequest); }); List requests = multiRequest.requests(); diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java index 011b24309f8f4..184a8d364c6fe 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java @@ -109,7 +109,7 @@ public static void parseSearchRequest(SearchRequest searchRequest, RestRequest r } 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 static void parseSearchRequest(SearchRequest searchRequest, RestRequest r // 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. diff --git a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index 815abf1b7a7c4..b2d1062ba0056 100644 --- a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/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 @@ private SearchSourceBuilder shallowCopy(QueryBuilder queryBuilder, QueryBuilder 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 void parseXContent(XContentParser parser) throws IOException { parser.getTokenLocation()); } } + if (checkTrailingTokens) { + token = parser.nextToken(); + if (token != null) { + throw new ParsingException(parser.getTokenLocation(), "Unexpected token [" + token + "] found after the main object."); + } + } } @Override diff --git a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java index d9fea03a56692..a772fa6951c2f 100644 --- a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java @@ -165,7 +165,7 @@ public void testResponseErrorToXContent() throws IOException { new MultiSearchResponse.Item(null, new IllegalStateException("baaaaaazzzz")) }, tookInMillis); - assertEquals("{\"took\":" + assertEquals("{\"took\":" + tookInMillis + ",\"responses\":[" + "{" @@ -225,7 +225,7 @@ public void testMultiLineSerialization() throws IOException { byte[] originalBytes = MultiSearchRequest.writeMultiLineFormat(originalRequest, xContentType.xContent()); MultiSearchRequest parsedRequest = new MultiSearchRequest(); CheckedBiConsumer 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 @@ private static MultiSearchRequest createMultiSearchRequest() throws IOException if (randomBoolean()) { searchRequest.allowPartialSearchResults(true); } - + // scroll is not supported in the current msearch api, so unset it: searchRequest.scroll((Scroll) null); diff --git a/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index 66d6f68b8a4aa..cda66cbfc6765 100644 --- a/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/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 void testFromXContent() throws IOException { 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