aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activejob/test/cases/test_helper_test.rb2
-rw-r--r--activerecord/CHANGELOG.md9
-rw-r--r--activerecord/lib/active_record/core.rb40
-rw-r--r--activerecord/lib/active_record/railtie.rb4
-rw-r--r--activerecord/test/cases/filter_attributes_test.rb32
-rw-r--r--activesupport/CHANGELOG.md5
-rw-r--r--activesupport/lib/active_support/core_ext/integer/multiple.rb2
-rw-r--r--activesupport/lib/active_support/tagged_logging.rb11
-rw-r--r--activesupport/test/tagged_logging_test.rb29
-rw-r--r--guides/source/action_mailer_basics.md2
-rw-r--r--guides/source/configuring.md2
-rw-r--r--railties/lib/rails/mailers_controller.rb2
-rw-r--r--railties/lib/rails/tasks/yarn.rake2
-rw-r--r--railties/test/application/configuration_test.rb3
-rw-r--r--railties/test/application/loading_test.rb33
15 files changed, 130 insertions, 48 deletions
diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb
index 805dd80ad1..160b876e85 100644
--- a/activejob/test/cases/test_helper_test.rb
+++ b/activejob/test/cases/test_helper_test.rb
@@ -1566,7 +1566,7 @@ class PerformedJobsTest < ActiveJob::TestCase
end
end
- def test_assert_performed_wiht_with_global_id_args
+ def test_assert_performed_with_with_global_id_args
ricardo = Person.new(9)
assert_performed_with(job: HelloJob, args: [ricardo]) do
HelloJob.perform_later(ricardo)
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 336946b756..0bb5dfe313 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -9,11 +9,16 @@
*Darwin Wu*
-* Configuration item `config.filter_parameters` could also filter out sensitive value of database column when call `#inspect`.
+* Configuration item `config.filter_parameters` could also filter out
+ sensitive values of database columns when call `#inspect`.
+ We also added `ActiveRecord::Base::filter_attributes`/`=` in order to
+ specify sensitive attributes to specific model.
```
Rails.application.config.filter_parameters += [:credit_card_number]
- Account.last.inspect # => #<Account id: 123, credit_card_number: [FILTERED] ...>
+ Account.last.inspect # => #<Account id: 123, name: "DHH", credit_card_number: [FILTERED] ...>
+ SecureAccount.filter_attributes += [:name]
+ SecureAccount.last.inspect # => #<SecureAccount id: 42, name: [FILTERED], credit_card_number: [FILTERED] ...>
```
*Zhang Kang*
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 82cf7563a2..392602bc0f 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -3,6 +3,7 @@
require "active_support/core_ext/hash/indifferent_access"
require "active_support/core_ext/string/filters"
require "concurrent/map"
+require "set"
module ActiveRecord
module Core
@@ -125,9 +126,7 @@ module ActiveRecord
class_attribute :default_connection_handler, instance_writer: false
- ##
- # Specifies columns which don't want to be exposed while calling #inspect
- class_attribute :filter_attributes, instance_writer: false, default: []
+ self.filter_attributes = []
def self.connection_handler
ActiveRecord::RuntimeRegistry.connection_handler || default_connection_handler
@@ -140,7 +139,7 @@ module ActiveRecord
self.default_connection_handler = ConnectionAdapters::ConnectionHandler.new
end
- module ClassMethods # :nodoc:
+ module ClassMethods
def initialize_find_by_cache # :nodoc:
@find_by_statement_cache = { true => Concurrent::Map.new, false => Concurrent::Map.new }
end
@@ -217,7 +216,7 @@ module ActiveRecord
generated_association_methods
end
- def generated_association_methods
+ def generated_association_methods # :nodoc:
@generated_association_methods ||= begin
mod = const_set(:GeneratedAssociationMethods, Module.new)
private_constant :GeneratedAssociationMethods
@@ -227,8 +226,22 @@ module ActiveRecord
end
end
+ # Returns columns which shouldn't be exposed while calling +#inspect+.
+ def filter_attributes
+ if defined?(@filter_attributes)
+ @filter_attributes
+ else
+ superclass.filter_attributes
+ end
+ 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
+
# Returns a string like 'Post(id:integer, title:string, body:text)'
- def inspect
+ def inspect # :nodoc:
if self == Base
super
elsif abstract_class?
@@ -244,7 +257,7 @@ module ActiveRecord
end
# Overwrite the default class equality method to provide support for decorated models.
- def ===(object)
+ def ===(object) # :nodoc:
object.is_a?(self)
end
@@ -493,13 +506,12 @@ module ActiveRecord
# Returns the contents of the record as a nicely formatted string.
def inspect
- filter_attributes = self.filter_attributes.map(&:to_s).to_set
# We check defined?(@attributes) not to issue warnings if the object is
# allocated but not initialized.
inspection = if defined?(@attributes) && @attributes
self.class.attribute_names.collect do |name|
if has_attribute?(name)
- if filter_attributes.include?(name) && !read_attribute(name).nil?
+ if filter_attribute?(name)
"#{name}: #{ActiveRecord::Core::FILTERED}"
else
"#{name}: #{attribute_for_inspect(name)}"
@@ -517,21 +529,19 @@ module ActiveRecord
# when pp is required.
def pretty_print(pp)
return super if custom_inspect_method_defined?
- filter_attributes = self.filter_attributes.map(&:to_s).to_set
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|
- column_value = read_attribute(column_name)
pp.breakable " "
pp.group(1) do
pp.text column_name
pp.text ":"
pp.breakable
- if filter_attributes.include?(column_name) && !column_value.nil?
+ if filter_attribute?(column_name)
pp.text ActiveRecord::Core::FILTERED
else
- pp.pp column_value
+ pp.pp read_attribute(column_name)
end
end
end
@@ -583,5 +593,9 @@ module ActiveRecord
def custom_inspect_method_defined?
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?
+ end
end
end
diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index 47351588d3..b213754641 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -180,9 +180,7 @@ end_warning
end
initializer "active_record.set_executor_hooks" do
- ActiveSupport.on_load(:active_record) do
- ActiveRecord::QueryCache.install_executor_hooks
- end
+ ActiveRecord::QueryCache.install_executor_hooks
end
initializer "active_record.add_watchable_files" do |app|
diff --git a/activerecord/test/cases/filter_attributes_test.rb b/activerecord/test/cases/filter_attributes_test.rb
index f88cecfe2b..af5badd87d 100644
--- a/activerecord/test/cases/filter_attributes_test.rb
+++ b/activerecord/test/cases/filter_attributes_test.rb
@@ -10,11 +10,12 @@ class FilterAttributesTest < ActiveRecord::TestCase
fixtures :"admin/users", :"admin/accounts"
setup do
+ @previous_filter_attributes = ActiveRecord::Base.filter_attributes
ActiveRecord::Base.filter_attributes = [:name]
end
teardown do
- ActiveRecord::Base.filter_attributes = []
+ ActiveRecord::Base.filter_attributes = @previous_filter_attributes
end
test "filter_attributes" do
@@ -35,20 +36,23 @@ class FilterAttributesTest < ActiveRecord::TestCase
assert_equal 1, account.inspect.scan("[FILTERED]").length
end
- Admin::Account.filter_attributes = []
-
- # Above changes should not impact other models
- Admin::User.all.each do |user|
- assert_includes user.inspect, "name: [FILTERED]"
- assert_equal 1, user.inspect.scan("[FILTERED]").length
+ begin
+ previous_account_filter_attributes = Admin::Account.filter_attributes
+ Admin::Account.filter_attributes = []
+
+ # Above changes should not impact other models
+ Admin::User.all.each do |user|
+ assert_includes user.inspect, "name: [FILTERED]"
+ assert_equal 1, user.inspect.scan("[FILTERED]").length
+ end
+
+ Admin::Account.all.each do |account|
+ assert_not_includes account.inspect, "name: [FILTERED]"
+ assert_equal 0, account.inspect.scan("[FILTERED]").length
+ end
+ ensure
+ Admin::Account.filter_attributes = previous_account_filter_attributes
end
-
- Admin::Account.all.each do |account|
- assert_not_includes account.inspect, "name: [FILTERED]"
- assert_equal 0, account.inspect.scan("[FILTERED]").length
- end
-
- Admin::Account.filter_attributes = [:name]
end
test "filter_attributes should not filter nil value" do
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index d1f079b3d8..37bd4da15e 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Changed `ActiveSupport::TaggedLogging.new` to return a new logger instance instead
+ of mutating the one received as parameter.
+
+ *Thierry Joyal*
+
* Define `unfreeze_time` as an alias of `travel_back` in `ActiveSupport::Testing::TimeHelpers`.
The alias is provided for symmetry with `freeze_time`.
diff --git a/activesupport/lib/active_support/core_ext/integer/multiple.rb b/activesupport/lib/active_support/core_ext/integer/multiple.rb
index e7606662d3..bd57a909c5 100644
--- a/activesupport/lib/active_support/core_ext/integer/multiple.rb
+++ b/activesupport/lib/active_support/core_ext/integer/multiple.rb
@@ -7,6 +7,6 @@ class Integer
# 6.multiple_of?(5) # => false
# 10.multiple_of?(2) # => true
def multiple_of?(number)
- number != 0 ? self % number == 0 : zero?
+ number == 0 ? self == 0 : self % number == 0
end
end
diff --git a/activesupport/lib/active_support/tagged_logging.rb b/activesupport/lib/active_support/tagged_logging.rb
index b069ac94d4..dd72da500c 100644
--- a/activesupport/lib/active_support/tagged_logging.rb
+++ b/activesupport/lib/active_support/tagged_logging.rb
@@ -61,8 +61,15 @@ module ActiveSupport
end
def self.new(logger)
- # Ensure we set a default formatter so we aren't extending nil!
- logger.formatter ||= ActiveSupport::Logger::SimpleFormatter.new
+ logger = logger.dup
+
+ if logger.formatter
+ logger.formatter = logger.formatter.dup
+ else
+ # Ensure we set a default formatter so we aren't extending nil!
+ logger.formatter = ActiveSupport::Logger::SimpleFormatter.new
+ end
+
logger.formatter.extend Formatter
logger.extend(self)
end
diff --git a/activesupport/test/tagged_logging_test.rb b/activesupport/test/tagged_logging_test.rb
index e2b41cf8ee..cff73472c3 100644
--- a/activesupport/test/tagged_logging_test.rb
+++ b/activesupport/test/tagged_logging_test.rb
@@ -19,9 +19,10 @@ class TaggedLoggingTest < ActiveSupport::TestCase
test "sets logger.formatter if missing and extends it with a tagging API" do
logger = Logger.new(StringIO.new)
assert_nil logger.formatter
- ActiveSupport::TaggedLogging.new(logger)
- assert_not_nil logger.formatter
- assert_respond_to logger.formatter, :tagged
+
+ other_logger = ActiveSupport::TaggedLogging.new(logger)
+ assert_not_nil other_logger.formatter
+ assert_respond_to other_logger.formatter, :tagged
end
test "tagged once" do
@@ -83,16 +84,28 @@ class TaggedLoggingTest < ActiveSupport::TestCase
end
test "keeps each tag in their own instance" do
- @other_output = StringIO.new
- @other_logger = ActiveSupport::TaggedLogging.new(MyLogger.new(@other_output))
+ other_output = StringIO.new
+ other_logger = ActiveSupport::TaggedLogging.new(MyLogger.new(other_output))
@logger.tagged("OMG") do
- @other_logger.tagged("BCX") do
+ other_logger.tagged("BCX") do
@logger.info "Cool story"
- @other_logger.info "Funky time"
+ other_logger.info "Funky time"
end
end
assert_equal "[OMG] Cool story\n", @output.string
- assert_equal "[BCX] Funky time\n", @other_output.string
+ assert_equal "[BCX] Funky time\n", other_output.string
+ end
+
+ test "does not share the same formatter instance of the original logger" do
+ other_logger = ActiveSupport::TaggedLogging.new(@logger)
+
+ @logger.tagged("OMG") do
+ other_logger.tagged("BCX") do
+ @logger.info "Cool story"
+ other_logger.info "Funky time"
+ end
+ end
+ assert_equal "[OMG] Cool story\n[BCX] Funky time\n", @output.string
end
test "cleans up the taggings on flush" do
diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md
index 406615f681..041a427f7c 100644
--- a/guides/source/action_mailer_basics.md
+++ b/guides/source/action_mailer_basics.md
@@ -787,7 +787,7 @@ files (environment.rb, production.rb, etc...)
|`sendmail_settings`|Allows you to override options for the `:sendmail` delivery method.<ul><li>`:location` - The location of the sendmail executable. Defaults to `/usr/sbin/sendmail`.</li><li>`:arguments` - The command line arguments to be passed to sendmail. Defaults to `-i`.</li></ul>|
|`raise_delivery_errors`|Whether or not errors should be raised if the email fails to be delivered. This only works if the external email server is configured for immediate delivery.|
|`delivery_method`|Defines a delivery method. Possible values are:<ul><li>`:smtp` (default), can be configured by using `config.action_mailer.smtp_settings`.</li><li>`:sendmail`, can be configured by using `config.action_mailer.sendmail_settings`.</li><li>`:file`: save emails to files; can be configured by using `config.action_mailer.file_settings`.</li><li>`:test`: save emails to `ActionMailer::Base.deliveries` array.</li></ul>See [API docs](http://api.rubyonrails.org/classes/ActionMailer/Base.html) for more info.|
-|`perform_deliveries`|Determines whether deliveries are actually carried out when the `deliver` method is invoked on the Mail message. By default they are, but this can be turned off to help functional testing. If this value is `false`, `deliveries` array will not be populated even if delivery_method is :test.|
+|`perform_deliveries`|Determines whether deliveries are actually carried out when the `deliver` method is invoked on the Mail message. By default they are, but this can be turned off to help functional testing. If this value is `false`, `deliveries` array will not be populated even if `delivery_method` is `:test`.|
|`deliveries`|Keeps an array of all the emails sent out through the Action Mailer with delivery_method :test. Most useful for unit and functional testing.|
|`default_options`|Allows you to set default values for the `mail` method options (`:from`, `:reply_to`, etc.).|
diff --git a/guides/source/configuring.md b/guides/source/configuring.md
index 892634c261..4c508f77f1 100644
--- a/guides/source/configuring.md
+++ b/guides/source/configuring.md
@@ -104,7 +104,7 @@ application. Accepts a valid week day symbol (e.g. `:monday`).
* `config.filter_parameters` used for filtering out the parameters that
you don't want shown in the logs, such as passwords or credit card
-numbers. By default, Rails filters out passwords by adding `Rails.application.config.filter_parameters += [:password]` in `config/initializers/filter_parameter_logging.rb`. Parameters filter works by partial matching regular expression.
+numbers. It also filters out sensitive values of database columns when call `#inspect` on an Active Record object. By default, Rails filters out passwords by adding `Rails.application.config.filter_parameters += [:password]` in `config/initializers/filter_parameter_logging.rb`. Parameters filter works by partial matching regular expression.
* `config.force_ssl` forces all requests to be served over HTTPS by using the `ActionDispatch::SSL` middleware, and sets `config.action_mailer.default_url_options` to be `{ protocol: 'https' }`. This can be configured by setting `config.ssl_options` - see the [ActionDispatch::SSL documentation](http://api.rubyonrails.org/classes/ActionDispatch/SSL.html) for details.
diff --git a/railties/lib/rails/mailers_controller.rb b/railties/lib/rails/mailers_controller.rb
index 0b0e802358..e2d36d7654 100644
--- a/railties/lib/rails/mailers_controller.rb
+++ b/railties/lib/rails/mailers_controller.rb
@@ -10,6 +10,8 @@ class Rails::MailersController < Rails::ApplicationController # :nodoc:
helper_method :part_query, :locale_query
+ content_security_policy(false)
+
def index
@previews = ActionMailer::Preview.all
@page_title = "Mailer Previews"
diff --git a/railties/lib/rails/tasks/yarn.rake b/railties/lib/rails/tasks/yarn.rake
index cf45a392e8..4fb8586b69 100644
--- a/railties/lib/rails/tasks/yarn.rake
+++ b/railties/lib/rails/tasks/yarn.rake
@@ -9,7 +9,7 @@ namespace :yarn do
rails_env = ENV["RAILS_ENV"]
valid_node_envs.include?(rails_env) ? rails_env : "production"
end
- system({ "NODE_ENV" => node_env }, "./bin/yarn install --no-progress --frozen-lockfile")
+ system({ "NODE_ENV" => node_env }, "#{Rails.root}/bin/yarn install --no-progress --frozen-lockfile")
end
end
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
index 83192edb39..44f6af5d4a 100644
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@@ -3,6 +3,7 @@
require "isolation/abstract_unit"
require "rack/test"
require "env_helpers"
+require "set"
class ::MyMailInterceptor
def self.delivering_email(email); email; end
@@ -2049,7 +2050,7 @@ module ApplicationTests
RUBY
app "development"
assert_equal [ :password, :credit_card_number ], Rails.application.config.filter_parameters
- assert_equal [ :password, :credit_card_number ], ActiveRecord::Base.filter_attributes
+ assert_equal [ "password", "credit_card_number" ].to_set, ActiveRecord::Base.filter_attributes
end
private
diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb
index 889ad16fb8..d7f4f09665 100644
--- a/railties/test/application/loading_test.rb
+++ b/railties/test/application/loading_test.rb
@@ -371,6 +371,39 @@ class LoadingTest < ActiveSupport::TestCase
end
end
+ test "active record query cache hooks are installed before first request" do
+ app_file "app/controllers/omg_controller.rb", <<-RUBY
+ begin
+ class OmgController < ActionController::Metal
+ ActiveSupport.run_load_hooks(:action_controller, self)
+ def show
+ if ActiveRecord::Base.connection.query_cache_enabled
+ self.response_body = ["Query cache is enabled."]
+ else
+ self.response_body = ["Expected ActiveRecord::Base.connection.query_cache_enabled to be true"]
+ end
+ end
+ end
+ rescue => e
+ puts "Error loading metal: \#{e.class} \#{e.message}"
+ end
+ RUBY
+
+ app_file "config/routes.rb", <<-RUBY
+ Rails.application.routes.draw do
+ get "/:controller(/:action)"
+ end
+ RUBY
+
+ require "#{rails_root}/config/environment"
+
+ require "rack/test"
+ extend Rack::Test::Methods
+
+ get "/omg/show"
+ assert_equal "Query cache is enabled.", last_response.body
+ end
+
private
def setup_ar!