From 858c63a0a401ac76e7c59069cea3700181d30748 Mon Sep 17 00:00:00 2001 From: "L.Fexon" Date: Tue, 1 Aug 2017 14:02:41 -0400 Subject: [PATCH] fixed usage of Parameters when a non-numeric key exists test for non-numeric key in nested attributes test: extra blank line between tests removed test for non-numeric key fixed (by Daniel) Update according to feedback --- actionpack/CHANGELOG.md | 2 ++ .../metal/strong_parameters.rb | 24 ++++++++++----- .../nested_parameters_permit_test.rb | 30 +++++++++++++++++-- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 9fcff6a6ca..d9046f1431 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,5 @@ +* Fix strong parameters blocks all attributes even when only some keys are invalid (non-numerical). It should only block invalid key's values instead. + *Stan Lo* Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index ae774b01f1..8bde82ccca 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -223,6 +223,12 @@ module ActionController # config.always_permitted_parameters = %w( controller action format ) cattr_accessor :always_permitted_parameters, default: %w( controller action ) + class << self + def nested_attribute?(key, value) # :nodoc: + key =~ /\A-?\d+\z/ && (value.is_a?(Hash) || value.is_a?(Parameters)) + end + end + # Returns a new instance of ActionController::Parameters. # Also, sets the +permitted+ attribute to the default value of # ActionController::Parameters.permit_all_parameters. @@ -811,8 +817,14 @@ module ActionController attr_writer :permitted - def fields_for_style? - @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && (v.is_a?(Hash) || v.is_a?(Parameters)) } + def nested_attributes? + @parameters.any? { |k, v| Parameters.nested_attribute?(k, v) } + end + + def each_nested_attribute + hash = self.class.new + self.each { |k, v| hash[k] = yield v if Parameters.nested_attribute?(k, v) } + hash end private @@ -857,15 +869,13 @@ module ActionController end end - def each_element(object) + def each_element(object, &block) case object when Array object.grep(Parameters).map { |el| yield el }.compact when Parameters - if object.fields_for_style? - hash = object.class.new - object.each { |k, v| hash[k] = yield v } - hash + if object.nested_attributes? + object.each_nested_attribute(&block) else yield object end diff --git a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb index 1403e224c0..6243b5c51b 100644 --- a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb @@ -125,7 +125,7 @@ class NestedParametersPermitTest < ActiveSupport::TestCase assert_nil permitted[:book][:genre] end - test "fields_for-style nested params" do + test "nested params with numeric keys" do params = ActionController::Parameters.new( book: { authors_attributes: { @@ -150,7 +150,33 @@ class NestedParametersPermitTest < ActiveSupport::TestCase assert_filtered_out permitted[:book][:authors_attributes]["0"], :age_of_death end - test "fields_for-style nested params with negative numbers" do + test "nested params with non_numeric keys" do + params = ActionController::Parameters.new( + book: { + authors_attributes: { + '0': { name: "William Shakespeare", age_of_death: "52" }, + '1': { name: "Unattributed Assistant" }, + '2': "Not a hash", + 'new_record': { name: "Some name" } + } + }) + permitted = params.permit book: { authors_attributes: [ :name ] } + + assert_not_nil permitted[:book][:authors_attributes]["0"] + assert_not_nil permitted[:book][:authors_attributes]["1"] + + assert_nil permitted[:book][:authors_attributes]["2"] + assert_nil permitted[:book][:authors_attributes]["new_record"] + assert_equal "William Shakespeare", permitted[:book][:authors_attributes]["0"][:name] + assert_equal "Unattributed Assistant", permitted[:book][:authors_attributes]["1"][:name] + + assert_equal( + { "book" => { "authors_attributes" => { "0" => { "name" => "William Shakespeare" }, "1" => { "name" => "Unattributed Assistant" } } } }, + permitted.to_h + ) + end + + test "nested params with negative numeric keys" do params = ActionController::Parameters.new( book: { authors_attributes: {