Merge pull request #34039 from yskkin/parameter_filter

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

4 files changed, 16 insertions, 125 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c972c64766..8205d79dd2 100644
--- a/actionpack/CHANGELOG.md
+++ b/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*
diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb
index ec012ad02d..9f8f178402 100644
--- a/actionpack/lib/action_dispatch/http/filter_parameters.rb
+++ b/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 ActionDispatch
     #   => 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 @@ module ActionDispatch
       end
 
       def parameter_filter_for(filters) # :doc:
-        ParameterFilter.new(filters)
+        ActiveSupport::ParameterFilter.new(filters)
       end
 
       KV_RE   = "[^&;=]+"
diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb
index 6689092859..ddeb3d81e2 100644
--- a/actionpack/lib/action_dispatch/http/parameter_filter.rb
+++ b/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
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index c7b68e5266..9d1246b3a4 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/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