From b1bc3b3cd352f68d79d7e232e9520eacb56ca41e Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Thu, 21 Jan 2010 11:48:27 +0700 Subject: Add deprecation warning for calling filter_parameter_logging ActionController::Base, and allow it to be configured in config.filter_parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- railties/lib/generators/rails/app/templates/config/application.rb | 4 ++++ railties/lib/rails/configuration.rb | 4 ++++ 2 files changed, 8 insertions(+) (limited to 'railties/lib') diff --git a/railties/lib/generators/rails/app/templates/config/application.rb b/railties/lib/generators/rails/app/templates/config/application.rb index 334820826f..8a7f024a4d 100644 --- a/railties/lib/generators/rails/app/templates/config/application.rb +++ b/railties/lib/generators/rails/app/templates/config/application.rb @@ -30,5 +30,9 @@ module <%= app_const_base %> # g.template_engine :erb # g.test_framework :test_unit, :fixture => true # end + + # Configure sensitive parameters which will be filtered from the log file. + # Check the documentation for ActionDispatch::Http::ParametersFilter for more information. + # config.filter_parameters :password end end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index b76a7ac2d8..ae4f4007e7 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -252,6 +252,10 @@ module Rails i18n end end + + def filter_parameters(*filter_words, &block) + ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words, &block) + end def environment_path "#{root}/config/environments/#{Rails.env}.rb" -- cgit v1.2.3 From 31fddf2ace29518399f15f718ff408737e0031a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 21 Jan 2010 11:39:57 +0100 Subject: Tidy up new filter_parameters implementation. --- .../rails/app/templates/app/controllers/application_controller.rb | 1 - railties/lib/generators/rails/app/templates/config/application.rb | 5 ++--- railties/lib/rails/configuration.rb | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) (limited to 'railties/lib') diff --git a/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb b/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb index 2cdf4eae54..e8065d9505 100644 --- a/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb +++ b/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb @@ -1,4 +1,3 @@ class ApplicationController < ActionController::Base protect_from_forgery - filter_parameter_logging :password end diff --git a/railties/lib/generators/rails/app/templates/config/application.rb b/railties/lib/generators/rails/app/templates/config/application.rb index 8a7f024a4d..ecebbc6146 100644 --- a/railties/lib/generators/rails/app/templates/config/application.rb +++ b/railties/lib/generators/rails/app/templates/config/application.rb @@ -30,9 +30,8 @@ module <%= app_const_base %> # g.template_engine :erb # g.test_framework :test_unit, :fixture => true # end - + # Configure sensitive parameters which will be filtered from the log file. - # Check the documentation for ActionDispatch::Http::ParametersFilter for more information. - # config.filter_parameters :password + config.filter_parameters :password end end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index ae4f4007e7..a9586b6e9e 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -254,7 +254,7 @@ module Rails end def filter_parameters(*filter_words, &block) - ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words, &block) + ActionDispatch::Request.filter_parameters(*filter_words, &block) end def environment_path -- cgit v1.2.3 From 48dc1ae309d6356991370b9ed4aa9efb25ec9f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 21 Jan 2010 12:10:39 +0100 Subject: Don't let generators die if rubygems puts crap in your load path. --- railties/lib/rails/generators.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'railties/lib') diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index d3175e6a9d..efeeecb685 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -243,7 +243,9 @@ module Rails raise unless e.message =~ /#{Regexp.escape(path)}$/ rescue NameError => e raise unless e.message =~ /Rails::Generator([\s(::)]|$)/ - warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}" + warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}.\n#{e.backtrace}" + rescue Exception => e + warn "[WARNING] Could not load generator #{path.inspect}. Error: #{e.message}.\n#{e.backtrace}" end end end -- cgit v1.2.3 From 378464a2e47bb849f3351cb8c87366554b7ce74d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 21 Jan 2010 13:05:30 +0100 Subject: Default to sync instrumentation. --- railties/lib/rails/commands/server.rb | 11 +------- railties/lib/rails/configuration.rb | 12 ++++----- railties/lib/rails/rack.rb | 1 + railties/lib/rails/rack/logger.rb | 38 ++++++++++++++++++++++++++++ railties/lib/rails/subscriber.rb | 7 +---- railties/lib/rails/subscriber/test_helper.rb | 26 +++---------------- 6 files changed, 50 insertions(+), 45 deletions(-) create mode 100644 railties/lib/rails/rack/logger.rb (limited to 'railties/lib') diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index 115499db05..b21ae2a17b 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -46,7 +46,6 @@ module Rails trap(:INT) { exit } puts "=> Ctrl-C to shutdown server" unless options[:daemonize] - initialize_log_tailer! unless options[:daemonize] super ensure puts 'Exiting' unless options[:daemonize] @@ -54,6 +53,7 @@ module Rails def middleware middlewares = [] + middlewares << [Rails::Rack::LogTailer, log_path] unless options[:daemonize] middlewares << [Rails::Rack::Debugger] if options[:debugger] Hash.new(middlewares) end @@ -71,14 +71,5 @@ module Rails :pid => "tmp/pids/server.pid" }) end - - protected - - # LogTailer should not be used as a middleware since the logging happens - # async in a request and the middleware calls are sync. So we send it - # to subscriber which will be responsible for calling tail! in the log tailer. - def initialize_log_tailer! - Rails::Subscriber.log_tailer = Rails::Rack::LogTailer.new(nil, log_path) - end end end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index a9586b6e9e..c7331f79c5 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -10,15 +10,15 @@ module Rails def self.default_middleware_stack ActionDispatch::MiddlewareStack.new.tap do |middleware| - middleware.use('ActionDispatch::Static', lambda { Rails.public_path }, :if => lambda { Rails.application.config.serve_static_assets }) + middleware.use('::ActionDispatch::Static', lambda { Rails.public_path }, :if => lambda { Rails.application.config.serve_static_assets }) middleware.use('::Rack::Lock', :if => lambda { !ActionController::Base.allow_concurrency }) middleware.use('::Rack::Runtime') - middleware.use('ActionDispatch::ShowExceptions', lambda { ActionController::Base.consider_all_requests_local }) - middleware.use('ActionDispatch::Notifications') - middleware.use('ActionDispatch::Callbacks', lambda { !Rails.application.config.cache_classes }) - middleware.use('ActionDispatch::Cookies') + middleware.use('::Rails::Rack::Logger') + middleware.use('::ActionDispatch::ShowExceptions', lambda { ActionController::Base.consider_all_requests_local }) + middleware.use('::ActionDispatch::Callbacks', lambda { !Rails.application.config.cache_classes }) + middleware.use('::ActionDispatch::Cookies') middleware.use(lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options }) - middleware.use('ActionDispatch::Flash', :if => lambda { ActionController::Base.session_store }) + middleware.use('::ActionDispatch::Flash', :if => lambda { ActionController::Base.session_store }) middleware.use(lambda { Rails::Rack::Metal.new(Rails.application.config.paths.app.metals.to_a, Rails.application.config.metals) }) middleware.use('ActionDispatch::ParamsParser') middleware.use('::Rack::MethodOverride') diff --git a/railties/lib/rails/rack.rb b/railties/lib/rails/rack.rb index 36945a6b0f..4bc0c2c88b 100644 --- a/railties/lib/rails/rack.rb +++ b/railties/lib/rails/rack.rb @@ -1,6 +1,7 @@ module Rails module Rack autoload :Debugger, "rails/rack/debugger" + autoload :Logger, "rails/rack/logger" autoload :LogTailer, "rails/rack/log_tailer" autoload :Metal, "rails/rack/metal" autoload :Static, "rails/rack/static" diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb new file mode 100644 index 0000000000..91a613092f --- /dev/null +++ b/railties/lib/rails/rack/logger.rb @@ -0,0 +1,38 @@ +require 'rails/subscriber' + +module Rails + module Rack + # Log the request started and flush all loggers after it. + class Logger < Rails::Subscriber + def initialize(app) + @app = app + end + + def call(env) + @env = env + before_dispatch + result = @app.call(@env) + after_dispatch + result + end + + protected + + def request + @request ||= ActionDispatch::Request.new(@env) + end + + def before_dispatch + path = request.request_uri.inspect rescue "unknown" + + info "\n\nStarted #{request.method.to_s.upcase} #{path} " << + "for #{request.remote_ip} at #{Time.now.to_s(:db)}" + end + + def after_dispatch + Rails::Subscriber.flush_all! + end + + end + end +end diff --git a/railties/lib/rails/subscriber.rb b/railties/lib/rails/subscriber.rb index 9965786d86..8c62f562d9 100644 --- a/railties/lib/rails/subscriber.rb +++ b/railties/lib/rails/subscriber.rb @@ -33,7 +33,7 @@ module Rails # Subscriber also has some helpers to deal with logging and automatically flushes # all logs when the request finishes (via action_dispatch.callback notification). class Subscriber - mattr_accessor :colorize_logging, :log_tailer + mattr_accessor :colorize_logging self.colorize_logging = true # Embed in a String to clear all previous ANSI sequences. @@ -69,11 +69,6 @@ module Rails Rails.logger.error "Could not log #{args[0].inspect} event. #{e.class}: #{e.message}" end end - - if args[0] == "action_dispatch.after_dispatch" && !subscribers.empty? - flush_all! - log_tailer.tail! if log_tailer - end end # Flush all subscribers' logger. diff --git a/railties/lib/rails/subscriber/test_helper.rb b/railties/lib/rails/subscriber/test_helper.rb index 1464767ed9..39b4117372 100644 --- a/railties/lib/rails/subscriber/test_helper.rb +++ b/railties/lib/rails/subscriber/test_helper.rb @@ -1,12 +1,12 @@ require 'rails/subscriber' -require 'active_support/notifications' module Rails class Subscriber # Provides some helpers to deal with testing subscribers by setting up # notifications. Take for instance ActiveRecord subscriber tests: # - # module SubscriberTest + # class SyncSubscriberTest < ActiveSupport::TestCase + # include Rails::Subscriber::TestHelper # Rails::Subscriber.add(:active_record, ActiveRecord::Railties::Subscriber.new) # # def test_basic_query_logging @@ -39,8 +39,6 @@ module Rails # module TestHelper def setup - Thread.abort_on_exception = true - @logger = MockLogger.new @notifier = ActiveSupport::Notifications::Notifier.new(queue) @@ -54,7 +52,6 @@ module Rails def teardown set_logger(nil) ActiveSupport::Notifications.notifier = nil - Thread.abort_on_exception = false end class MockLogger @@ -92,26 +89,9 @@ module Rails def set_logger(logger) Rails.logger = logger end - end - - module SyncTestHelper - include TestHelper - - def queue - ActiveSupport::Notifications::Fanout.new(true) - end - end - - module AsyncTestHelper - include TestHelper def queue - ActiveSupport::Notifications::Fanout.new(false) - end - - def wait - sleep(0.01) - super + ActiveSupport::Notifications::Fanout.new end end end -- cgit v1.2.3 From 04063393f9392b83cf5ccd0a16f217cc7261e15c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 21 Jan 2010 14:11:24 +0100 Subject: Give higher priority to rails generators. --- railties/lib/rails/generators.rb | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'railties/lib') diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index efeeecb685..83b8c74966 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -134,9 +134,14 @@ module Rails lookups = [] lookups << "#{base}:#{name}" if base lookups << "#{name}:#{context}" if context - lookups << "#{name}:#{name}" unless name.to_s.include?(?:) - lookups << "#{name}" - lookups << "rails:#{name}" unless base || context || name.to_s.include?(?:) + + unless base || context + unless name.to_s.include?(?:) + lookups << "#{name}:#{name}" + lookups << "rails:#{name}" + end + lookups << "#{name}" + end lookup(lookups) @@ -232,9 +237,9 @@ module Rails load_generators_from_railties! paths = namespaces_to_paths(namespaces) - paths.each do |path| - ["generators", "rails_generators"].each do |base| - path = "#{base}/#{path}_generator" + paths.each do |raw_path| + ["rails_generators", "generators"].each do |base| + path = "#{base}/#{raw_path}_generator" begin require path @@ -243,9 +248,9 @@ module Rails raise unless e.message =~ /#{Regexp.escape(path)}$/ rescue NameError => e raise unless e.message =~ /Rails::Generator([\s(::)]|$)/ - warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}.\n#{e.backtrace}" + warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}.\n#{e.backtrace.join("\n")}" rescue Exception => e - warn "[WARNING] Could not load generator #{path.inspect}. Error: #{e.message}.\n#{e.backtrace}" + warn "[WARNING] Could not load generator #{path.inspect}. Error: #{e.message}.\n#{e.backtrace.join("\n")}" end end end @@ -280,7 +285,7 @@ module Rails paths = [] namespaces.each do |namespace| pieces = namespace.split(":") - paths << pieces.dup.push(pieces.last).join("/") + paths << pieces.dup.push(pieces.last).join("/") unless pieces.uniq.size == 1 paths << pieces.join("/") end paths.uniq! -- cgit v1.2.3 From fc4f237864541f5012f9b8cc8e0ec81960377e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 21 Jan 2010 16:50:11 +0100 Subject: Make filter parameters based on request, so they can be modified for anything in the middleware stack. --- railties/lib/generators/rails/app/templates/config/application.rb | 2 +- railties/lib/rails/application.rb | 1 + railties/lib/rails/configuration.rb | 7 ++----- 3 files changed, 4 insertions(+), 6 deletions(-) (limited to 'railties/lib') diff --git a/railties/lib/generators/rails/app/templates/config/application.rb b/railties/lib/generators/rails/app/templates/config/application.rb index ecebbc6146..78a355d2f4 100644 --- a/railties/lib/generators/rails/app/templates/config/application.rb +++ b/railties/lib/generators/rails/app/templates/config/application.rb @@ -32,6 +32,6 @@ module <%= app_const_base %> # end # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters :password + config.filter_parameters << :password end end diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 743681359c..5c9892c630 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -110,6 +110,7 @@ module Rails end def call(env) + env["action_dispatch.parameter_filter"] = config.filter_parameters app.call(env) end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index c7331f79c5..7f1783a6b9 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -69,7 +69,7 @@ module Rails class Configuration < Railtie::Configuration attr_accessor :after_initialize_blocks, :cache_classes, :colorize_logging, - :consider_all_requests_local, :dependency_loading, + :consider_all_requests_local, :dependency_loading, :filter_parameters, :load_once_paths, :logger, :metals, :plugins, :preload_frameworks, :reload_plugins, :serve_static_assets, :time_zone, :whiny_nils @@ -83,6 +83,7 @@ module Rails super @load_once_paths = [] @after_initialize_blocks = [] + @filter_parameters = [] @dependency_loading = true @serve_static_assets = true end @@ -252,10 +253,6 @@ module Rails i18n end end - - def filter_parameters(*filter_words, &block) - ActionDispatch::Request.filter_parameters(*filter_words, &block) - end def environment_path "#{root}/config/environments/#{Rails.env}.rb" -- cgit v1.2.3