Null check field names in QueryStringQueryBuilder (#18194)

* Null check field names in QueryStringQuerybuilder

Signed-off-by: Daniel Widdis <[email protected]>

* Restrict test to run on >=3.1.0

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Andrew Ross <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>

Author: dbwiddis

CHANGELOG.md

@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix the native plugin installation error cause by the pgp public key change ([#18147](https://github.com/opensearch-project/OpenSearch/pull/18147))
 - Fix object field exists query ([#17843](https://github.com/opensearch-project/OpenSearch/pull/17843))
 - Use Bad Request status for InputCoercionEcception ([#18161](https://github.com/opensearch-project/OpenSearch/pull/18161))
+- Null check field names in QueryStringQueryBuilder ([#18194](https://github.com/opensearch-project/OpenSearch/pull/18194))
 
 ### Security
 

rest-api-spec/src/main/resources/rest-api-spec/test/search/50_multi_match.yml

@@ -1,7 +1,4 @@
-"Cross fields do not return negative scores":
-  - skip:
-      version: " - 2.99.99"
-      reason: "This fix is in 2.15. Until we do the BWC dance, we need to skip all pre-3.0, though."
+setup:
   - do:
       index:
         index: test
@@ -19,6 +16,12 @@
         body: { "color" : "orange red yellow purple" }
   - do:
       indices.refresh: { }
+
+---
+"Cross fields do not return negative scores":
+  - skip:
+      version: " - 2.14.99"
+      reason: "This fix is in 2.15.0"
   - do:
       search:
         index: test
@@ -33,3 +36,22 @@
   - match: { hits.total.value: 3 }
   - match: { hits.hits.0._id: "2" }
   - gt: { hits.hits.2._score: 0.0 }
+
+---
+"Query string with null field throws 400":
+  - skip:
+      version: " - 3.0.99"
+      reason: "This fix is in 3.1.0"
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          query:
+            query_string:
+              query: "red"
+              fields: ["color", null, "shape"]
+
+  - match: { status: 400 }
+  - match: { error.type: parsing_exception }
+  - match: { error.reason: "[query_string] field name in [fields] cannot be null" }

server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java

@@ -685,6 +685,12 @@ public static QueryStringQueryBuilder fromXContent(XContentParser parser) throws
                 if (FIELDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
                     List<String> fields = new ArrayList<>();
                     while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
+                        if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
+                            throw new ParsingException(
+                                parser.getTokenLocation(),
+                                "[" + QueryStringQueryBuilder.NAME + "] field name in [" + currentFieldName + "] cannot be null"
+                            );
+                        }
                         fields.add(parser.text());
                     }
                     fieldsAndWeights = QueryParserHelper.parseFieldsAndWeights(fields);

server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java

@@ -71,6 +71,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.Fuzziness;
 import org.opensearch.common.xcontent.json.JsonXContent;
+import org.opensearch.core.common.ParsingException;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.index.mapper.FieldNamesFieldMapper;
 import org.opensearch.index.mapper.MapperService;
@@ -1013,6 +1014,18 @@ public void testToQueryWildcardNonExistingFields() throws IOException {
         assertThat(expectedQuery, equalTo(query));
     }
 
+    public void testNullFieldInFieldsArray() throws IOException {
+        String queryAsString = "{\n"
+            + "  \"query_string\" : {\n"
+            + "    \"query\" : \"test\",\n"
+            + "    \"fields\" : [ \"field1\", null, \"field2\" ]\n"
+            + "  }\n"
+            + "}";
+
+        ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(queryAsString));
+        assertEquals("[query_string] field name in [fields] cannot be null", e.getMessage());
+    }
+
     public void testToQueryTextParsing() throws IOException {
         {
             QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo bar").field(TEXT_FIELD_NAME).field(KEYWORD_FIELD_NAME);