Merge pull request #36127 from st0012/fix_non_num_keys

Improve nested parameter resolving - continuation of 29888

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.

actionpack/lib/action_controller/metal/strong_parameters.rb

@@ -223,6 +223,12 @@ class Parameters
     #    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 @@ def deep_dup
 
       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 @@ def convert_value_to_parameters(value)
         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

actionpack/test/controller/parameters/nested_parameters_permit_test.rb

@@ -125,7 +125,7 @@ def assert_filtered_out(params, key)
     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 @@ def assert_filtered_out(params, key)
     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: {