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

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 <tt>ActionController::Parameters</tt>.
     # Also, sets the +permitted+ attribute to the default value of
     # <tt>ActionController::Parameters.permit_all_parameters</tt>.
@@ -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: {