Browse files

Move FilterParameterLogging to a stand alone module and make it work …

…on new base
  • Loading branch information...
1 parent faaff38 commit 1d168afcb146872cb7e49b6d513629fbb19e39b0 @lifo lifo committed May 22, 2009
View
2 actionpack/Rakefile
@@ -66,7 +66,7 @@ Rake::TestTask.new(:test_new_base_on_old_tests) do |t|
render render_json render_xml
send_file request_forgery_protection rescue url_rewriter verification webservice
http_basic_authentication http_digest_authentication
- action_pack_assertions assert_select
+ action_pack_assertions assert_select filter_params
).map { |name| "test/controller/#{name}_test.rb" }
end
View
1 actionpack/lib/action_controller.rb
@@ -66,6 +66,7 @@ def self.load_all!
autoload :UrlRewriter, 'action_controller/routing/generation/url_rewriter'
autoload :UrlWriter, 'action_controller/routing/generation/url_rewriter'
autoload :Verification, 'action_controller/base/verification'
+ autoload :FilterParameterLogging, 'action_controller/base/filter_parameter_logging'
module Assertions
autoload :DomAssertions, 'action_controller/testing/assertions/dom'
View
58 actionpack/lib/action_controller/base/base.rb
@@ -448,55 +448,6 @@ def append_view_path(path)
@view_paths = superclass.view_paths.dup if @view_paths.nil?
@view_paths.push(*path)
end
-
- # Replace sensitive parameter data from the request log.
- # Filters parameters that have any of the arguments as a substring.
- # Looks in all subhashes of the param hash for keys to filter.
- # If a block is given, each key and value of the parameter hash and all
- # subhashes is passed to it, the value or key
- # can be replaced using String#replace or similar method.
- #
- # Examples:
- # filter_parameter_logging
- # => Does nothing, just slows the logging process down
- #
- # filter_parameter_logging :password
- # => replaces the value to all keys matching /password/i with "[FILTERED]"
- #
- # filter_parameter_logging :foo, "bar"
- # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
- #
- # filter_parameter_logging { |k,v| v.reverse! if k =~ /secret/i }
- # => reverses the value to all keys matching /secret/i
- #
- # filter_parameter_logging(:foo, "bar") { |k,v| v.reverse! if k =~ /secret/i }
- # => reverses the value to all keys matching /secret/i, and
- # replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
- def filter_parameter_logging(*filter_words, &block)
- parameter_filter = Regexp.new(filter_words.collect{ |s| s.to_s }.join('|'), true) if filter_words.length > 0
-
- define_method(:filter_parameters) do |unfiltered_parameters|
- filtered_parameters = {}
-
- unfiltered_parameters.each do |key, value|
- if key =~ parameter_filter
- filtered_parameters[key] = '[FILTERED]'
- elsif value.is_a?(Hash)
- filtered_parameters[key] = filter_parameters(value)
- elsif block_given?
- key = key.dup
- value = value.dup if value
- yield key, value
- filtered_parameters[key] = value
- else
- filtered_parameters[key] = value
- end
- end
-
- filtered_parameters
- end
- protected :filter_parameters
- end
@@exempt_from_layout = [ActionView::TemplateHandlers::RJS]
@@ -853,13 +804,6 @@ def log_processing_for_request_id
logger.info(request_id)
end
- def log_processing_for_parameters
- parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup
- parameters = parameters.except!(:controller, :action, :format, :_method)
-
- logger.info " Parameters: #{parameters.inspect}" unless parameters.empty?
- end
-
def default_render #:nodoc:
render
end
@@ -933,7 +877,7 @@ def process_cleanup
[ Filters, Layout, Renderer, Redirector, Responder, Benchmarking, Rescue, Flash, MimeResponds, Helpers,
Cookies, Caching, Verification, Streaming, SessionManagement,
HttpAuthentication::Basic::ControllerMethods, HttpAuthentication::Digest::ControllerMethods, RecordIdentifier,
- RequestForgeryProtection, Translation
+ RequestForgeryProtection, Translation, FilterParameterLogging
].each do |mod|
include mod
end
View
97 actionpack/lib/action_controller/base/filter_parameter_logging.rb
@@ -0,0 +1,97 @@
+module ActionController
+ module FilterParameterLogging
+ extend ActiveSupport::DependencyModule
+
+ # TODO : Remove the defined? check when new base is the main base
+ if defined?(ActionController::Http)
+ depends_on AbstractController::Logger
+ end
+
+ included do
+ if defined?(ActionController::Http)
+ include InstanceMethodsForNewBase
+ end
+ end
+
+ module ClassMethods
+ # Replace sensitive parameter data from the request log.
+ # Filters parameters that have any of the arguments as a substring.
+ # Looks in all subhashes of the param hash for keys to filter.
+ # If a block is given, each key and value of the parameter hash and all
+ # subhashes is passed to it, the value or key
+ # can be replaced using String#replace or similar method.
+ #
+ # Examples:
+ # filter_parameter_logging
+ # => Does nothing, just slows the logging process down
+ #
+ # filter_parameter_logging :password
+ # => replaces the value to all keys matching /password/i with "[FILTERED]"
+ #
+ # filter_parameter_logging :foo, "bar"
+ # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
+ #
+ # filter_parameter_logging { |k,v| v.reverse! if k =~ /secret/i }
+ # => reverses the value to all keys matching /secret/i
+ #
+ # filter_parameter_logging(:foo, "bar") { |k,v| v.reverse! if k =~ /secret/i }
+ # => reverses the value to all keys matching /secret/i, and
+ # replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
+ def filter_parameter_logging(*filter_words, &block)
+ parameter_filter = Regexp.new(filter_words.collect{ |s| s.to_s }.join('|'), true) if filter_words.length > 0
+
+ define_method(:filter_parameters) do |unfiltered_parameters|
+ filtered_parameters = {}
+
+ unfiltered_parameters.each do |key, value|
+ if key =~ parameter_filter
+ filtered_parameters[key] = '[FILTERED]'
+ elsif value.is_a?(Hash)
+ filtered_parameters[key] = filter_parameters(value)
+ elsif block_given?
+ key = key.dup
+ value = value.dup if value
+ yield key, value
+ filtered_parameters[key] = value
+ else
+ filtered_parameters[key] = value
+ end
+ end
+
+ filtered_parameters
+ end
+ protected :filter_parameters
+ end
+ end
+
+ module InstanceMethodsForNewBase
+ # TODO : Fix the order of information inside such that it's exactly same as the old base
+ def process(*)
+ ret = super
+
+ if logger
+ parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup
+ parameters = parameters.except!(:controller, :action, :format, :_method)
+
+ unless parameters.empty?
+ # TODO : Move DelayedLog to AS
+ log = AbstractController::Logger::DelayedLog.new { " Parameters: #{parameters.inspect}" }
+ logger.info(log)
+ end
+ end
+
+ ret
+ end
+ end
+
+ private
+
+ # TODO : This method is not needed for the new base
+ def log_processing_for_parameters
+ parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup
+ parameters = parameters.except!(:controller, :action, :format, :_method)
+
+ logger.info " Parameters: #{parameters.inspect}" unless parameters.empty?
+ end
+ end
+end
View
1 actionpack/lib/action_controller/new_base.rb
@@ -32,6 +32,7 @@ module ActionController
autoload :RequestForgeryProtection, 'action_controller/base/request_forgery_protection'
autoload :Streaming, 'action_controller/base/streaming'
autoload :HttpAuthentication, 'action_controller/base/http_authentication'
+ autoload :FilterParameterLogging, 'action_controller/base/filter_parameter_logging'
require 'action_controller/routing'
end
View
1 actionpack/lib/action_controller/new_base/base.rb
@@ -33,6 +33,7 @@ class Base < Http
include ActionController::Streaming
include ActionController::HttpAuthentication::Basic::ControllerMethods
include ActionController::HttpAuthentication::Digest::ControllerMethods
+ include ActionController::FilterParameterLogging
# TODO: Extract into its own module
# This should be moved together with other normalizing behavior
View
45 actionpack/test/controller/filter_params_test.rb
@@ -1,13 +1,30 @@
require 'abstract_unit'
class FilterParamController < ActionController::Base
+ def payment
+ head :ok
+ end
end
-class FilterParamTest < Test::Unit::TestCase
- def setup
- @controller = FilterParamController.new
+class FilterParamTest < ActionController::TestCase
+ tests FilterParamController
+
+ class MockLogger
+ attr_reader :logged
+ attr_accessor :level
+
+ def initialize
+ @level = Logger::DEBUG
+ end
+
+ def method_missing(method, *args)
+ @logged ||= []
+ @logged << args.first
+ end
end
+ setup :set_logger
+
def test_filter_parameters
assert FilterParamController.respond_to?(:filter_parameter_logging)
assert !@controller.respond_to?(:filter_parameters)
@@ -46,4 +63,26 @@ def test_filter_parameters_is_protected
assert !FilterParamController.action_methods.include?('filter_parameters')
assert_raise(NoMethodError) { @controller.filter_parameters([{'password' => '[FILTERED]'}]) }
end
+
+ def test_filter_parameters_inside_logs
+ FilterParamController.filter_parameter_logging(:lifo, :amount)
+
+ get :payment, :lifo => 'Pratik', :amount => '420', :step => '1'
+
+ filtered_params_logs = logs.detect {|l| l =~ /\AParameters/ }
+
+ assert filtered_params_logs.index('"amount"=>"[FILTERED]"')
+ assert filtered_params_logs.index('"lifo"=>"[FILTERED]"')
+ assert filtered_params_logs.index('"step"=>"1"')
+ end
+
+ private
+
+ def set_logger
+ @controller.logger = MockLogger.new
+ end
+
+ def logs
+ @logs ||= @controller.logger.logged.compact.map {|l| l.to_s.strip}
+ end
end

0 comments on commit 1d168af

Please sign in to comment.