Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change ActionController::Parameters#require behavior when value is empty #9660

Merged
merged 1 commit into from Nov 2, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 23 additions & 9 deletions actionpack/lib/action_controller/metal/strong_parameters.rb
Expand Up @@ -9,8 +9,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:

Expand All @@ -20,6 +18,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")
Expand Down Expand Up @@ -154,20 +166,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.
Expand Down
Expand Up @@ -13,7 +13,8 @@ class ExceptionWrapper
'ActionController::UnknownFormat' => :not_acceptable,
'ActionController::InvalidAuthenticityToken' => :unprocessable_entity,
'ActionController::BadRequest' => :bad_request,
'ActionController::ParameterMissing' => :bad_request
'ActionController::ParameterMissing' => :bad_request,
'ActionController::EmptyParameter' => :bad_request
)

cattr_accessor :rescue_templates
Expand Down
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions actionpack/test/controller/required_params_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions actionpack/test/dispatch/debug_exceptions_test.rb
Expand Up @@ -41,6 +41,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
Expand Down Expand Up @@ -120,6 +122,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 "does not show filtered parameters" do
Expand Down