Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,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
---------------------

Expand Down
30 changes: 27 additions & 3 deletions solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,11 @@ static int calculateMinShouldMatch(int optionalClauseCount, String spec) {
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 {
Expand All @@ -694,11 +698,12 @@ static int calculateMinShouldMatch(int optionalClauseCount, String spec) {
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;
}

Expand All @@ -707,6 +712,25 @@ static int calculateMinShouldMatch(int optionalClauseCount, String spec) {

}

/**
* 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
Expand Down
10 changes: 10 additions & 0 deletions solr/core/src/test/org/apache/solr/util/SolrPluginUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
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;
Expand Down Expand Up @@ -338,6 +339,15 @@ public void testMinShouldMatchCalculator() {

}

@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 */
Expand Down