From 5109740c6be67047df56feb164012c3a1a3c619b Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Fri, 25 Jul 2014 12:00:14 -0400 Subject: Make `AC::Params#to_h` return Hash with safe keys `ActionController::Parameters#to_h` now returns a `Hash` with unpermitted keys removed. This change is to reflect on a security concern where some method performed on an `ActionController::Parameters` may yield a `Hash` object which does not maintain `permitted?` status. If you would like to get a `Hash` with all the keys intact, duplicate and mark it as permitted before calling `#to_h`. params = ActionController::Parameters.new(name: 'Senjougahara Hitagi') params.to_h # => {} unsafe_params = params.dup.permit! unsafe_params.to_h # => {"name"=>"Senjougahara Hitagi"} safe_params = params.permit(:name) safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} This change is consider a stopgap as we cannot chage the code to stop `ActionController::Parameters` to inherit from `HashWithIndifferentAccess` in the next minor release. Also, adding a CHANGELOG entry to mention that `ActionController::Parameters` will not inheriting from `HashWithIndifferentAccess` in the next major version. --- actionpack/CHANGELOG.md | 35 +++++++++++++++++++ .../action_controller/metal/strong_parameters.rb | 19 +++++++++++ .../parameters/parameters_permit_test.rb | 39 ++++++++++++++++++++++ 3 files changed, 93 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1c84bac3ff..e2731d0ee5 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,38 @@ +* `ActionController::Parameters` will stop inheriting from `Hash` and + `HashWithIndifferentAccess` in the next major release. If you use any method + that is not available on `ActionController::Parameters` you should consider + calling `#to_h` to convert it to a `Hash` first before calling that method. + + *Prem Sichanugrist* + +* `ActionController::Parameters#to_h` now returns a `Hash` with unpermitted + keys removed. This change is to reflect on a security concern where some + method performed on an `ActionController::Parameters` may yield a `Hash` + object which does not maintain `permitted?` status. If you would like to + get a `Hash` with all the keys intact, duplicate and mark it as permitted + before calling `#to_h`. + + params = ActionController::Parameters.new({ + name: 'Senjougahara Hitagi', + oddity: 'Heavy stone crab' + }) + params.to_h + # => {} + + unsafe_params = params.dup.permit! + unsafe_params.to_h + # => {"name"=>"Senjougahara Hitagi", "oddity"=>"Heavy stone crab"} + + safe_params = params.permit(:name) + safe_params.to_h + # => {"name"=>"Senjougahara Hitagi"} + + This change is consider a stopgap as we cannot change the code to stop + `ActionController::Parameters` to inherit from `HashWithIndifferentAccess` + in the next minor release. + + *Prem Sichanugrist* + * Deprecated TagAssertions. *Kasper Timm Hansen* diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index bc27ecaa20..764474ad4e 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -141,6 +141,25 @@ module ActionController @permitted = self.class.permit_all_parameters end + # Returns a safe +Hash+ representation of this parameter with all + # unpermitted keys removed. + # + # params = ActionController::Parameters.new({ + # name: 'Senjougahara Hitagi', + # oddity: 'Heavy stone crab' + # }) + # params.to_h # => {} + # + # safe_params = params.permit(:name) + # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} + def to_h + if permitted? + super + else + slice(*self.class.always_permitted_parameters).permit!.to_h + end + end + # Attribute that keeps track of converted arrays, if any, to avoid double # looping in the common use case permit + mass-assignment. Defined in a # method to instantiate it only if needed. diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index aa894ffa17..c8cc654afd 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -277,4 +277,43 @@ class ParametersPermitTest < ActiveSupport::TestCase test "permitting parameters as an array" do assert_equal "32", @params[:person].permit([ :age ])[:age] end + + test "to_h returns empty hash on unpermitted params" do + assert @params.to_h.is_a? Hash + assert_not @params.to_h.is_a? ActionController::Parameters + assert @params.to_h.empty? + end + + test "to_h returns converted hash on permitted params" do + @params.permit! + + assert @params.to_h.is_a? Hash + assert_not @params.to_h.is_a? ActionController::Parameters + assert_equal @params.to_hash, @params.to_h + end + + test "to_h returns converted hash when .permit_all_parameters is set" do + begin + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") + + assert params.to_h.is_a? Hash + assert_not @params.to_h.is_a? ActionController::Parameters + assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h) + ensure + ActionController::Parameters.permit_all_parameters = false + end + end + + test "to_h returns always permitted parameter on unpermitted params" do + params = ActionController::Parameters.new( + controller: "users", + action: "create", + user: { + name: "Sengoku Nadeko" + } + ) + + assert_equal({ "controller" => "users", "action" => "create" }, params.to_h) + end end -- cgit v1.2.3 From bd7f47190e942d1e514f702eafd1083909dd2884 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Fri, 25 Jul 2014 16:16:58 -0400 Subject: Add missing `Hash` methods to `AC::Parameters` This is to make sure that `permitted` status is maintained on the resulting object. I found these methods that needs to be redefined by looking for `self.class.new` in the code. * extract! * transform_keys * transform_values --- .../action_controller/metal/strong_parameters.rb | 40 ++++++++++++++++++++++ .../parameters/parameters_permit_test.rb | 21 ++++++++++++ 2 files changed, 61 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 764474ad4e..02652a84b3 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -355,6 +355,46 @@ module ActionController end end + # Removes and returns the key/value pairs matching the given keys. + # + # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) + # params.extract!(:a, :b) # => {"a"=>1, "b"=>2} + # params # => {"c"=>3} + def extract!(*keys) + self.class.new(super).tap do |new_instance| + new_instance.permitted = @permitted + end + end + + # Returns a new ActionController::Parameters with the results of + # running +block+ once for every value. The keys are unchanged. + # + # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) + # params.transform_values { |x| x * 2 } + # # => {"a"=>2, "b"=>4, "c"=>6} + def transform_values + if block_given? + self.class.new(super).tap do |new_instance| + new_instance.permitted = @permitted + end + else + super + end + end + + # This method is here only to make sure that the returned object has the + # correct +permitted+ status. It should not matter since the parent of + # this object is +HashWithIndifferentAccess+ + def transform_keys # :nodoc: + if block_given? + self.class.new(super).tap do |new_instance| + new_instance.permitted = @permitted + end + else + super + end + end + # Returns an exact copy of the ActionController::Parameters # instance. +permitted+ state is kept on the duped object. # diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index c8cc654afd..b9d2145eb4 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'action_dispatch/http/upload' require 'action_controller/metal/strong_parameters' +require 'active_support/core_ext/hash/transform_values' class ParametersPermitTest < ActiveSupport::TestCase def assert_filtered_out(params, key) @@ -204,6 +205,9 @@ class ParametersPermitTest < ActiveSupport::TestCase assert !@params.fetch(:person).permitted? assert !@params.values_at(:person).first.permitted? + + assert !@params.transform_keys { |k| k }.permitted? + assert !@params.transform_values { |v| v }.permitted? end test "permitted is sticky on accessors" do @@ -217,17 +221,34 @@ class ParametersPermitTest < ActiveSupport::TestCase assert @params.fetch(:person).permitted? assert @params.values_at(:person).first.permitted? + + assert @params.transform_keys { |k| k }.permitted? + assert @params.transform_values { |v| v }.permitted? end test "not permitted is sticky on mutators" do assert !@params.delete_if { |k| k == "person" }.permitted? assert !@params.keep_if { |k,v| k == "person" }.permitted? + + assert !@params.slice!(:person).permitted? + + assert !@params.extract!(:person).permitted? + + assert !@params.transform_keys! { |k| k }.permitted? + assert !@params.transform_values! { |v| v }.permitted? end test "permitted is sticky on mutators" do @params.permit! assert @params.delete_if { |k| k == "person" }.permitted? assert @params.keep_if { |k,v| k == "person" }.permitted? + + assert @params.slice!(:person).permitted? + + assert @params.extract!(:person).permitted? + + assert @params.transform_keys! { |k| k }.permitted? + assert @params.transform_values! { |v| v }.permitted? end test "not permitted is sticky beyond merges" do -- cgit v1.2.3 From 9704379c780db81f0aea7038cab596c3d19909a8 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Fri, 25 Jul 2014 16:23:38 -0400 Subject: Refactor code to reduce duplicate `self.class.new` --- .../action_controller/metal/strong_parameters.rb | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 02652a84b3..beffe9172b 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -350,9 +350,7 @@ module ActionController # params.slice(:a, :b) # => {"a"=>1, "b"=>2} # params.slice(:d) # => {} def slice(*keys) - self.class.new(super).tap do |new_instance| - new_instance.permitted = @permitted - end + new_instance_with_inherited_permitted_status(super) end # Removes and returns the key/value pairs matching the given keys. @@ -361,9 +359,7 @@ module ActionController # params.extract!(:a, :b) # => {"a"=>1, "b"=>2} # params # => {"c"=>3} def extract!(*keys) - self.class.new(super).tap do |new_instance| - new_instance.permitted = @permitted - end + new_instance_with_inherited_permitted_status(super) end # Returns a new ActionController::Parameters with the results of @@ -374,9 +370,7 @@ module ActionController # # => {"a"=>2, "b"=>4, "c"=>6} def transform_values if block_given? - self.class.new(super).tap do |new_instance| - new_instance.permitted = @permitted - end + new_instance_with_inherited_permitted_status(super) else super end @@ -387,9 +381,7 @@ module ActionController # this object is +HashWithIndifferentAccess+ def transform_keys # :nodoc: if block_given? - self.class.new(super).tap do |new_instance| - new_instance.permitted = @permitted - end + new_instance_with_inherited_permitted_status(super) else super end @@ -415,6 +407,12 @@ module ActionController end private + def new_instance_with_inherited_permitted_status(hash) + self.class.new(hash).tap do |new_instance| + new_instance.permitted = @permitted + end + end + def convert_hashes_to_parameters(key, value, assign_if_converted=true) converted = convert_value_to_parameters(value) self[key] = converted if assign_if_converted && !converted.equal?(value) -- cgit v1.2.3 From 0663e8f1796c8a26c9823ac062420aac267aee16 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 18 Aug 2014 17:02:56 -0400 Subject: Seperate Parameters accessors and mutators tests --- .../test/controller/parameters/accessors_test.rb | 116 +++++++++++++++++++++ .../test/controller/parameters/mutators_test.rb | 99 ++++++++++++++++++ .../parameters/parameters_permit_test.rb | 57 ---------- 3 files changed, 215 insertions(+), 57 deletions(-) create mode 100644 actionpack/test/controller/parameters/accessors_test.rb create mode 100644 actionpack/test/controller/parameters/mutators_test.rb (limited to 'actionpack') diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb new file mode 100644 index 0000000000..7f4e595330 --- /dev/null +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -0,0 +1,116 @@ +require 'abstract_unit' +require 'action_controller/metal/strong_parameters' +require 'active_support/core_ext/hash/transform_values' + +class ParametersAccessorsTest < ActiveSupport::TestCase + setup do + @params = ActionController::Parameters.new( + person: { + age: '32', + name: { + first: 'David', + last: 'Heinemeier Hansson' + }, + addresses: [{city: 'Chicago', state: 'Illinois'}] + } + ) + end + + test "[] retains permitted status" do + @params.permit! + assert @params[:person].permitted? + assert @params[:person][:name].permitted? + end + + test "[] retains unpermitted status" do + assert_not @params[:person].permitted? + assert_not @params[:person][:name].permitted? + end + + test "each carries permitted status" do + @params.permit! + @params.each { |key, value| assert(value.permitted?) if key == "person" } + end + + test "each carries unpermitted status" do + @params.each { |key, value| assert_not(value.permitted?) if key == "person" } + end + + test "except retains permitted status" do + @params.permit! + assert @params.except(:person).permitted? + assert @params[:person].except(:name).permitted? + end + + test "except retains unpermitted status" do + assert_not @params.except(:person).permitted? + assert_not @params[:person].except(:name).permitted? + end + + test "fetch retains permitted status" do + @params.permit! + assert @params.fetch(:person).permitted? + assert @params[:person].fetch(:name).permitted? + end + + test "fetch retains unpermitted status" do + assert_not @params.fetch(:person).permitted? + assert_not @params[:person].fetch(:name).permitted? + end + + test "reject retains permitted status" do + assert_not @params.reject { |k| k == "person" }.permitted? + end + + test "reject retains unpermitted status" do + @params.permit! + assert @params.reject { |k| k == "person" }.permitted? + end + + test "select retains permitted status" do + @params.permit! + assert @params.select { |k| k == "person" }.permitted? + end + + test "select retains unpermitted status" do + assert_not @params.select { |k| k == "person" }.permitted? + end + + test "slice retains permitted status" do + @params.permit! + assert @params.slice(:person).permitted? + end + + test "slice retains unpermitted status" do + assert_not @params.slice(:person).permitted? + end + + test "transform_keys retains permitted status" do + @params.permit! + assert @params.transform_keys { |k| k }.permitted? + end + + test "transform_keys retains unpermitted status" do + assert_not @params.transform_keys { |k| k }.permitted? + end + + test "transform_values retains permitted status" do + @params.permit! + assert @params.transform_values { |v| v }.permitted? + end + + test "transform_values retains unpermitted status" do + assert_not @params.transform_values { |v| v }.permitted? + end + + test "values_at retains permitted status" do + @params.permit! + assert @params.values_at(:person).first.permitted? + assert @params[:person].values_at(:name).first.permitted? + end + + test "values_at retains unpermitted status" do + assert_not @params.values_at(:person).first.permitted? + assert_not @params[:person].values_at(:name).first.permitted? + end +end diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb new file mode 100644 index 0000000000..300dbfa61c --- /dev/null +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -0,0 +1,99 @@ +require 'abstract_unit' +require 'action_controller/metal/strong_parameters' +require 'active_support/core_ext/hash/transform_values' + +class ParametersMutatorsTest < ActiveSupport::TestCase + setup do + @params = ActionController::Parameters.new( + person: { + age: '32', + name: { + first: 'David', + last: 'Heinemeier Hansson' + }, + addresses: [{city: 'Chicago', state: 'Illinois'}] + } + ) + end + + test "delete retains permitted status" do + @params.permit! + assert @params.delete(:person).permitted? + end + + test "delete retains unpermitted status" do + assert_not @params.delete(:person).permitted? + end + + test "delete_if retains permitted status" do + @params.permit! + assert @params.delete_if { |k| k == "person" }.permitted? + end + + test "delete_if retains unpermitted status" do + assert_not @params.delete_if { |k| k == "person" }.permitted? + end + + test "extract! retains permitted status" do + @params.permit! + assert @params.extract!(:person).permitted? + end + + test "extract! retains unpermitted status" do + assert_not @params.extract!(:person).permitted? + end + + test "keep_if retains permitted status" do + @params.permit! + assert @params.keep_if { |k,v| k == "person" }.permitted? + end + + test "keep_if retains unpermitted status" do + assert_not @params.keep_if { |k,v| k == "person" }.permitted? + end + + test "reject! retains permitted status" do + @params.permit! + assert @params.reject! { |k| k == "person" }.permitted? + end + + test "reject! retains unpermitted status" do + assert_not @params.reject! { |k| k == "person" }.permitted? + end + + test "select! retains permitted status" do + @params.permit! + assert @params.select! { |k| k == "person" }.permitted? + end + + test "select! retains unpermitted status" do + assert_not @params.select! { |k| k == "person" }.permitted? + end + + test "slice! retains permitted status" do + @params.permit! + assert @params.slice!(:person).permitted? + end + + test "slice! retains unpermitted status" do + assert_not @params.slice!(:person).permitted? + end + + test "transform_keys! retains permitted status" do + @params.permit! + assert @params.transform_keys! { |k| k }.permitted? + end + + test "transform_keys! retains unpermitted status" do + assert_not @params.transform_keys! { |k| k }.permitted? + end + + test "transform_values! retains permitted status" do + @params.permit! + assert @params.transform_values! { |v| v }.permitted? + end + + test "transform_values! retains unpermitted status" do + assert_not @params.transform_values! { |v| v }.permitted? + end +end diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index b9d2145eb4..ba98ad7605 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -1,7 +1,6 @@ require 'abstract_unit' require 'action_dispatch/http/upload' require 'action_controller/metal/strong_parameters' -require 'active_support/core_ext/hash/transform_values' class ParametersPermitTest < ActiveSupport::TestCase def assert_filtered_out(params, key) @@ -195,62 +194,6 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "monkey", @params.fetch(:foo) { "monkey" } end - test "not permitted is sticky on accessors" do - assert !@params.slice(:person).permitted? - assert !@params[:person][:name].permitted? - assert !@params[:person].except(:name).permitted? - - @params.each { |key, value| assert(!value.permitted?) if key == "person" } - - assert !@params.fetch(:person).permitted? - - assert !@params.values_at(:person).first.permitted? - - assert !@params.transform_keys { |k| k }.permitted? - assert !@params.transform_values { |v| v }.permitted? - end - - test "permitted is sticky on accessors" do - @params.permit! - assert @params.slice(:person).permitted? - assert @params[:person][:name].permitted? - assert @params[:person].except(:name).permitted? - - @params.each { |key, value| assert(value.permitted?) if key == "person" } - - assert @params.fetch(:person).permitted? - - assert @params.values_at(:person).first.permitted? - - assert @params.transform_keys { |k| k }.permitted? - assert @params.transform_values { |v| v }.permitted? - end - - test "not permitted is sticky on mutators" do - assert !@params.delete_if { |k| k == "person" }.permitted? - assert !@params.keep_if { |k,v| k == "person" }.permitted? - - assert !@params.slice!(:person).permitted? - - assert !@params.extract!(:person).permitted? - - assert !@params.transform_keys! { |k| k }.permitted? - assert !@params.transform_values! { |v| v }.permitted? - end - - test "permitted is sticky on mutators" do - @params.permit! - assert @params.delete_if { |k| k == "person" }.permitted? - assert @params.keep_if { |k,v| k == "person" }.permitted? - - assert @params.slice!(:person).permitted? - - assert @params.extract!(:person).permitted? - - assert @params.transform_keys! { |k| k }.permitted? - assert @params.transform_values! { |v| v }.permitted? - end - test "not permitted is sticky beyond merges" do assert !@params.merge(a: "b").permitted? end -- cgit v1.2.3 From 3591dd59e0d5b3e99c1f54619dd78aa7dbba374e Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 18 Aug 2014 20:39:00 -0400 Subject: Fix failing test on several methods on Parameter * `each` * `each_pair` * `delete` * `select!` --- .../action_controller/metal/strong_parameters.rb | 26 +++++++++++++++++++++- .../test/controller/parameters/accessors_test.rb | 9 ++++++++ .../test/controller/parameters/mutators_test.rb | 4 ++-- 3 files changed, 36 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index beffe9172b..7bfe370da4 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -160,6 +160,18 @@ module ActionController end end + # Convert all hashes in values into parameters, then yield each pair like + # the same way as Hash#each_pair + def each_pair(&block) + super do |key, value| + convert_hashes_to_parameters(key, value) + end + + super + end + + alias_method :each, :each_pair + # Attribute that keeps track of converted arrays, if any, to avoid double # looping in the common use case permit + mass-assignment. Defined in a # method to instantiate it only if needed. @@ -195,7 +207,6 @@ module ActionController # Person.new(params) # => # def permit! each_pair do |key, value| - value = convert_hashes_to_parameters(key, value) Array.wrap(value).each do |v| v.permit! if v.respond_to? :permit! end @@ -387,6 +398,19 @@ module ActionController end end + # Deletes and returns a key-value pair from +Parameters+ whose key is equal + # to key. If the key is not found, returns the default value. If the + # optional code block is given and the key is not found, pass in the key + # and return the result of block. + def delete(key, &block) + convert_hashes_to_parameters(key, super, false) + end + + # Equivalent to Hash#keep_if, but returns nil if no changes were made. + def select!(&block) + convert_value_to_parameters(super) + end + # Returns an exact copy of the ActionController::Parameters # instance. +permitted+ state is kept on the duped object. # diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 7f4e595330..97875c3cbb 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -36,6 +36,15 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each_pair carries permitted status" do + @params.permit! + @params.each_pair { |key, value| assert(value.permitted?) if key == "person" } + end + + test "each_pair carries unpermitted status" do + @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } + end + test "except retains permitted status" do @params.permit! assert @params.except(:person).permitted? diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index 300dbfa61c..744d8664be 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -63,11 +63,11 @@ class ParametersMutatorsTest < ActiveSupport::TestCase test "select! retains permitted status" do @params.permit! - assert @params.select! { |k| k == "person" }.permitted? + assert @params.select! { |k| k != "person" }.permitted? end test "select! retains unpermitted status" do - assert_not @params.select! { |k| k == "person" }.permitted? + assert_not @params.select! { |k| k != "person" }.permitted? end test "slice! retains permitted status" do -- cgit v1.2.3 From 3fcbbc8a1c7aec94aa325fd583ead92a7cd291b6 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 18 Aug 2014 23:42:42 -0400 Subject: User `#to_hash` instead of calling `super` Ruby 1.9.3 does not implement Hash#to_h, so we can't call `super` on it. --- actionpack/lib/action_controller/metal/strong_parameters.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 7bfe370da4..7038f0997f 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -154,7 +154,7 @@ module ActionController # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} def to_h if permitted? - super + to_hash else slice(*self.class.always_permitted_parameters).permit!.to_h end -- cgit v1.2.3