Permalink
Browse files

Merge pull request #115 from sebasoga/change_require_behaviour

Change behavior ActionController::Parameters#require when value is empty
  • Loading branch information...
2 parents e039055 + c6ba4f9 commit fd00868c49003456954bb81f187fd0430566beee @guilleiguaran guilleiguaran committed Nov 2, 2013
Showing with 66 additions and 16 deletions.
  1. +3 −2 README.md
  2. +27 −13 lib/action_controller/parameters.rb
  3. +21 −0 test/action_controller_required_params_test.rb
  4. +15 −1 test/parameters_require_test.rb
View
@@ -14,8 +14,9 @@ class PeopleController < ActionController::Base
end
# This will pass with flying colors as long as there's a person key in the parameters, otherwise
- # it'll raise a ActionController::MissingParameter exception, which will get caught by
- # ActionController::Base and turned into that 400 Bad Request reply.
+ # it'll raise an ActionController::ParameterMissing exception if the key is not present or
+ # it'll raise an ActionController:EmptyParameter exception if the key value is blank,
+ # which will get caught by ActionController::Base and turned into that 400 Bad Request reply.
def update
person = current_account.people.find(params[:id])
person.update_attributes!(person_params)
@@ -17,6 +17,15 @@ def initialize(param)
end
end
+ class EmptyParameter < IndexError
+ attr_reader :param
+
+ def initialize(param)
+ @param = param
+ super("value is empty for required key: #{param}")
+ end
+ end
+
class UnpermittedParameters < IndexError
attr_reader :params
@@ -29,7 +38,7 @@ def initialize(params)
class Parameters < ActiveSupport::HashWithIndifferentAccess
attr_accessor :permitted
alias :permitted? :permitted
-
+
cattr_accessor :action_on_unpermitted_parameters, :instance_accessor => false
# Never raise an UnpermittedParameters exception because of these params
@@ -52,7 +61,8 @@ def permit!
end
def require(key)
- self[key].presence || raise(ActionController::ParameterMissing.new(key))
+ raise(ActionController::ParameterMissing.new(key)) unless self.key?(key)
+ self[key].presence || raise(ActionController::EmptyParameter.new(key))
end
alias :required :require
@@ -215,23 +225,23 @@ def fields_for_style?(object)
object.is_a?(Hash) && object.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) }
end
- def unpermitted_parameters!(params)
+ def unpermitted_parameters!(params)
return unless self.class.action_on_unpermitted_parameters
-
+
unpermitted_keys = unpermitted_keys(params)
- if unpermitted_keys.any?
- case self.class.action_on_unpermitted_parameters
+ if unpermitted_keys.any?
+ case self.class.action_on_unpermitted_parameters
when :log
name = "unpermitted_parameters.action_controller"
ActiveSupport::Notifications.instrument(name, :keys => unpermitted_keys)
- when :raise
- raise ActionController::UnpermittedParameters.new(unpermitted_keys)
- end
- end
- end
-
- def unpermitted_keys(params)
+ when :raise
+ raise ActionController::UnpermittedParameters.new(unpermitted_keys)
+ end
+ end
+ end
+
+ def unpermitted_keys(params)
self.keys - params.keys - NEVER_UNPERMITTED_PARAMS
end
end
@@ -243,6 +253,10 @@ module StrongParameters
rescue_from(ActionController::ParameterMissing) do |parameter_missing_exception|
render :text => "Required parameter missing: #{parameter_missing_exception.param}", :status => :bad_request
end
+
+ rescue_from(ActionController::EmptyParameter) do |empty_parameter_exception|
+ render :text => "Required parameter value is empty: #{empty_parameter_exception.param}", :status => :bad_request
+ end
end
def params
@@ -5,6 +5,11 @@ def create
params.require(:book).require(:name)
head :ok
end
+
+ def update
+ params.require(:book)
+ head :ok
+ end
end
class ActionControllerRequiredParamsTest < ActionController::TestCase
@@ -27,4 +32,20 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase
post :create, { :magazine => { :name => "Mjallo!" } }
assert_equal "Required parameter missing: book", response.body
end
+
+ test "empty required parameters will raise an exception" do
+ put :update, { :book => {} }
+ assert_response :bad_request
+
+ put :update, { :book => '' }
+ assert_response :bad_request
+
+ put :update, { :book => nil }
+ assert_response :bad_request
+ end
+
+ test "empty parameters will be mentioned in the return" do
+ put :update, { :book => {} }
+ assert_equal "Required parameter value is empty: book", response.body
+ end
end
@@ -2,9 +2,23 @@
require 'action_controller/parameters'
class ParametersRequireTest < ActiveSupport::TestCase
- test "required parameters must be present not merely not nil" do
+ test "required parameters must be present" do
assert_raises(ActionController::ParameterMissing) do
+ ActionController::Parameters.new(:name => {}).require(:person)
+ end
+ end
+
+ test "required parameters can't be blank" do
+ assert_raises(ActionController::EmptyParameter) do
ActionController::Parameters.new(:person => {}).require(:person)
end
+
+ assert_raises(ActionController::EmptyParameter) do
+ ActionController::Parameters.new(:person => '').require(:person)
+ end
+
+ assert_raises(ActionController::EmptyParameter) do
+ ActionController::Parameters.new(:person => nil).require(:person)
+ end
end
end

0 comments on commit fd00868

Please sign in to comment.