Add explicit array filters

actionpack/lib/action_controller/metal/strong_parameters.rb

@@ -675,7 +675,7 @@ def require(key)
     #     params.permit(person: { '0': [:email], '1': [:phone]}).to_h
     #     # => {"person"=>{"0"=>{"email"=>"[email protected]"}, "1"=>{"phone"=>"555-6789"}}}
     def permit(*filters)
-      permit_only(*filters, &method(:unpermitted_parameters))
+      permit_filters(filters, &method(:unpermitted_parameters))
     end
 
     # Same as permit, but raises on unpermitted parameters by default.
@@ -684,12 +684,7 @@ def permit(*filters)
     # If a block is given, it is called with the unpermitted keys.
     def permit_only(*filters, &block)
       block ||= method(:raise_unpermitted_parameters)
-      params = permit_filters(filters, &block)
-
-      unpermitted = unpermitted_keys(params)
-      block.call(unpermitted) if unpermitted.any?
-
-      params
+      permit_filters(filters, &block)
     end
 
     # `expect` is the preferred way to require and permit parameters and work
@@ -819,13 +814,15 @@ def permit_only(*filters, &block)
     #    object  # => #<ActionController::Parameters {"pie"=>"pumpkin"} permitted: true>
     #
     def expect(*filters)
-      expect_only filters, &method(:unpermitted_parameters)
+      expect_only(*filters, &method(:unpermitted_parameters))
     end
 
-    # Same as expect, but raises on unpermitted parameters, regardless of ActionController::Parameters.action_on_unpermitted_parameters
+    # Same as expect, but raises on unpermitted parameters, ignoring ActionController::Parameters.action_on_unpermitted_parameters
+    #
+    # Accepts a block which will yield the unpermitted keys if any are found.
     def expect_only(*filters, &block)
       block ||= method(:raise_unpermitted_parameters)
-      params = permit_filters(filters, &block)
+      params = permit_filters(filters, explicit_arrays: true, check_unpermitted: false, &block)
       keys = filters.flatten.flat_map { |f| f.is_a?(Hash) ? f.keys : f }
       values = params.require(keys)
       values.size == 1 ? values.first : values
@@ -848,20 +845,22 @@ def expect_only(*filters, &block)
     #     person_params[:age]  # => nil
     #
     def allow(*filters)
-      allow_only filters, &method(:unpermitted_parameters)
+      allow_only(*filters, &method(:unpermitted_parameters))
     end
 
-    # Same as allow, but raises on unpermitted parameters, regardless of ActionController::Parameters.action_on_unpermitted_parameters
+    # Same as allow, but raises on unpermitted parameters, ignoring ActionController::Parameters.action_on_unpermitted_parameters
+    #
+    # Accepts a block which will yield the unpermitted keys if any are found.
     def allow_only(*filters, &block)
       block ||= method(:raise_unpermitted_parameters)
-      params = permit_filters(filters, &block)
+      params = permit_filters(filters, explicit_arrays: true, check_unpermitted: false, &block)
       values = filters.flatten.flat_map do |filter|
         case filter
         when Symbol, String
           params.fetch(filter, nil)
         when Hash
           filter.map do |key, value|
-            params.fetch(key) { value == EMPTY_ARRAY ? [] : self.class.new.permit! }
+            params.fetch(key) { value == EMPTY_ARRAY || array_filter?(value) ? [] : self.class.new.permit! }
           end
         end
       end
@@ -1205,6 +1204,39 @@ def each_nested_attribute
         hash
       end
 
+      # Filters params given the set of filters.
+      #
+      # explicit_arrays: true switches to explicit array param filtering.
+      #   This is a new requirement for permit to distinguish between:
+      #     { comment: [:text] }
+      #     { comments: [[:text]] }
+      #   With explicit_arrays: false, the first filters would allow
+      #   comment to return either a hash or array of hashs.
+      #   With explicit_arrays: true, the first way would only allow a hash
+      #   with key :text while the second way would allow only an array of hashes
+      #   with key :text.
+      # check_unpermitted: true calls the passed block when unpermitted keys are found.
+      #   It is intentionally not passed to hash_filter, defaulting to true for nested keys.
+      def permit_filters(filters, explicit_arrays: false, check_unpermitted: true, &block)
+        params = self.class.new
+
+        filters.flatten.each do |filter|
+          case filter
+          when Symbol, String
+            permitted_scalar_filter(params, filter)
+          when Hash
+            hash_filter(params, filter, explicit_arrays:, &block)
+          end
+        end
+
+        if check_unpermitted
+          unpermitted = unpermitted_keys(params)
+          block.call(unpermitted) if unpermitted.any?
+        end
+
+        params.permit!
+      end
+
     private
       def new_instance_with_inherited_permitted_status(hash)
         self.class.new(hash, @logging_context).tap do |new_instance|
@@ -1294,6 +1326,28 @@ def specify_numeric_keys?(filter)
         end
       end
 
+      # When an array is expected, you must specify an array explicitly
+      # using the following format:
+      #
+      #     params.expect(comments: [[:flavor]])
+      #
+      # Which will match only the following array formats:
+      #
+      #     { pies: [{ flavor: "rhubarb" }, { flavor: "apple" }] }
+      #     { pies: { "0" => { flavor: "key lime" }, "1" =>  { flavor: "mince" } } }
+      #
+      # Previously, arrays were specified the same way as hashes:
+      #
+      #     params.expect(pies: [:flavor])
+      #
+      # which would also allow matching with a hash (or vice versa):
+      #
+      #     { pies: { flavor: "cherry" } }
+      #
+      def array_filter?(filter)
+        filter.is_a?(Array) && filter.size == 1 && filter.first.is_a?(Array)
+      end
+
       def each_element(object, filter, &block)
         case object
         when Array
@@ -1360,22 +1414,6 @@ def permitted_scalar?(value)
         PERMITTED_SCALAR_TYPES.any? { |type| value.is_a?(type) }
       end
 
-      # Filters params given the set of filters.
-      def permit_filters(filters, &block)
-        params = self.class.new
-
-        filters.flatten.each do |filter|
-          case filter
-          when Symbol, String
-            permitted_scalar_filter(params, filter)
-          when Hash
-            hash_filter(params, filter, &block)
-          end
-        end
-
-        params.permit!
-      end
-
       # Adds existing keys to the params if their values are scalar.
       #
       # For example:
@@ -1413,7 +1451,7 @@ def non_scalar?(value)
 
       EMPTY_ARRAY = [] # :nodoc:
       EMPTY_HASH  = {} # :nodoc:
-      def hash_filter(params, filter, &block)
+      def hash_filter(params, filter, explicit_arrays: false, &block)
         filter = filter.with_indifferent_access
 
         # Slicing filters out non-declared keys.
@@ -1431,15 +1469,40 @@ def hash_filter(params, filter, &block)
             if value.is_a?(Parameters)
               params[key] = permit_any_in_parameters(value)
             end
+          elsif array_filter?(filter[key])
+            # Declaration { comments: [[:text]] }
+            params[key] = each_array_element(value, filter[key].first) do |element|
+              element.permit_filters(Array.wrap(filter[key].first), explicit_arrays:, &block)
+            end
+          elsif explicit_arrays
+            if value.is_a?(Parameters)
+              # Treat params as a hash even if they could be interpretted as an array.
+              params[key] = value.permit_filters(Array.wrap(filter[key]), explicit_arrays:, &block)
+            end
           elsif non_scalar?(value)
-            # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }.
+            # Declaration { user: { address: ... } } or { user: [:name, :age, { address: ... }] }.
+            # Allow arrays with old syntax { comments: [:text] }
             params[key] = each_element(value, filter[key]) do |element|
-              element.permit_only(filter[key], &block)
+              element.permit_filters(Array.wrap(filter[key]), explicit_arrays:, &block)
             end
           end
         end
       end
 
+      # Called with the params being filtered (object) and a nested array (filter)
+      def each_array_element(object, filter, &block)
+        case object
+        when Array
+          object.grep(Parameters).filter_map(&block)
+        when Parameters
+          if object.nested_attributes? && !specify_numeric_keys?(filter)
+            object.each_nested_attribute(&block)
+          else
+            [] # Array is expected but params are not an array.
+          end
+        end
+      end
+
       def permit_any_in_parameters(params)
         self.class.new.tap do |sanitized|
           params.each do |key, value|

actionpack/test/controller/parameters/parameters_allow_test.rb

@@ -38,19 +38,44 @@ class ParametersAllowTest < ActiveSupport::TestCase
   end
 
   test "keys to arrays: returns permitted params in hash key order" do
-    name, addresses = @params[:person].allow(name: [:first, :last], addresses: [:city])
+    name, addresses = @params[:person].allow(name: [:first, :last], addresses: [[:city, :state]])
 
     assert_equal({ "first" => "David", "last" => "Heinemeier Hansson" }, name.to_h)
-    assert_equal({ "city" => "Chicago" }, addresses.first.to_h)
+    assert_equal({ "city" => "Chicago", "state" => "Illinois" }, addresses.first.to_h)
   end
 
-  test "key and hash: returns permitted params" do
-    params = ActionController::Parameters.new(name: "Martin", age: 40, pies: [{ type: "dessert", flavor: "pumpkin"}])
-    name, age, pies = params.allow(:name, :age, pies: [:type, :flavor])
+  test "key to array of keys: returns nil when params is an array" do
+    params = ActionController::Parameters.new(name: "Martin", pies: [{ flavor: "pumpkin" }])
+    pies = params.allow(pies: [:flavor])
 
-    assert_equal "Martin", name
-    assert_equal 40, age
-    assert_equal({ "type" => "dessert", "flavor" => "pumpkin" }, pies.first.to_h)
+    assert_equal({}, pies.to_h)
+  end
+
+  test "key to array of keys: returns nil when params is nested attributes array" do
+    params = ActionController::Parameters.new(name: "Martin", pies: { "0" => { flavor: "pumpkin" } })
+    pies = params.allow(pies: [:flavor])
+
+    assert_equal({}, pies.to_h)
+  end
+
+  test "key to nested array: returns permitted array" do
+    params = ActionController::Parameters.new(name: "Martin", pies: [{ flavor: "pumpkin" }, { flavor: "chicken pot" }])
+    pies = params.allow(pies: [[:flavor]])
+
+    assert_equal({ "flavor" => "pumpkin" }, pies[0].to_h)
+    assert_equal({ "flavor" => "chicken pot" }, pies[1].to_h)
+  end
+
+  test "key to nested array: returns array when params is a hash" do
+    params = ActionController::Parameters.new(name: "Martin", pies: { flavor: "pumpkin" })
+
+    assert_equal [], params.allow(pies: [[:flavor]])
+  end
+
+  test "key to nested array: returns empty array when params empty array" do
+    params = ActionController::Parameters.new(name: "Martin", pies: [])
+
+    assert_equal [], params.allow(pies: [[:flavor]])
   end
 
   test "key to mixed array: returns permitted params" do

actionpack/test/controller/parameters/parameters_expect_test.rb

@@ -38,19 +38,34 @@ class ParametersExpectTest < ActiveSupport::TestCase
   end
 
   test "keys to arrays: returns permitted params in hash key order" do
-    name, addresses = @params[:person].expect(name: [:first, :last], addresses: [:city])
+    name, addresses = @params[:person].expect(name: [:first, :last], addresses: [[:city]])
 
     assert_equal({ "first" => "David", "last" => "Heinemeier Hansson" }, name.to_h)
     assert_equal({ "city" => "Chicago" }, addresses.first.to_h)
   end
 
-  test "key and hash: returns permitted params" do
-    params = ActionController::Parameters.new(name: "Martin", age: 40, pies: [{ type: "dessert", flavor: "pumpkin"}])
-    name, age, pies = params.expect(:name, :age, pies: [:type, :flavor])
+  test "key to array of keys: raises when params is an array" do
+    params = ActionController::Parameters.new(name: "Martin", pies: [{ flavor: "pumpkin" }])
 
-    assert_equal "Martin", name
-    assert_equal 40, age
-    assert_equal({ "type" => "dessert", "flavor" => "pumpkin" }, pies.first.to_h)
+    assert_raises(ActionController::ParameterMissing) do
+      params.expect(pies: [:flavor])
+    end
+  end
+
+  test "key to nested array: returns permitted array" do
+    params = ActionController::Parameters.new(name: "Martin", pies: [{ flavor: "pumpkin" }, { flavor: "chicken pot" }])
+    pies = params.expect(pies: [[:flavor]])
+
+    assert_equal({ "flavor" => "pumpkin" }, pies[0].to_h)
+    assert_equal({ "flavor" => "chicken pot" }, pies[1].to_h)
+  end
+
+  test "key to nested array: raises when params is a hash" do
+    params = ActionController::Parameters.new(name: "Martin", pies: { flavor: "pumpkin" })
+
+    assert_raises(ActionController::ParameterMissing) do
+      params.expect(pies: [[:flavor]])
+    end
   end
 
   test "key to mixed array: returns permitted params" do