diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 9900f4a37c6..ef919a8ee8b 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -134,6 +134,10 @@ Bug Fixes * SOLR-3623: Fixed inconsistent treatment of third-party dependencies for solr contribs analysis-extras & uima (hossman) +* SOLR-3652: Fixed range faceting to error instead of looping infinitely + when 'gap' is zero -- or effectively zero due to floating point arithmetic + underflow. (hossman) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java b/solr/core/src/java/org/apache/solr/request/SimpleFacets.java index a1e9823a48e..f25e7aa4182 100644 --- a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java +++ b/solr/core/src/java/org/apache/solr/request/SimpleFacets.java @@ -924,6 +924,11 @@ public class SimpleFacets { (SolrException.ErrorCode.BAD_REQUEST, "date facet infinite loop (is gap negative?)"); } + if (high.equals(low)) { + throw new SolrException + (SolrException.ErrorCode.BAD_REQUEST, + "date facet infinite loop: gap is effectively zero"); + } final boolean includeLower = (include.contains(FacetRangeInclude.LOWER) || (include.contains(FacetRangeInclude.EDGE) && low.equals(start))); @@ -1113,6 +1118,11 @@ public class SimpleFacets { (SolrException.ErrorCode.BAD_REQUEST, "range facet infinite loop (is gap negative? did the math overflow?)"); } + if (high.compareTo(low) == 0) { + throw new SolrException + (SolrException.ErrorCode.BAD_REQUEST, + "range facet infinite loop: gap is either zero, or too small relative start/end and caused underflow: " + low + " + " + gap + " = " + high ); + } final boolean includeLower = (include.contains(FacetRangeInclude.LOWER) || diff --git a/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java b/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java index 5ea20ee9804..e251ed6398f 100644 --- a/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java +++ b/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java @@ -2004,4 +2004,61 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 { ,"*[count(//lst[@name='facet_fields']/lst/int)=0]" ); } + + /** + * kind of an absurd tests because if there is an inifnite loop, it + * would ver finish -- but at least it ensures that if one of + * these requests return, they return an error + */ + public void testRangeFacetInfiniteLoopDetection() { + + for (String field : new String[] {"foo_f", "foo_sf", + "foo_d", "foo_sd", + "foo_i", "foo_si"}) { + assertQEx("no zero gap error: " + field, + req("q", "*:*", + "facet", "true", + "facet.range", field, + "facet.range.start", "23", + "facet.range.gap", "0", + "facet.range.end", "100"), + 400); + } + for (String field : new String[] {"foo_pdt", "foo_dt"}) { + for (String type : new String[] {"date", "range"}) { + assertQEx("no zero gap error for facet." + type + ": " + field, + req("q", "*:*", + "facet", "true", + "facet." + type, field, + "facet."+type+".start", "NOW", + "facet."+type+".gap", "+0DAYS", + "facet."+type+".end", "NOW+10DAY"), + 400); + } + } + + for (String field : new String[] {"foo_f", "foo_sf"}) { + assertQEx("no float underflow error: " + field, + req("q", "*:*", + "facet", "true", + "facet.range", field, + "facet.range.start", "100000000000", + "facet.range.end", "100000086200", + "facet.range.gap", "2160"), + 400); + } + + for (String field : new String[] {"foo_d", "foo_sd"}) { + assertQEx("no double underflow error: " + field, + req("q", "*:*", + "facet", "true", + "facet.range", field, + "facet.range.start", "9900000000000", + "facet.range.end", "9900000086200", + "facet.range.gap", "0.0003"), + 400); + } + + } + }