SOLR-13187: Fix NPE when invalid qParser is specified

* When non-existent qParser is specified return 400 error code
* SOLR-13197: Fix NPE when createQParser is called in StatsField

solr/CHANGES.txt

@@ -180,6 +180,9 @@ Bug Fixes
 * SOLR-12127: Updates containing 'set' operation with value null or empty list should be considered as Atomic Update
   (Oliver Kuldmäe, Munendra S N, Ishan Chattopadhyaya)
 
+* SOLR-13187: Fix NPE when invalid query parser is specified and return 400 error code.
+  (Cesar Rodriguez, Marek, Charles Sanders, Munendra S N, Mikhail Khludnev)
+
 Other Changes
 ----------------------
 

solr/core/src/java/org/apache/solr/handler/component/StatsField.java

@@ -149,7 +149,7 @@ public class StatsField {
     }
     
     /**
-     * Given a String, returns the corrisponding Stat enum value if any, otherwise returns null.
+     * Given a String, returns the corresponding Stat enum value if any, otherwise returns null.
      */
     public static Stat forName(String paramKey) {
       try {
@@ -245,16 +245,20 @@ public class StatsField {
       } else {
         // we have a non trivial request to compute stats over a query (or function)
 
-        // NOTE we could use QParser.getParser(...) here, but that would redundently
+        // NOTE we could use QParser.getParser(...) here, but that would redundantly
         // reparse everything.  ( TODO: refactor a common method in QParser ?)
         QParserPlugin qplug = rb.req.getCore().getQueryPlugin(parserName);
+        if (qplug == null) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "invalid query parser '" + parserName +
+              (originalParam == null? "'": "' for query '" + originalParam + "'"));
+        }
         QParser qp = qplug.createParser(localParams.get(QueryParsing.V),
                                          localParams, params, rb.req);
 
         // figure out what type of query we are dealing, get the most direct ValueSource
         vs = extractValueSource(qp.parse());
 
-        // if this ValueSource directly corrisponds to a SchemaField, act as if
+        // if this ValueSource directly corresponds to a SchemaField, act as if
         // we were asked to compute stats on it directly
         // ie:  "stats.field={!func key=foo}field(foo)" == "stats.field=foo"
         sf = extractSchemaField(vs, searcher.getSchema());

solr/core/src/java/org/apache/solr/search/QParser.java

@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Map;
 
 import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
@@ -360,10 +361,16 @@ public abstract class QParser {
       }
 
       parserName = localParams.get(QueryParsing.TYPE,parserName);
-      qstr = localParams.get("v");
+      qstr = localParams.get(QueryParsing.V);
     }
 
     QParserPlugin qplug = req.getCore().getQueryPlugin(parserName);
+    if (qplug == null) {
+      // there should a way to include parameter for which parsing failed
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+          "invalid query parser '" + parserName + (stringIncludingLocalParams == null?
+              "'": "' for query '" + stringIncludingLocalParams + "'"));
+    }
     QParser parser =  qplug.createParser(qstr, localParams, req.getParams(), req);
 
     parser.stringIncludingLocalParams = stringIncludingLocalParams;

solr/core/src/test/org/apache/solr/search/QueryParsingTest.java

@@ -17,6 +17,7 @@
 package org.apache.solr.search;
 import org.apache.lucene.search.Query;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.request.SolrQueryRequest;
 import org.junit.BeforeClass;
@@ -94,4 +95,45 @@ public class QueryParsingTest extends SolrTestCaseJ4 {
                   ("strdist(\"a value\",literal('a value'),edit)",
                    NAME, req).getQuery());
   }
+
+  public void testGetQParser() throws Exception {
+    // invalid defType
+    SolrException exception = expectThrows(SolrException.class, () -> h.query(req("q", "ad", "defType", "bleh")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'bleh' for query 'ad'", exception.getMessage());
+
+    // invalid qparser override in the local params
+    exception = expectThrows(SolrException.class, () -> h.query(req("q", "{!bleh}")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'bleh' for query '{!bleh}'", exception.getMessage());
+
+    // invalid qParser with fq params
+    exception = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "fq", "{!some}")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'some' for query '{!some}'", exception.getMessage());
+
+    // invalid qparser with function queries
+    exception = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "defType", "edismax", "boost", "{!hmm}")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'hmm' for query '{!hmm}'", exception.getMessage());
+
+    exception = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "defType", "edismax", "boost", "query({!bleh v=ak})")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'bleh' for query '{!bleh v=ak}'", exception.getMessage());
+
+    exception = expectThrows(SolrException.class, () ->
+        h.query(req("q", "*:*", "defType", "edismax", "boost", "query($qq)", "qq", "{!bleh v=a}")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'bleh' for query '{!bleh v=a}'", exception.getMessage());
+
+    // ranking doesn't use defType
+    exception = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "rq", "{!bleh}")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'bleh' for query '{!bleh}'", exception.getMessage());
+
+    // with stats.field
+    exception = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "stats", "true", "stats.field", "{!bleh}")));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, exception.code());
+    assertEquals("invalid query parser 'bleh' for query '{!bleh}'", exception.getMessage());
+  }
 }