Null check field names in QueryStringQueryBuilder (opensearch-project#18194)

dbwiddis · andrross · web-flow · commit 379e4643fe13 · 2025-05-05T14:09:43.000-07:00
* Null check field names in QueryStringQuerybuilder

Signed-off-by: Daniel Widdis &lt;widdis@gmail.com&gt;

* Restrict test to run on &gt;=3.1.0

Signed-off-by: Daniel Widdis &lt;widdis@gmail.com&gt;

---------

Signed-off-by: Daniel Widdis &lt;widdis@gmail.com&gt;
Signed-off-by: Andrew Ross &lt;andrross@amazon.com&gt;
Co-authored-by: Andrew Ross &lt;andrross@amazon.com&gt;
diff --git a/CHANGELOG.md b/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
 
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/50_multi_match.yml b/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" }
diff --git a/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java b/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);
diff --git a/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java b/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);
