lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gerlowsk...@apache.org
Subject lucene-solr:branch_7x: SOLR-12427: Correct status for invalid 'start', 'rows'
Date Sat, 30 Jun 2018 19:07:08 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_7x 8afc12357 -> 9a395f83c


SOLR-12427: Correct status for invalid 'start', 'rows'

Prior to this commit we correctly handled negative start/rows param
values by returning a 400 (BAD REQUEST) with an appropriate error
message, but would return an ugly 500 with stack trace for non-numeric
input values.  This commit corrects this later case to also return
a 400 status code with a nicer error message.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/9a395f83
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/9a395f83
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/9a395f83

Branch: refs/heads/branch_7x
Commit: 9a395f83ccd83bca568056f178757dd032007140
Parents: 8afc123
Author: Jason Gerlowski <gerlowskija@apache.org>
Authored: Fri Jun 29 19:58:37 2018 -0400
Committer: Jason Gerlowski <gerlowskija@apache.org>
Committed: Sat Jun 30 15:06:49 2018 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 ++
 .../java/org/apache/solr/search/QParser.java    | 30 +++++++++++---------
 .../org/apache/solr/TestDistributedSearch.java  | 19 +++++++++++++
 3 files changed, 38 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9a395f83/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0185057..3f3e449 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -98,6 +98,8 @@ Bug Fixes
 * SOLR-12326: JSON Facet API: terms facet shard requests now indicate if they have more buckets
to prevent
   unnecessary refinement requests. (yonk)
 
+* SOLR-12427: Improve error message for invalid 'start', 'rows' parameters. (Munendra S N
via Jason Gerlowski)
+
 Optimizations
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9a395f83/solr/core/src/java/org/apache/solr/search/QParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/QParser.java b/solr/core/src/java/org/apache/solr/search/QParser.java
index 54b84e7..8a89a70 100644
--- a/solr/core/src/java/org/apache/solr/search/QParser.java
+++ b/solr/core/src/java/org/apache/solr/search/QParser.java
@@ -16,6 +16,12 @@
  */
 package org.apache.solr.search;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.lucene.search.Query;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
@@ -24,8 +30,6 @@ import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.request.SolrQueryRequest;
 
-import java.util.*;
-
 /**
  * <b>Note: This API is experimental and may change in non backward-compatible ways
in the future</b>
  * 
@@ -244,16 +248,16 @@ public abstract class QParser {
     getQuery(); // ensure query is parsed first
 
     String sortStr = null;
-    String startS = null;
-    String rowsS = null;
+    Integer start = null;
+    Integer rows = null;
 
     if (localParams != null) {
       sortStr = localParams.get(CommonParams.SORT);
-      startS = localParams.get(CommonParams.START);
-      rowsS = localParams.get(CommonParams.ROWS);
+      start = localParams.getInt(CommonParams.START);
+      rows = localParams.getInt(CommonParams.ROWS);
 
       // if any of these parameters are present, don't go back to the global params
-      if (sortStr != null || startS != null || rowsS != null) {
+      if (sortStr != null || start != null || rows != null) {
         useGlobalParams = false;
       }
     }
@@ -262,16 +266,16 @@ public abstract class QParser {
       if (sortStr ==null) {
           sortStr = params.get(CommonParams.SORT);
       }
-      if (startS==null) {
-        startS = params.get(CommonParams.START);
+      if (start == null) {
+        start = params.getInt(CommonParams.START);
       }
-      if (rowsS==null) {
-        rowsS = params.get(CommonParams.ROWS);
+      if (rows == null) {
+        rows = params.getInt(CommonParams.ROWS);
       }
     }
 
-    int start = startS != null ? Integer.parseInt(startS) : CommonParams.START_DEFAULT;
-    int rows = rowsS != null ? Integer.parseInt(rowsS) : CommonParams.ROWS_DEFAULT;
+    start = start != null ? start : CommonParams.START_DEFAULT;
+    rows = rows != null ? rows : CommonParams.ROWS_DEFAULT;
 
     SortSpec sort = SortSpecParsing.parseSortSpec(sortStr, req);
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9a395f83/solr/core/src/test/org/apache/solr/TestDistributedSearch.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/TestDistributedSearch.java b/solr/core/src/test/org/apache/solr/TestDistributedSearch.java
index e0d4951..d4ad79d 100644
--- a/solr/core/src/test/org/apache/solr/TestDistributedSearch.java
+++ b/solr/core/src/test/org/apache/solr/TestDistributedSearch.java
@@ -1211,6 +1211,16 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase
{
 
   private void validateCommonQueryParameters() throws Exception {
     ignoreException("parameter cannot be negative");
+
+    try {
+      SolrQuery query = new SolrQuery();
+      query.setParam("start", "non_numeric_value").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.setStart(-1).setQuery("*");
@@ -1228,6 +1238,15 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase
{
     } catch (SolrException e) {
       assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
     }
+
+    try {
+      SolrQuery query = new SolrQuery();
+      query.setParam("rows", "non_numeric_value").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());
+    }
     resetExceptionIgnores();
   }
 }


Mime
View raw message