diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 6f698f5b541d..361326f1ca51 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -2231,6 +2231,7 @@ "public com.yahoo.search.query.Model getModel()", "public com.yahoo.search.query.Trace getTrace()", "public com.yahoo.container.jdisc.HttpRequest getHttpRequest()", + "public java.util.Map getRequestMap()", "public java.net.URI getUri()", "public com.yahoo.search.query.SessionId getSessionId()", "public com.yahoo.search.query.SessionId getSessionId(java.lang.String)", diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index c77c99f32f2f..e85ff321b7b7 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -49,6 +49,7 @@ import java.net.URI; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -156,6 +157,9 @@ public static Type getType(String typeString) { /** The synchronous view of the JDisc request causing this query */ private final HttpRequest httpRequest; + /** Request map used to construct this query */ + private final Map requestMap; + /** The context, or null if there is no context */ private QueryContext context = null; @@ -333,6 +337,7 @@ public Query(HttpRequest request, CompiledQueryProfile queryProfile) { public Query(HttpRequest request, Map requestMap, CompiledQueryProfile queryProfile) { super(new QueryPropertyAliases(propertyAliases)); this.httpRequest = request; + this.requestMap = Collections.unmodifiableMap(requestMap); init(requestMap, queryProfile, Embedder.throwsOnUse.asMap(), ZoneInfo.defaultInfo(), SchemaInfo.empty()); } @@ -355,6 +360,7 @@ private Query(HttpRequest request, SchemaInfo schemaInfo) { super(new QueryPropertyAliases(propertyAliases)); this.httpRequest = request; + this.requestMap = Collections.unmodifiableMap(requestMap); init(requestMap, queryProfile, embedders, zoneInfo, schemaInfo); } @@ -413,6 +419,7 @@ private Query(Query query, long startTime) { super(query.properties().clone()); this.startTime = startTime; this.httpRequest = query.httpRequest; + this.requestMap = query.requestMap; query.copyPropertiesTo(this); } @@ -969,6 +976,14 @@ private void copyPropertiesTo(Query clone) { */ public HttpRequest getHttpRequest() { return httpRequest; } + /** + * Return the request map used to construct this query. + * Falls back to the HTTP request parameters if no request map was explicitly specified. + */ + public Map getRequestMap() { + return this.requestMap != null ? this.requestMap : httpRequest.propertyMap(); + } + public URI getUri() { return httpRequest != null ? httpRequest.getUri() : null; } /** Returns the session id of this query, or null if none is assigned */ diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java index 0602f4ba6964..34769d086124 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java @@ -62,7 +62,7 @@ public Result search(Query query, Execution execution) { } public static void validate(Query query) { - if (query.getHttpRequest().getProperty(GROUPING_GLOBAL_MAX_GROUPS.toString()) != null) + if (query.getRequestMap().get(GROUPING_GLOBAL_MAX_GROUPS.toString()) != null) throw new IllegalInputException(GROUPING_GLOBAL_MAX_GROUPS + " must be specified in a query profile."); } diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index 59ae86cf3ad6..2f2959f78f54 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -473,7 +473,7 @@ private void log(String request, Query query, Throwable e) { } private Result validateQuery(Query query) { - DefaultProperties.requireNotPresentIn(query.getHttpRequest().propertyMap()); + DefaultProperties.requireNotPresentIn(query.getRequestMap()); int maxHits = query.properties().getInteger(DefaultProperties.MAX_HITS); int maxOffset = query.properties().getInteger(DefaultProperties.MAX_OFFSET); diff --git a/container-search/src/test/java/com/yahoo/search/handler/SearchHandlerTest.java b/container-search/src/test/java/com/yahoo/search/handler/SearchHandlerTest.java index 78091d77c59d..892c991e0533 100644 --- a/container-search/src/test/java/com/yahoo/search/handler/SearchHandlerTest.java +++ b/container-search/src/test/java/com/yahoo/search/handler/SearchHandlerTest.java @@ -154,6 +154,36 @@ private void testInvalidQueryParam(RequestHandlerTestDriver testDriver) { assertTrue(response.contains("\"code\":" + com.yahoo.container.protect.Error.ILLEGAL_QUERY.code)); } + @Test + void testQueryProfileOnlyQueryParam() { + try (var tester = new SearchHandlerTester()) { + verifyQueryProfileOnlyResponse(sendQueryViaGet(tester.driver, "maxHits"), "maxHits"); + verifyQueryProfileOnlyResponse(sendQueryViaPost(tester.driver, "maxHits"), "maxHits"); + verifyQueryProfileOnlyResponse(sendQueryViaGet(tester.driver, "maxOffset"), "maxOffset"); + verifyQueryProfileOnlyResponse(sendQueryViaPost(tester.driver, "maxOffset"), "maxOffset"); + verifyQueryProfileOnlyResponse(sendQueryViaGet(tester.driver, "maxQueryItems"), "maxQueryItems"); + verifyQueryProfileOnlyResponse(sendQueryViaPost(tester.driver, "maxQueryItems"), "maxQueryItems"); + } + } + + private RequestHandlerTestDriver.MockResponseHandler sendQueryViaGet(RequestHandlerTestDriver testDriver, String parameter) { + return testDriver.sendRequest("http://localhost/search/?query=status_code%3A0&" + parameter + "=42"); + } + + private RequestHandlerTestDriver.MockResponseHandler sendQueryViaPost(RequestHandlerTestDriver testDriver, String parameter) { + return testDriver.sendRequest("http://localhost/search/", + com.yahoo.jdisc.http.HttpRequest.Method.POST, + "{ \"query\": \"status_code:0\", \"" + parameter + "\": 42 }", + "application/json"); + } + + private void verifyQueryProfileOnlyResponse(RequestHandlerTestDriver.MockResponseHandler responseHandler, String parameter) { + String response = responseHandler.readAll(); + assertEquals(500, responseHandler.getStatus()); + assertTrue(response.contains(parameter + " must be specified in a query profile")); + assertTrue(response.contains("\"code\":" + com.yahoo.container.protect.Error.UNSPECIFIED.code)); + } + @Test void testResultStatus() { try (var tester = new SearchHandlerTester()) { diff --git a/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java index 8fc9cc1e0b0a..32a36c86c31e 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java @@ -3,6 +3,7 @@ import com.google.common.base.Charsets; import com.yahoo.component.chain.Chain; +import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.language.Language; import com.yahoo.processing.IllegalInputException; import com.yahoo.search.Query; @@ -22,9 +23,13 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import java.io.ByteArrayInputStream; import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.net.URLEncoder; import java.util.ArrayList; +import java.util.Map; +import java.util.HashMap; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -397,13 +402,13 @@ void globalMaxGroupsIsCarriedOver() { @Test void globalMaxGroupsCannotBeSetInRequest() { + String yql = "select foo from bar where baz contains 'cox' | all(group(a) each(output(count())))"; + verifyThatQueryWithGlobalMaxGroupsProducesError(makeQueryWithGlobalMaxGroupsFromGetRequest(yql)); + verifyThatQueryWithGlobalMaxGroupsProducesError(makeQueryWithGlobalMaxGroupsFromPostRequest(yql)); + } + + void verifyThatQueryWithGlobalMaxGroupsProducesError(Query query) { try { - URIBuilder builder = new URIBuilder(); - builder.setPath("search/"); - builder.setParameter("yql", "select foo from bar where baz contains 'cox' " + - "| all(group(a) each(output(count())))"); - builder.setParameter("grouping.globalMaxGroups", "-1"); - Query query = new Query(builder.toString()); execution.search(query); fail(); } @@ -412,6 +417,28 @@ void globalMaxGroupsCannotBeSetInRequest() { } } + Query makeQueryWithGlobalMaxGroupsFromGetRequest(String yql) { + URIBuilder builder = new URIBuilder(); + builder.setPath("search/"); + builder.setParameter("yql", yql); + builder.setParameter("grouping.globalMaxGroups", "-1"); + return new Query(builder.toString()); + } + + Query makeQueryWithGlobalMaxGroupsFromPostRequest(String yql) { + String data = "{ \"yql\": \"" + yql + "\", \"grouping.globalMaxGroups\": 42 }"; + var stream = new ByteArrayInputStream(data.getBytes(StandardCharsets.UTF_8)); + var request = HttpRequest.createTestRequest("/search/", com.yahoo.jdisc.http.HttpRequest.Method.POST, stream); + + // SearchHandler extracts data from HTTP request and puts parameters into requestMap + // The point is that "grouping.globalMaxGroups" will not be a parameter of the HTTP request! + Map requestMap = new HashMap<>(request.propertyMap()); + requestMap.put("grouping.globalMaxGroups", "42"); + requestMap.put("yql", yql); + + return new Query(request, requestMap, null); + } + @Test void verifyThatWarmupIsSane() { assertTrue(MinimalQueryInserter.warmup());