You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by yo...@apache.org on 2015/03/24 21:08:03 UTC
svn commit: r1668978 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/CHANGES.txt solr/core/
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
solr/core/src/test/org/apache/solr/TestDistributedSearch.java
Author: yonik
Date: Tue Mar 24 20:08:03 2015
New Revision: 1668978
URL: http://svn.apache.org/r1668978
Log:
SOLR-7254: invalid start/rows should throw 400 rather than 500 error code
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/core/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/TestDistributedSearch.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1668978&r1=1668977&r2=1668978&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Tue Mar 24 20:08:03 2015
@@ -232,6 +232,10 @@ Bug Fixes
* SOLR-7294: Migrate API fails with 'Invalid status request: notfoundretried 6times' message.
(Jessica Cheng Mallet, shalin)
+* SOLR-7254: Make an invalid negative start/rows throw a HTTP 400 error (Bad Request) instead
+ of causing a 500 error. (Ramkumar Aiyengar, Hrishikesh Gadre, yonik)
+
+
Optimizations
----------------------
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java?rev=1668978&r1=1668977&r2=1668978&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java Tue Mar 24 20:08:03 2015
@@ -216,6 +216,16 @@ public class QueryComponent extends Sear
if (params.getBool(GroupParams.GROUP, false)) {
prepareGrouping(rb);
+ } else {
+ //Validate only in case of non-grouping search.
+ if(rb.getSortSpec().getCount() < 0) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "'rows' parameter cannot be negative");
+ }
+ }
+
+ //Input validation.
+ if (rb.getQueryCommand().getOffset() < 0) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "'start' parameter cannot be negative");
}
}
@@ -306,10 +316,6 @@ public class QueryComponent extends Sear
statsCache.receiveGlobalStats(req);
}
- if (rb.getQueryCommand().getOffset() < 0) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "'start' parameter cannot be negative");
- }
-
// -1 as flag if not set.
long timeAllowed = params.getLong(CommonParams.TIME_ALLOWED, -1L);
if (null != rb.getCursorMark() && 0 < timeAllowed) {
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/TestDistributedSearch.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/TestDistributedSearch.java?rev=1668978&r1=1668977&r2=1668978&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/TestDistributedSearch.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/TestDistributedSearch.java Tue Mar 24 20:08:03 2015
@@ -19,8 +19,8 @@ package org.apache.solr;
import org.apache.commons.lang.StringUtils;
import org.apache.lucene.util.LuceneTestCase.Slow;
-
import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.embedded.JettySolrRunner;
@@ -32,6 +32,7 @@ import org.apache.solr.client.solrj.resp
import org.apache.solr.cloud.ChaosMonkey;
import org.apache.solr.common.EnumFieldValue;
import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.ShardParams;
@@ -152,6 +153,9 @@ public class TestDistributedSearch exten
handle.put("timestamp", SKIPVAL);
handle.put("_version_", SKIPVAL); // not a cloud test, but may use updateLog
+ //Test common query parameters.
+ validateCommonQueryParameters();
+
// random value sort
for (String f : fieldNames) {
query("q","*:*", "sort",f+" desc");
@@ -1087,4 +1091,24 @@ public class TestDistributedSearch exten
super.validateControlData(control);
assertNull("Expected the partialResults header to be null", control.getHeader().get("partialResults"));
}
+
+ private void validateCommonQueryParameters() throws Exception {
+ try {
+ SolrQuery query = new SolrQuery();
+ query.setStart(-1).setQuery("*");
+ QueryResponse resp = query(query);
+ fail("Expected the last query to fail, but got response: " + resp);
+ } catch (SolrException e) {
+ assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
+ }
+
+ try {
+ SolrQuery query = new SolrQuery();
+ query.setRows(-1).setStart(0).setQuery("*");
+ QueryResponse resp = query(query);
+ fail("Expected the last query to fail, but got response: " + resp);
+ } catch (SolrException e) {
+ assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
+ }
+ }
}