Skip to content
Browse files

Merge pull request #9660 from sebasoga/change_strong_parameters_requi…

…re_behaviour

Change ActionController::Parameters#require behavior when value is empty
  • Loading branch information...
2 parents 1918b12 + b3f894c commit c2b5a8e61ba0f35015e6ac949a5c8fce2042a1f2 @guilleiguaran guilleiguaran committed Nov 1, 2013
View
32 actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -10,8 +10,6 @@ module ActionController
# params = ActionController::Parameters.new(a: {})
# params.fetch(:b)
# # => ActionController::ParameterMissing: param not found: b
- # params.require(:a)
- # # => ActionController::ParameterMissing: param not found: a
class ParameterMissing < KeyError
attr_reader :param # :nodoc:
@@ -21,6 +19,20 @@ def initialize(param) # :nodoc:
end
end
+ # Raised when a required parameter value is empty.
+ #
+ # params = ActionController::Parameters.new(a: {})
+ # params.require(:a)
+ # # => ActionController::EmptyParameter: value is empty for required key: a
+ class EmptyParameter < IndexError
+ attr_reader :param
+
+ def initialize(param)
+ @param = param
+ super("value is empty for required key: #{param}")
+ end
+ end
+
# Raised when a supplied parameter is not expected.
#
# params = ActionController::Parameters.new(a: "123", b: "456")
@@ -157,20 +169,22 @@ def permit!
self
end
- # Ensures that a parameter is present. If it's present, returns
- # the parameter at the given +key+, otherwise raises an
- # <tt>ActionController::ParameterMissing</tt> error.
+ # Ensures that a parameter is present. If it's present and not empty,
+ # returns the parameter at the given +key+, if it's empty raises
+ # an <tt>ActionController::EmptyParameter</tt> error, otherwise
+ # raises an <tt>ActionController::ParameterMissing</tt> error.
#
# ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person)
# # => {"name"=>"Francesco"}
#
- # ActionController::Parameters.new(person: nil).require(:person)
- # # => ActionController::ParameterMissing: param not found: person
- #
# ActionController::Parameters.new(person: {}).require(:person)
+ # # => ActionController::EmptyParameter: value is empty for required key: person
+ #
+ # ActionController::Parameters.new(name: {}).require(:person)
# # => ActionController::ParameterMissing: param not found: person
def require(key)
- self[key].presence || raise(ParameterMissing.new(key))
+ raise(ActionController::ParameterMissing.new(key)) unless self.key?(key)
+ self[key].presence || raise(ActionController::EmptyParameter.new(key))
end
# Alias of #require.
View
3 actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
@@ -15,7 +15,8 @@ class ExceptionWrapper
'ActionController::InvalidAuthenticityToken' => :unprocessable_entity,
'ActionDispatch::ParamsParser::ParseError' => :bad_request,
'ActionController::BadRequest' => :bad_request,
- 'ActionController::ParameterMissing' => :bad_request
+ 'ActionController::ParameterMissing' => :bad_request,
+ 'ActionController::EmptyParameter' => :bad_request
)
cattr_accessor :rescue_templates
View
8 actionpack/test/controller/parameters/parameters_require_test.rb
@@ -2,8 +2,14 @@
require 'action_controller/metal/strong_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
end
View
19 actionpack/test/controller/required_params_test.rb
@@ -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
@@ -20,6 +25,20 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase
end
end
+ test "empty required parameters will raise an exception" do
+ assert_raise ActionController::EmptyParameter do
+ put :update, {book: {}}
+ end
+
+ assert_raise ActionController::EmptyParameter do
+ put :update, {book: ''}
+ end
+
+ assert_raise ActionController::EmptyParameter do
+ put :update, {book: nil}
+ end
+ end
+
test "required parameters that are present will not raise" do
post :create, { book: { name: "Mjallo!" } }
assert_response :ok
View
6 actionpack/test/dispatch/debug_exceptions_test.rb
@@ -43,6 +43,8 @@ def call(env)
raise ActionController::UrlGenerationError, "No route matches"
when "/parameter_missing"
raise ActionController::ParameterMissing, :missing_param_key
+ when "/required_key_empty_value"
+ raise ActionController::EmptyParameter, :empty_param_key
else
raise "puke!"
end
@@ -126,6 +128,10 @@ def setup
get "/parameter_missing", {}, {'action_dispatch.show_exceptions' => true}
assert_response 400
assert_match(/ActionController::ParameterMissing/, body)
+
+ get "/required_key_empty_value", {}, {'action_dispatch.show_exceptions' => true}
+ assert_response 400
+ assert_match(/ActionController::EmptyParameter/, body)
end
test "rescue with text error for xhr request" do

0 comments on commit c2b5a8e

Please sign in to comment.
Something went wrong with that request. Please try again.