From b17d630e509adc9e62b07c599583558e715a869f Mon Sep 17 00:00:00 2001
From: Chris Hennick <4961925+Pr0methean@users.noreply.github.com>
Date: Fri, 1 Nov 2019 10:40:57 -0700
Subject: [PATCH] SOLR-13207: Handle query errors in calculateMinShouldMatch
 (#978)

Traps error that arises when the < operator is used at the end of a query field.
Also handles NumberFormatException when the operand isn't a number.
---
 solr/CHANGES.txt                              |  3 ++
 .../org/apache/solr/util/SolrPluginUtils.java | 30 +++++++++++++++++--
 .../apache/solr/util/SolrPluginUtilsTest.java | 10 +++++++
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 43508962fc5..6c56b464d3e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -145,6 +145,9 @@ Bug Fixes
 * SOLR-13823: Fix ClassCastEx when score is requested with group.query. This also fixes score not being generated
   for distributed group.query case. (Uwe Jäger, Munendra S N)
 
+* SOLR-13207: In dismax and edismax "minimum should match" queries, the < operator not followed by a number will now return
+  an HTTP 400 error rather than 500, and the error message will explain that < must be followed by a number. (Chris Hennick)
+
 Other Changes
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
index c8f7f73b85f..c4ad454cf1f 100644
--- a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
@@ -678,7 +678,11 @@ public class SolrPluginUtils {
       spec = spaceAroundLessThanPattern.matcher(spec).replaceAll("<");
       for (String s : spacePattern.split(spec)) {
         String[] parts = lessThanPattern.split(s,0);
-        int upperBound = Integer.parseInt(parts[0]);
+        if (parts.length == 0) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Operator < must be followed by a number");
+        }
+        int upperBound = checkedParseInt(parts[0], "Operator < must be followed by a number");
         if (optionalClauseCount <= upperBound) {
           return result;
         } else {
@@ -694,11 +698,12 @@ public class SolrPluginUtils {
     if (-1 < spec.indexOf('%')) {
       /* percentage - assume the % was the last char.  If not, let Integer.parseInt fail. */
       spec = spec.substring(0,spec.length()-1);
-      int percent = Integer.parseInt(spec);
+      int percent = checkedParseInt(spec,
+          "% must be preceded by a number and not combined with other operators");
       float calc = (result * percent) * (1/100f);
       result = calc < 0 ? result + (int)calc : (int)calc;
     } else {
-      int calc = Integer.parseInt(spec);
+      int calc = checkedParseInt(spec, "Input should be a number");
       result = calc < 0 ? result + calc : calc;
     }
 
@@ -707,6 +712,25 @@ public class SolrPluginUtils {
 
   }
 
+  /**
+   * Wrapper of {@link Integer#parseInt(String)} that wraps any {@link NumberFormatException} in a
+   * {@link SolrException} with HTTP 400 Bad Request status.
+   *
+   * @param input the string to parse
+   * @param errorMessage the error message for any SolrException
+   * @return the integer value of {@code input}
+   * @throws SolrException when parseInt throws NumberFormatException
+   */
+  private static int checkedParseInt(String input, String errorMessage) {
+    int percent;
+    try {
+      percent = Integer.parseInt(input);
+    } catch (NumberFormatException e) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, errorMessage, e);
+    }
+    return percent;
+  }
+
 
   /**
    * Recursively walks the "from" query pulling out sub-queries and
diff --git a/solr/core/src/test/org/apache/solr/util/SolrPluginUtilsTest.java b/solr/core/src/test/org/apache/solr/util/SolrPluginUtilsTest.java
index 4f50cd03175..58d56a171b7 100644
--- a/solr/core/src/test/org/apache/solr/util/SolrPluginUtilsTest.java
+++ b/solr/core/src/test/org/apache/solr/util/SolrPluginUtilsTest.java
@@ -33,6 +33,7 @@ import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.search.QParser;
 import org.apache.solr.util.SolrPluginUtils.DisjunctionMaxQueryParser;
@@ -338,6 +339,15 @@ public class SolrPluginUtilsTest extends SolrTestCaseJ4 {
         
   }
 
+  @Test
+  public void testMinShouldMatchBadQueries() {
+    expectThrows(SolrException.class, () -> calcMSM(1, "1<"));
+    expectThrows(SolrException.class, () -> calcMSM(1, "1<x"));
+    expectThrows(SolrException.class, () -> calcMSM(1, "x%"));
+    expectThrows(SolrException.class, () -> calcMSM(1, "%%"));
+    expectThrows(SolrException.class, () -> calcMSM(1, "x"));
+  }
+
   @Test
   public void testMinShouldMatchAutoRelax() {
     /* The basics should not be affected by autoRelax */