1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Fix exception overwritten for parameters fetch method

When executing an `ActionController::Parameters#fetch` with a block
that raises a `KeyError` the raised `KeyError` will be rescued and
converted to an `ActionController::ParameterMissing` exception,
covering up the original exception.

[Jonas Schubert Erlandsson & Roque Pinel]
This commit is contained in:
Roque Pinel 2015-07-18 18:40:47 -04:00
parent 3f1c5d39c0
commit 780af27bf9
3 changed files with 25 additions and 3 deletions

View file

@ -1,3 +1,8 @@
* Fix `ActionController::Parameters#fetch` overwriting `KeyError` returned by
default block.
*Jonas Schuber Erlandsson*, *Roque Pinel*
* `ActionController::Parameters` no longer inherits from * `ActionController::Parameters` no longer inherits from
`HashWithIndifferentAccess` `HashWithIndifferentAccess`

View file

@ -380,11 +380,15 @@ module ActionController
def fetch(key, *args, &block) def fetch(key, *args, &block)
convert_hashes_to_parameters( convert_hashes_to_parameters(
key, key,
@parameters.fetch(key, *args, &block), @parameters.fetch(key) {
if block_given?
yield
else
args.fetch(0) { raise ActionController::ParameterMissing.new(key) }
end
},
false false
) )
rescue KeyError
raise ActionController::ParameterMissing.new(key)
end end
# Returns a new <tt>ActionController::Parameters</tt> instance that # Returns a new <tt>ActionController::Parameters</tt> instance that

View file

@ -194,6 +194,19 @@ class ParametersPermitTest < ActiveSupport::TestCase
assert_equal "monkey", @params.fetch(:foo) { "monkey" } assert_equal "monkey", @params.fetch(:foo) { "monkey" }
end end
test "fetch doesnt raise ParameterMissing exception if there is a default that is nil" do
assert_equal nil, @params.fetch(:foo, nil)
assert_equal nil, @params.fetch(:foo) { nil }
end
test 'KeyError in fetch block should not be coverd up' do
params = ActionController::Parameters.new
e = assert_raises(KeyError) do
params.fetch(:missing_key) { {}.fetch(:also_missing) }
end
assert_match(/:also_missing$/, e.message)
end
test "not permitted is sticky beyond merges" do test "not permitted is sticky beyond merges" do
assert !@params.merge(a: "b").permitted? assert !@params.merge(a: "b").permitted?
end end