diff options
-rw-r--r-- | actionpack/CHANGELOG.md | 5 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/strong_parameters.rb | 10 | ||||
-rw-r--r-- | actionpack/test/controller/parameters/parameters_permit_test.rb | 13 |
3 files changed, 25 insertions, 3 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4b081591c0..6a68c057ac 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -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 `HashWithIndifferentAccess` diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 06d625e4d5..cf6a64009f 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -380,11 +380,15 @@ module ActionController def fetch(key, *args, &block) convert_hashes_to_parameters( key, - @parameters.fetch(key, *args, &block), + @parameters.fetch(key) { + if block_given? + yield + else + args.fetch(0) { raise ActionController::ParameterMissing.new(key) } + end + }, false ) - rescue KeyError - raise ActionController::ParameterMissing.new(key) end # Returns a new <tt>ActionController::Parameters</tt> instance that diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 05532ec21b..2dd2826196 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -194,6 +194,19 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "monkey", @params.fetch(:foo) { "monkey" } 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 assert !@params.merge(a: "b").permitted? end |