Merge pull request #305 from projectblacklight/bad_range_limit_params

bad query params to #range_limit action should not result in uncaught exception

lib/blacklight_range_limit/range_limit_builder.rb

@@ -48,6 +48,12 @@ def add_range_limit_params(solr_params)
     # range_field, range_start, range_end
     def fetch_specific_range_limit(solr_params)
       field_key = blacklight_params[:range_field] # what field to fetch for
+
+      unless  blacklight_params[:range_start].present? && blacklight_params[:range_start].kind_of?(String) &&
+              blacklight_params[:range_end].present? && blacklight_params[:range_end].kind_of?(String)
+        raise BlacklightRangeLimit::InvalidRange
+      end
+
       start = blacklight_params[:range_start].to_i
       finish = blacklight_params[:range_end].to_i
 
@@ -61,6 +67,9 @@ def fetch_specific_range_limit(solr_params)
       solr_params[:rows] = 0
 
       return solr_params
+    rescue BlacklightRangeLimit::InvalidRange
+      # This will make Rails return a 400
+      raise ActionController::BadRequest, "invalid range_start (#{blacklight_params[:range_start]}) or range_end (#{blacklight_params[:range_end]})"
     end
 
     # hacky polyfill for new Blacklight behavior we need, if we don't have it yet

spec/controllers/range_limit_action_method_spec.rb

@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+RSpec.describe CatalogController,  type: :controller do
+  # Note that ActionController::BadRequest is caught by rails and turned into a 400
+  # response, and ActionController::RoutingError is caught by raisl and turned into 404
+  describe "bad params" do
+    let (:facet_field) { "pub_date_si" }
+
+    it "without start param present raise BadRequest " do
+      expect {
+        get :range_limit, params: {
+          "range_field"=> facet_field,
+          "range_start"=>"1931"
+        }
+      }.to raise_error(ActionController::BadRequest)
+    end
+
+    it "without end param raise BadRequest " do
+      expect {
+        get :range_limit, params: {
+          "range_field"=> facet_field,
+          "range_start"=>"1931"
+        }
+      }.to raise_error(ActionController::BadRequest)
+    end
+
+    it "without either boundary raise BadRequest" do
+      expect {
+        get :range_limit, params: {
+          "range_field"=> facet_field,
+        }
+      }.to raise_error(ActionController::BadRequest)
+    end
+
+    it "without a range_field raise RoutingError" do
+      expect {
+        get :range_limit, params: {}
+      }.to raise_error(ActionController::RoutingError)
+    end
+
+    it "with params out of order raise BadRequest"  do
+      expect {
+        get :range_limit, params: {
+          "range_field"=> facet_field,
+          "range_start"=>"1940",
+          "range_end"=>"1930"
+        }
+      }.to raise_error(ActionController::BadRequest)
+    end
+
+    it "with one of the params is an array raise BadRequest" do
+      expect {
+        get :range_limit, params: {
+          "range_field"=> facet_field,
+          "range_start"=>"1931",
+          "range_end"=>["1940"]
+        }
+      }.to raise_error(ActionController::BadRequest)
+    end
+  end
+end