diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/filter_parameters.rb | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/parameter_filter.rb | 85 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request_test.rb | 44 | ||||
| -rw-r--r-- | activesupport/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | activesupport/lib/active_support/parameter_filter.rb | 106 | ||||
| -rw-r--r-- | activesupport/test/parameter_filter_test.rb | 51 |
7 files changed, 177 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 diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 586ed28693..b796df26aa 100644 --- a/activesupport/CHANGELOG.md +++ b/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`. diff --git a/activesupport/lib/active_support/parameter_filter.rb b/activesupport/lib/active_support/parameter_filter.rb new file mode 100644 index 0000000000..59945e9daa --- /dev/null +++ b/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 diff --git a/activesupport/test/parameter_filter_test.rb b/activesupport/test/parameter_filter_test.rb new file mode 100644 index 0000000000..3403a3188b --- /dev/null +++ b/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 |