Deprecate ActionDispatch::Http::ParameterFilter in favor of ActiveSup…

actionpack/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Deprecate `ActionDispatch::Http::ParameterFilter` in favor of `ActiveSupport::ParameterFilter`.
+
+    *Yoshiyuki Kinjo*
+
 *   Remove undocumented `params` option from `url_for` helper.
 
     *Ilkka Oksanen*

actionpack/lib/action_dispatch/http/filter_parameters.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require "action_dispatch/http/parameter_filter"
+require "active_support/parameter_filter"
 
 module ActionDispatch
   module Http
@@ -28,8 +28,8 @@ module Http
     #   => reverses the value to all keys matching /secret/i
     module FilterParameters
       ENV_MATCH = [/RAW_POST_DATA/, "rack.request.form_vars"] # :nodoc:
-      NULL_PARAM_FILTER = ParameterFilter.new # :nodoc:
-      NULL_ENV_FILTER   = ParameterFilter.new ENV_MATCH # :nodoc:
+      NULL_PARAM_FILTER = ActiveSupport::ParameterFilter.new # :nodoc:
+      NULL_ENV_FILTER   = ActiveSupport::ParameterFilter.new ENV_MATCH # :nodoc:
 
       def initialize
         super
@@ -69,7 +69,7 @@ def env_filter # :doc:
       end
 
       def parameter_filter_for(filters) # :doc:
-        ParameterFilter.new(filters)
+        ActiveSupport::ParameterFilter.new(filters)
       end
 
       KV_RE   = "[^&;=]+"

actionpack/lib/action_dispatch/http/parameter_filter.rb

@@ -1,87 +1,12 @@
 # frozen_string_literal: true
 
-require "active_support/core_ext/object/duplicable"
-require "active_support/core_ext/array/extract"
+require "active_support/deprecation/constant_accessor"
+require "active_support/parameter_filter"
 
 module ActionDispatch
   module Http
-    class ParameterFilter
-      FILTERED = "[FILTERED]" # :nodoc:
-
-      def initialize(filters = [])
-        @filters = filters
-      end
-
-      def filter(params)
-        compiled_filter.call(params)
-      end
-
-    private
-
-      def compiled_filter
-        @compiled_filter ||= CompiledFilter.compile(@filters)
-      end
-
-      class CompiledFilter # :nodoc:
-        def self.compile(filters)
-          return lambda { |params| params.dup } if filters.empty?
-
-          strings, regexps, blocks = [], [], []
-
-          filters.each do |item|
-            case item
-            when Proc
-              blocks << item
-            when Regexp
-              regexps << item
-            else
-              strings << Regexp.escape(item.to_s)
-            end
-          end
-
-          deep_regexps = regexps.extract! { |r| r.to_s.include?("\\.") }
-          deep_strings = strings.extract! { |s| s.include?("\\.") }
-
-          regexps << Regexp.new(strings.join("|"), true) unless strings.empty?
-          deep_regexps << Regexp.new(deep_strings.join("|"), true) unless deep_strings.empty?
-
-          new regexps, deep_regexps, blocks
-        end
-
-        attr_reader :regexps, :deep_regexps, :blocks
-
-        def initialize(regexps, deep_regexps, blocks)
-          @regexps = regexps
-          @deep_regexps = deep_regexps.any? ? deep_regexps : nil
-          @blocks = blocks
-        end
-
-        def call(params, parents = [], original_params = params)
-          filtered_params = params.class.new
-
-          params.each do |key, value|
-            parents.push(key) if deep_regexps
-            if regexps.any? { |r| key =~ r }
-              value = FILTERED
-            elsif deep_regexps && (joined = parents.join(".")) && deep_regexps.any? { |r| joined =~ r }
-              value = FILTERED
-            elsif value.is_a?(Hash)
-              value = call(value, parents, original_params)
-            elsif value.is_a?(Array)
-              value = value.map { |v| v.is_a?(Hash) ? call(v, parents, original_params) : v }
-            elsif blocks.any?
-              key = key.dup if key.duplicable?
-              value = value.dup if value.duplicable?
-              blocks.each { |b| b.arity == 2 ? b.call(key, value) : b.call(key, value, original_params) }
-            end
-            parents.pop if deep_regexps
-
-            filtered_params[key] = value
-          end
-
-          filtered_params
-        end
-      end
-    end
+    include ActiveSupport::Deprecation::DeprecatedConstantAccessor
+    deprecate_constant "ParameterFilter", "ActiveSupport::ParameterFilter",
+      message: "ActionDispatch::Http::ParameterFilter is deprecated and will be removed from Rails 6.1. Use ActiveSupport::ParameterFilter instead."
   end
 end

actionpack/test/dispatch/request_test.rb

@@ -1059,47 +1059,9 @@ class RequestParameters < BaseRequestTest
 end
 
 class RequestParameterFilter < BaseRequestTest
-  test "process parameter filter" do
-    test_hashes = [
-    [{ "foo" => "bar" }, { "foo" => "bar" }, %w'food'],
-    [{ "foo" => "bar" }, { "foo" => "[FILTERED]" }, %w'foo'],
-    [{ "foo" => "bar", "bar" => "foo" }, { "foo" => "[FILTERED]", "bar" => "foo" }, %w'foo baz'],
-    [{ "foo" => "bar", "baz" => "foo" }, { "foo" => "[FILTERED]", "baz" => "[FILTERED]" }, %w'foo baz'],
-    [{ "bar" => { "foo" => "bar", "bar" => "foo" } }, { "bar" => { "foo" => "[FILTERED]", "bar" => "foo" } }, %w'fo'],
-    [{ "foo" => { "foo" => "bar", "bar" => "foo" } }, { "foo" => "[FILTERED]" }, %w'f banana'],
-    [{ "deep" => { "cc" => { "code" => "bar", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, { "deep" => { "cc" => { "code" => "[FILTERED]", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, %w'deep.cc.code'],
-    [{ "baz" => [{ "foo" => "baz" }, "1"] }, { "baz" => [{ "foo" => "[FILTERED]" }, "1"] }, [/foo/]]]
-
-    test_hashes.each do |before_filter, after_filter, filter_words|
-      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
-      assert_equal after_filter, parameter_filter.filter(before_filter)
-
-      filter_words << "blah"
-      filter_words << lambda { |key, value|
-        value.reverse! if key =~ /bargain/
-      }
-      filter_words << lambda { |key, value, original_params|
-        value.replace("world!") if original_params["barg"]["blah"] == "bar" && key == "hello"
-      }
-
-      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
-      before_filter["barg"] = { :bargain => "gain", "blah" => "bar", "bar" => { "bargain" => { "blah" => "foo", "hello" => "world" } } }
-      after_filter["barg"]  = { :bargain => "niag", "blah" => "[FILTERED]", "bar" => { "bargain" => { "blah" => "[FILTERED]", "hello" => "world!" } } }
-
-      assert_equal after_filter, parameter_filter.filter(before_filter)
-    end
-  end
-
-  test "parameter filter should maintain hash with indifferent access" do
-    test_hashes = [
-      [{ "foo" => "bar" }.with_indifferent_access, ["blah"]],
-      [{ "foo" => "bar" }.with_indifferent_access, []]
-    ]
-
-    test_hashes.each do |before_filter, filter_words|
-      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
-      assert_instance_of ActiveSupport::HashWithIndifferentAccess,
-                         parameter_filter.filter(before_filter)
+  test "parameter filter is deprecated" do
+    assert_deprecated do
+      ActionDispatch::Http::ParameterFilter.new(["blah"])
     end
   end
 

activesupport/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Add `ActiveSupport::ParameterFilter`.
+
+    *Yoshiyuki Kinjo*
+
 *   Rename `Module#parent`, `Module#parents`, and `Module#parent_name` to
     `module_parent`, `module_parents`, and `module_parent_name`.
 

activesupport/lib/active_support/parameter_filter.rb

@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+require "active_support/core_ext/object/duplicable"
+require "active_support/core_ext/array/extract"
+
+module ActiveSupport
+  # +ParameterFilter+ allows you to specify keys for sensitive data from
+  # hash-like object and replace corresponding value. Filtering only certain
+  # sub-keys from a hash is possible by using the dot notation:
+  # 'credit_card.number'. If a proc is given, each key and value of a hash and
+  # all sub-hashes are passed to it, where the value or the key can be replaced
+  # using String#replace or similar methods.
+  #
+  #   ActiveSupport::ParameterFilter.new([:password])
+  #   => replaces the value to all keys matching /password/i with "[FILTERED]"
+  #
+  #   ActiveSupport::ParameterFilter.new([:foo, "bar"])
+  #   => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
+  #
+  #   ActiveSupport::ParameterFilter.new(["credit_card.code"])
+  #   => replaces { credit_card: {code: "xxxx"} } with "[FILTERED]", does not
+  #   change { file: { code: "xxxx"} }
+  #
+  #   ActiveSupport::ParameterFilter.new([-> (k, v) do
+  #     v.reverse! if k =~ /secret/i
+  #   end])
+  #   => reverses the value to all keys matching /secret/i
+  class ParameterFilter
+    FILTERED = "[FILTERED]" # :nodoc:
+
+    def initialize(filters = [])
+      @filters = filters
+    end
+
+    def filter(params)
+      compiled_filter.call(params)
+    end
+
+  private
+
+    def compiled_filter
+      @compiled_filter ||= CompiledFilter.compile(@filters)
+    end
+
+    class CompiledFilter # :nodoc:
+      def self.compile(filters)
+        return lambda { |params| params.dup } if filters.empty?
+
+        strings, regexps, blocks = [], [], []
+
+        filters.each do |item|
+          case item
+          when Proc
+            blocks << item
+          when Regexp
+            regexps << item
+          else
+            strings << Regexp.escape(item.to_s)
+          end
+        end
+
+        deep_regexps = regexps.extract! { |r| r.to_s.include?("\\.") }
+        deep_strings = strings.extract! { |s| s.include?("\\.") }
+
+        regexps << Regexp.new(strings.join("|"), true) unless strings.empty?
+        deep_regexps << Regexp.new(deep_strings.join("|"), true) unless deep_strings.empty?
+
+        new regexps, deep_regexps, blocks
+      end
+
+      attr_reader :regexps, :deep_regexps, :blocks
+
+      def initialize(regexps, deep_regexps, blocks)
+        @regexps = regexps
+        @deep_regexps = deep_regexps.any? ? deep_regexps : nil
+        @blocks = blocks
+      end
+
+      def call(params, parents = [], original_params = params)
+        filtered_params = params.class.new
+
+        params.each do |key, value|
+          parents.push(key) if deep_regexps
+          if regexps.any? { |r| key =~ r }
+            value = FILTERED
+          elsif deep_regexps && (joined = parents.join(".")) && deep_regexps.any? { |r| joined =~ r }
+            value = FILTERED
+          elsif value.is_a?(Hash)
+            value = call(value, parents, original_params)
+          elsif value.is_a?(Array)
+            value = value.map { |v| v.is_a?(Hash) ? call(v, parents, original_params) : v }
+          elsif blocks.any?
+            key = key.dup if key.duplicable?
+            value = value.dup if value.duplicable?
+            blocks.each { |b| b.arity == 2 ? b.call(key, value) : b.call(key, value, original_params) }
+          end
+          parents.pop if deep_regexps
+
+          filtered_params[key] = value
+        end
+
+        filtered_params
+      end
+    end
+  end
+end

activesupport/test/parameter_filter_test.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "abstract_unit"
+require "active_support/core_ext/hash"
+require "active_support/parameter_filter"
+
+class ParameterFilterTest < ActiveSupport::TestCase
+  test "process parameter filter" do
+    test_hashes = [
+    [{ "foo" => "bar" }, { "foo" => "bar" }, %w'food'],
+    [{ "foo" => "bar" }, { "foo" => "[FILTERED]" }, %w'foo'],
+    [{ "foo" => "bar", "bar" => "foo" }, { "foo" => "[FILTERED]", "bar" => "foo" }, %w'foo baz'],
+    [{ "foo" => "bar", "baz" => "foo" }, { "foo" => "[FILTERED]", "baz" => "[FILTERED]" }, %w'foo baz'],
+    [{ "bar" => { "foo" => "bar", "bar" => "foo" } }, { "bar" => { "foo" => "[FILTERED]", "bar" => "foo" } }, %w'fo'],
+    [{ "foo" => { "foo" => "bar", "bar" => "foo" } }, { "foo" => "[FILTERED]" }, %w'f banana'],
+    [{ "deep" => { "cc" => { "code" => "bar", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, { "deep" => { "cc" => { "code" => "[FILTERED]", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, %w'deep.cc.code'],
+    [{ "baz" => [{ "foo" => "baz" }, "1"] }, { "baz" => [{ "foo" => "[FILTERED]" }, "1"] }, [/foo/]]]
+
+    test_hashes.each do |before_filter, after_filter, filter_words|
+      parameter_filter = ActiveSupport::ParameterFilter.new(filter_words)
+      assert_equal after_filter, parameter_filter.filter(before_filter)
+
+      filter_words << "blah"
+      filter_words << lambda { |key, value|
+        value.reverse! if key =~ /bargain/
+      }
+      filter_words << lambda { |key, value, original_params|
+        value.replace("world!") if original_params["barg"]["blah"] == "bar" && key == "hello"
+      }
+
+      parameter_filter = ActiveSupport::ParameterFilter.new(filter_words)
+      before_filter["barg"] = { :bargain => "gain", "blah" => "bar", "bar" => { "bargain" => { "blah" => "foo", "hello" => "world" } } }
+      after_filter["barg"]  = { :bargain => "niag", "blah" => "[FILTERED]", "bar" => { "bargain" => { "blah" => "[FILTERED]", "hello" => "world!" } } }
+
+      assert_equal after_filter, parameter_filter.filter(before_filter)
+    end
+  end
+
+  test "parameter filter should maintain hash with indifferent access" do
+    test_hashes = [
+      [{ "foo" => "bar" }.with_indifferent_access, ["blah"]],
+      [{ "foo" => "bar" }.with_indifferent_access, []]
+    ]
+
+    test_hashes.each do |before_filter, filter_words|
+      parameter_filter = ActiveSupport::ParameterFilter.new(filter_words)
+      assert_instance_of ActiveSupport::HashWithIndifferentAccess,
+                         parameter_filter.filter(before_filter)
+    end
+  end
+end