mirrors
/
rails
mirror of https://github.com/rails/rails
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Implement AR#inspect using ParamterFilter. 

AR instance support `filter_parameters` since #33756.
Though Regex or Proc is valid as `filter_parameters`,
they are not supported as AR#inspect.

I also add :mask option and #filter_params to
`ActiveSupport::ParameterFilter#new` to implement this.
This commit is contained in:
Yoshiyuki Kinjo 2018-10-09 16:14:51 +09:00 committed by Yoshiyuki Kinjo
parent 99c87ad247
commit 32b03b4615
7 changed files with 185 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
activerecord/CHANGELOG.md
View File

@ -138,13 +138,13 @@
specify sensitive attributes to specific model.
```
Rails.application.config.filter_parameters += [:credit_card_number]
Account.last.inspect # => #<Account id: 123, name: "DHH", credit_card_number: [FILTERED] ...>
Rails.application.config.filter_parameters += [:credit_card_number, /phone/]
Account.last.inspect # => #<Account id: 123, name: "DHH", credit_card_number: [FILTERED], telephone_number: [FILTERED] ...>
SecureAccount.filter_attributes += [:name]
SecureAccount.last.inspect # => #<SecureAccount id: 42, name: [FILTERED], credit_card_number: [FILTERED] ...>
```
*Zhang Kang*
*Zhang Kang*, *Yoshiyuki Kinjo*
* Deprecate `column_name_length`, `table_name_length`, `columns_per_table`,
`indexes_per_table`, `columns_per_multicolumn_index`, `sql_query_length`,

19
activerecord/lib/active_record/attribute_methods.rb
View File

@ -336,14 +336,7 @@ module ActiveRecord
# # => "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]"
def attribute_for_inspect(attr_name)
value = read_attribute(attr_name)
if value.is_a?(String) && value.length > 50
"#{value[0, 50]}...".inspect
elsif value.is_a?(Date) || value.is_a?(Time)
%("#{value.to_s(:db)}")
else
value.inspect
end
format_for_inspect(value)
end
# Returns +true+ if the specified +attribute+ has been set by the user or by a
@ -463,6 +456,16 @@ module ActiveRecord
end
end
def format_for_inspect(value)
if value.is_a?(String) && value.length > 50
"#{value[0, 50]}...".inspect
elsif value.is_a?(Date) || value.is_a?(Time)
%("#{value.to_s(:db)}")
else
value.inspect
end
end
def readonly_attribute?(name)
self.class.readonly_attributes.include?(name)
end

41
activerecord/lib/active_record/core.rb
View File

@ -2,15 +2,13 @@
require "active_support/core_ext/hash/indifferent_access"
require "active_support/core_ext/string/filters"
require "active_support/parameter_filter"
require "concurrent/map"
require "set"
module ActiveRecord
module Core
extend ActiveSupport::Concern
FILTERED = "[FILTERED]" # :nodoc:
included do
##
# :singleton-method:
@ -239,9 +237,7 @@ module ActiveRecord
end
# Specifies columns which shouldn't be exposed while calling +#inspect+.
def filter_attributes=(attributes_names)
@filter_attributes = attributes_names.map(&:to_s).to_set
end
attr_writer :filter_attributes
# Returns a string like 'Post(id:integer, title:string, body:text)'
def inspect # :nodoc:
@ -514,11 +510,14 @@ module ActiveRecord
inspection = if defined?(@attributes) && @attributes
self.class.attribute_names.collect do |name|
if has_attribute?(name)
if filter_attribute?(name)
"#{name}: #{ActiveRecord::Core::FILTERED}"
attr = read_attribute(name)
value = if attr.nil?
attr.inspect
else
"#{name}: #{attribute_for_inspect(name)}"
attr = format_for_inspect(attr)
inspection_filter.filter_param(name, attr)
end
"#{name}: #{value}"
end
end.compact.join(", ")
else
@ -534,18 +533,16 @@ module ActiveRecord
return super if custom_inspect_method_defined?
pp.object_address_group(self) do
if defined?(@attributes) && @attributes
column_names = self.class.column_names.select { |name| has_attribute?(name) || new_record? }
pp.seplist(column_names, proc { pp.text "," }) do |column_name|
attr_names = self.class.attribute_names.select { |name| has_attribute?(name) }
pp.seplist(attr_names, proc { pp.text "," }) do |attr_name|
pp.breakable " "
pp.group(1) do
pp.text column_name
pp.text attr_name
pp.text ":"
pp.breakable
if filter_attribute?(column_name)
pp.text ActiveRecord::Core::FILTERED
else
pp.pp read_attribute(column_name)
end
value = read_attribute(attr_name)
value = inspection_filter.filter_param(attr_name, value) unless value.nil?
pp.pp value
end
end
else
@ -597,8 +594,14 @@ module ActiveRecord
self.class.instance_method(:inspect).owner != ActiveRecord::Base.instance_method(:inspect).owner
end
def filter_attribute?(attribute_name)
self.class.filter_attributes.include?(attribute_name) && !read_attribute(attribute_name).nil?
def inspection_filter
@inspection_filter ||= begin
mask = DelegateClass(::String).new(ActiveSupport::ParameterFilter::FILTERED)
def mask.pretty_print(pp)
pp.text __getobj__
end
ActiveSupport::ParameterFilter.new(self.class.filter_attributes, mask: mask)
end
end
end
end

56
activerecord/test/cases/filter_attributes_test.rb
View File

@ -4,6 +4,7 @@ require "cases/helper"
require "models/admin"
require "models/admin/user"
require "models/admin/account"
require "models/user"
require "pp"
class FilterAttributesTest < ActiveRecord::TestCase
@ -30,6 +31,32 @@ class FilterAttributesTest < ActiveRecord::TestCase
end
end
test "string filter_attributes perform pertial match" do
ActiveRecord::Base.filter_attributes = ["n"]
Admin::Account.all.each do |account|
assert_includes account.inspect, "name: [FILTERED]"
assert_equal 1, account.inspect.scan("[FILTERED]").length
end
end
test "regex filter_attributes are accepted" do
ActiveRecord::Base.filter_attributes = [/\An\z/]
account = Admin::Account.find_by(name: "37signals")
assert_includes account.inspect, 'name: "37signals"'
assert_equal 0, account.inspect.scan("[FILTERED]").length
ActiveRecord::Base.filter_attributes = [/\An/]
account = Admin::Account.find_by(name: "37signals")
assert_includes account.reload.inspect, "name: [FILTERED]"
assert_equal 1, account.inspect.scan("[FILTERED]").length
end
test "proc filter_attributes are accepted" do
ActiveRecord::Base.filter_attributes = [ lambda { |key, value| value.reverse! if key == "name" } ]
account = Admin::Account.find_by(name: "37signals")
assert_includes account.inspect, 'name: "slangis73"'
end
test "filter_attributes could be overwritten by models" do
Admin::Account.all.each do |account|
assert_includes account.inspect, "name: [FILTERED]"
@ -37,7 +64,6 @@ class FilterAttributesTest < ActiveRecord::TestCase
end
begin
previous_account_filter_attributes = Admin::Account.filter_attributes
Admin::Account.filter_attributes = []
# Above changes should not impact other models
@ -51,7 +77,7 @@ class FilterAttributesTest < ActiveRecord::TestCase
assert_equal 0, account.inspect.scan("[FILTERED]").length
end
ensure
Admin::Account.filter_attributes = previous_account_filter_attributes
Admin::Account.remove_instance_variable(:@filter_attributes)
end
end
@ -63,6 +89,18 @@ class FilterAttributesTest < ActiveRecord::TestCase
assert_equal 0, account.inspect.scan("[FILTERED]").length
end
test "filter_attributes should handle [FILTERED] value properly" do
begin
User.filter_attributes = ["auth"]
user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]")
assert_includes user.inspect, "auth_token: [FILTERED]"
assert_includes user.inspect, 'token: "[FILTERED]"'
ensure
User.remove_instance_variable(:@filter_attributes)
end
end
test "filter_attributes on pretty_print" do
user = admin_users(:david)
actual = "".dup
@ -81,4 +119,18 @@ class FilterAttributesTest < ActiveRecord::TestCase
assert_not_includes actual, "name: [FILTERED]"
assert_equal 0, actual.scan("[FILTERED]").length
end
test "filter_attributes on pretty_print should handle [FILTERED] value properly" do
begin
User.filter_attributes = ["auth"]
user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]")
actual = "".dup
PP.pp(user, StringIO.new(actual))
assert_includes actual, "auth_token: [FILTERED]"
assert_includes actual, 'token: "[FILTERED]"'
ensure
User.remove_instance_variable(:@filter_attributes)
end
end
end

62
activesupport/lib/active_support/parameter_filter.rb
View File

@ -28,22 +28,36 @@ module ActiveSupport
class ParameterFilter
FILTERED = "[FILTERED]" # :nodoc:
def initialize(filters = [])
# Create instance with given filters. Supported type of filters are +String+, +Regexp+, and +Proc+.
# Other types of filters are treated as +String+ using +to_s+.
# For +Proc+ filters, key, value, and optional original hash is passed to block arguments.
#
# ==== Options
#
# * <tt>:mask</tt> - A replaced object when filtered. Defaults to +"[FILTERED]"+
def initialize(filters = [], mask: FILTERED)
@filters = filters
@mask = mask
end
# Mask value of +params+ if key matches one of filters.
def filter(params)
compiled_filter.call(params)
end
# Returns filtered value for given key. For +Proc+ filters, third block argument is not populated.
def filter_param(key, value)
@filters.empty? ? value : compiled_filter.value_for_key(key, value)
end
private
def compiled_filter
@compiled_filter ||= CompiledFilter.compile(@filters)
@compiled_filter ||= CompiledFilter.compile(@filters, mask: @mask)
end
class CompiledFilter # :nodoc:
def self.compile(filters)
def self.compile(filters, mask:)
return lambda { |params| params.dup } if filters.empty?
strings, regexps, blocks = [], [], []
@ -65,42 +79,46 @@ module ActiveSupport
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
new regexps, deep_regexps, blocks, mask: mask
end
attr_reader :regexps, :deep_regexps, :blocks
def initialize(regexps, deep_regexps, blocks)
def initialize(regexps, deep_regexps, blocks, mask:)
@regexps = regexps
@deep_regexps = deep_regexps.any? ? deep_regexps : nil
@blocks = blocks
@mask = mask
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
filtered_params[key] = value_for_key(key, value, parents, original_params)
end
filtered_params
end
def value_for_key(key, value, parents = [], original_params = nil)
parents.push(key) if deep_regexps
if regexps.any? { |r| r.match?(key) }
value = @mask
elsif deep_regexps && (joined = parents.join(".")) && deep_regexps.any? { |r| r.match?(joined) }
value = @mask
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
value
end
end
end
end

54
activesupport/test/parameter_filter_test.rb
View File

@ -36,6 +36,51 @@ class ParameterFilterTest < ActiveSupport::TestCase
end
end
test "filter should return mask option when value is filtered" do
mask = Object.new.freeze
test_hashes = [
[{ "foo" => "bar" }, { "foo" => "bar" }, %w'food'],
[{ "foo" => "bar" }, { "foo" => mask }, %w'foo'],
[{ "foo" => "bar", "bar" => "foo" }, { "foo" => mask, "bar" => "foo" }, %w'foo baz'],
[{ "foo" => "bar", "baz" => "foo" }, { "foo" => mask, "baz" => mask }, %w'foo baz'],
[{ "bar" => { "foo" => "bar", "bar" => "foo" } }, { "bar" => { "foo" => mask, "bar" => "foo" } }, %w'fo'],
[{ "foo" => { "foo" => "bar", "bar" => "foo" } }, { "foo" => mask }, %w'f banana'],
[{ "deep" => { "cc" => { "code" => "bar", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, { "deep" => { "cc" => { "code" => mask, "bar" => "foo" }, "ss" => { "code" => "bar" } } }, %w'deep.cc.code'],
[{ "baz" => [{ "foo" => "baz" }, "1"] }, { "baz" => [{ "foo" => mask }, "1"] }, [/foo/]]]
test_hashes.each do |before_filter, after_filter, filter_words|
parameter_filter = ActiveSupport::ParameterFilter.new(filter_words, mask: mask)
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, mask: mask)
before_filter["barg"] = { :bargain => "gain", "blah" => "bar", "bar" => { "bargain" => { "blah" => "foo", "hello" => "world" } } }
after_filter["barg"] = { :bargain => "niag", "blah" => mask, "bar" => { "bargain" => { "blah" => mask, "hello" => "world!" } } }
assert_equal after_filter, parameter_filter.filter(before_filter)
end
end
test "filter_param" do
parameter_filter = ActiveSupport::ParameterFilter.new(["foo", /bar/])
assert_equal "[FILTERED]", parameter_filter.filter_param("food", "secret vlaue")
assert_equal "[FILTERED]", parameter_filter.filter_param("baz.foo", "secret vlaue")
assert_equal "[FILTERED]", parameter_filter.filter_param("barbar", "secret vlaue")
assert_equal "non secret value", parameter_filter.filter_param("baz", "non secret value")
end
test "filter_param can work with empty filters" do
parameter_filter = ActiveSupport::ParameterFilter.new
assert_equal "bar", parameter_filter.filter_param("foo", "bar")
end
test "parameter filter should maintain hash with indifferent access" do
test_hashes = [
[{ "foo" => "bar" }.with_indifferent_access, ["blah"]],
@ -48,4 +93,13 @@ class ParameterFilterTest < ActiveSupport::TestCase
parameter_filter.filter(before_filter)
end
end
test "filter_param should return mask option when value is filtered" do
mask = Object.new.freeze
parameter_filter = ActiveSupport::ParameterFilter.new(["foo", /bar/], mask: mask)
assert_equal mask, parameter_filter.filter_param("food", "secret vlaue")
assert_equal mask, parameter_filter.filter_param("baz.foo", "secret vlaue")
assert_equal mask, parameter_filter.filter_param("barbar", "secret vlaue")
assert_equal "non secret value", parameter_filter.filter_param("baz", "non secret value")
end
end

2
railties/test/application/configuration_test.rb
View File

@ -2109,7 +2109,7 @@ module ApplicationTests
RUBY
app "development"
assert_equal [ :password, :credit_card_number ], Rails.application.config.filter_parameters
assert_equal [ "password", "credit_card_number" ].to_set, ActiveRecord::Base.filter_attributes
assert_equal [ :password, :credit_card_number ], ActiveRecord::Base.filter_attributes
end
test "ActiveStorage.routes_prefix can be configured via config.active_storage.routes_prefix" do