Fix validation of queries sent via HTTP POST

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)",

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<String, String> 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<String, String> 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<String, String> 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 */

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.");
     }
 

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);

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()) {

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<String, String> 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());