Apache/lucene
1
0
Fork 0
mirror of https://github.com/apache/lucene.git synced 2025-02-14 14:05:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

SOLR-11883: reporting syntax errors as Bad Request instead of 500.

Browse Source
This commit is contained in:
Mikhail Khludnev 2019-02-23 23:46:48 +03:00
parent 4aa0645ea6
commit d68e992d03
3 changed files with 93 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
solr/CHANGES.txt
View File

@ -53,6 +53,9 @@ Bug Fixes
* SOLR-12708: Async collection actions should not hide internal failures (Mano Kovacs, Varun Thacker, Tomás Fernández Löbbe)
* SOLR-11883: 500 code on functional query syntax errors and parameter dereferencing errors
(Munendra S N via Mikhail Khludnev)
Improvements
----------------------
* SOLR-12999: Index replication could delete segments before downloading segments from master if there is not enough

60
solr/core/src/java/org/apache/solr/search/FunctionQParser.java
View File

@ -16,9 +16,16 @@
*/
package org.apache.solr.search;
import java.util.ArrayList;
import java.util.List;
import org.apache.lucene.queries.function.FunctionQuery;
import org.apache.lucene.queries.function.ValueSource;
import org.apache.lucene.queries.function.valuesource.*;
import org.apache.lucene.queries.function.valuesource.ConstValueSource;
import org.apache.lucene.queries.function.valuesource.DoubleConstValueSource;
import org.apache.lucene.queries.function.valuesource.LiteralValueSource;
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
import org.apache.lucene.queries.function.valuesource.VectorValueSource;
import org.apache.lucene.search.Query;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
@ -27,9 +34,6 @@ import org.apache.solr.schema.SchemaField;
import org.apache.solr.search.facet.AggValueSource;
import org.apache.solr.search.function.FieldNameValueSource;
import java.util.ArrayList;
import java.util.List;
public class FunctionQParser extends QParser {
public static final int FLAG_CONSUME_DELIMITER = 0x01; // consume delimiter after parsing arg
@ -130,7 +134,11 @@ public class FunctionQParser extends QParser {
*/
public String parseId() throws SyntaxError {
String value = parseArg();
if (argWasQuoted) throw new SyntaxError("Expected identifier instead of quoted string:" + value);
if (argWasQuoted()) {
throw new SyntaxError("Expected identifier instead of quoted string:" + value);
} else if (value == null) {
throw new SyntaxError("Expected identifier instead of 'null' for function " + sp);
}
return value;
}
@ -142,8 +150,11 @@ public class FunctionQParser extends QParser {
public Float parseFloat() throws SyntaxError {
String str = parseArg();
if (argWasQuoted()) throw new SyntaxError("Expected float instead of quoted string:" + str);
float value = Float.parseFloat(str);
return value;
try {
return Float.parseFloat(str);
} catch (NumberFormatException | NullPointerException e) {
throw new SyntaxError("Expected float instead of '" + str + "' for function " + sp);
}
}
/**
@ -153,8 +164,11 @@ public class FunctionQParser extends QParser {
public double parseDouble() throws SyntaxError {
String str = parseArg();
if (argWasQuoted()) throw new SyntaxError("Expected double instead of quoted string:" + str);
double value = Double.parseDouble(str);
return value;
try {
return Double.parseDouble(str);
} catch (NumberFormatException | NullPointerException e) {
throw new SyntaxError("Expected double instead of '" + str + "' for function " + sp);
}
}
/**
@ -163,9 +177,12 @@ public class FunctionQParser extends QParser {
*/
public int parseInt() throws SyntaxError {
String str = parseArg();
if (argWasQuoted()) throw new SyntaxError("Expected double instead of quoted string:" + str);
int value = Integer.parseInt(str);
return value;
if (argWasQuoted()) throw new SyntaxError("Expected integer instead of quoted string:" + str);
try {
return Integer.parseInt(str);
} catch (NumberFormatException | NullPointerException e) {
throw new SyntaxError("Expected integer instead of '" + str + "' for function " + sp);
}
}
@ -248,6 +265,12 @@ public class FunctionQParser extends QParser {
String qstr = getParam(param);
qstr = qstr==null ? "" : qstr;
nestedQuery = subQuery(qstr, null).getQuery();
// nestedQuery would be null when de-referenced query value is not specified
// Ex: query($qq) in request with no qq param specified
if (nestedQuery == null) {
throw new SyntaxError("Missing param " + param + " while parsing function '" + sp.val + "'");
}
}
else {
int start = sp.pos;
@ -277,9 +300,14 @@ public class FunctionQParser extends QParser {
sp.pos += end-start; // advance past nested query
nestedQuery = sub.getQuery();
// handling null check on nestedQuery separately, so that proper error can be returned
// one case this would be possible when v is specified but v's value is empty or has only spaces
if (nestedQuery == null) {
throw new SyntaxError("Nested function query returned null for '" + sp.val + "'");
}
}
consumeArgumentDelimiter();
return nestedQuery;
}
@ -369,8 +397,7 @@ public class FunctionQParser extends QParser {
}
valueSource = argParser.parse(this);
sp.expect(")");
}
else {
} else {
if ("true".equals(id)) {
valueSource = new BoolConstValueSource(true);
} else if ("false".equals(id)) {
@ -413,14 +440,13 @@ public class FunctionQParser extends QParser {
hasParen = sp.opt("(");
ValueSourceParser argParser = req.getCore().getValueSourceParser(id);
argParser = req.getCore().getValueSourceParser(id);
if (argParser == null) {
throw new SyntaxError("Unknown aggregation " + id + " in (" + sp + ")");
}
ValueSource vv = argParser.parse(this);
if (!(vv instanceof AggValueSource)) {
if (argParser == null) {
if (argParser == null) { // why this??
throw new SyntaxError("Expected aggregation from " + id + " but got (" + vv + ") in (" + sp + ")");
}
}

47
solr/core/src/test/org/apache/solr/search/function/TestFunctionQuery.java
View File

@ -423,6 +423,53 @@ public class TestFunctionQuery extends SolrTestCaseJ4 {
,"*//doc[1]/str[.='120']"
,"*//doc[2]/str[.='121']"
);
// test a query that doesn't specify nested query val
assertQEx("Should fail because of missing qq",
"Missing param qq while parsing function 'query($qq)'",
req("q", "*:*", "fq","id:120 OR id:121", "defType","edismax", "boost","query($qq)"),
SolrException.ErrorCode.BAD_REQUEST
);
assertQEx("Should fail because of missing sortfunc in sort",
"Can't determine a Sort Order (asc or desc) in sort spec '{!func v=$sortfunc} desc'",
req("q", "*:*", "fq","id:120 OR id:121", "sort","{!func v=$sortfunc} desc", "sortfunc","query($qq)"),
SolrException.ErrorCode.BAD_REQUEST
);
assertQEx("Should fail because of missing qq in boost",
"Nested local params must have value in v parameter. got 'query({!dismax v=$qq})",
req("q", "*:*", "fq","id:120 OR id:121", "defType","edismax", "boost","query({!dismax v=$qq})"),
SolrException.ErrorCode.BAD_REQUEST
);
assertQEx("Should fail as empty value is specified for v",
"Nested function query returned null for 'query({!v=})'",
req("q", "*:*", "defType","edismax", "boost","query({!v=})"), SolrException.ErrorCode.BAD_REQUEST
);
assertQEx("Should fail as v's value contains only spaces",
"Nested function query returned null for 'query({!v= })'",
req("q", "*:*", "defType","edismax", "boost","query({!v= })"), SolrException.ErrorCode.BAD_REQUEST
);
// no field specified in ord()
assertQEx("Should fail as no field is specified in ord func",
"Expected identifier instead of 'null' for function 'ord()'",
req("q", "*:*", "defType","edismax","boost","ord()"), SolrException.ErrorCode.BAD_REQUEST
);
assertQEx("Should fail as no field is specified in rord func",
"Expected identifier instead of 'null' for function 'rord()'",
req("q", "*:*", "defType","edismax","boost","rord()"), SolrException.ErrorCode.BAD_REQUEST
);
// test parseFloat
assertQEx("Should fail as less args are specified for recip func",
"Expected float instead of 'null' for function 'recip(1,2)'",
req("q", "*:*","defType","edismax", "boost","recip(1,2)"), SolrException.ErrorCode.BAD_REQUEST
);
assertQEx("Should fail as invalid value is specified for recip func",
"Expected float instead of 'f' for function 'recip(1,2,3,f)'",
req("q", "*:*","defType","edismax", "boost","recip(1,2,3,f)"), SolrException.ErrorCode.BAD_REQUEST
);
// this should pass
assertQ(req("q", "*:*","defType","edismax", "boost","recip(1, 2, 3, 4)"));
}
@Test