Make filter parameters based on request, so they can be modified for anything in the middleware stack.

10 files changed, 115 insertions, 104 deletions

diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index cf66e99fa5..a66aafd80e 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -74,8 +74,12 @@ module ActionController
     end
 
     # This method has been moved to ActionDispatch::Request.filter_parameters
-    def self.filter_parameter_logging(*)
+    def self.filter_parameter_logging(*args, &block)
       ActiveSupport::Deprecation.warn("Setting filter_parameter_logging in ActionController is deprecated and has no longer effect, please set 'config.filter_parameters' in config/application.rb instead", caller)
+      filter = Rails.application.config.filter_parameters
+      filter.concat(args)
+      filter << block if block
+      filter
     end
 
     def _normalize_options(action=nil, options={}, &blk)
diff --git a/actionpack/lib/action_controller/railties/subscriber.rb b/actionpack/lib/action_controller/railties/subscriber.rb
index d257d6ac2c..1f0e6bf51a 100644
--- a/actionpack/lib/action_controller/railties/subscriber.rb
+++ b/actionpack/lib/action_controller/railties/subscriber.rb
@@ -1,10 +1,14 @@
 module ActionController
   module Railties
     class Subscriber < Rails::Subscriber
+      INTERNAL_PARAMS = %w(controller action format _method only_path)
+
       def start_processing(event)
         payload = event.payload
+        params  = payload[:params].except(*INTERNAL_PARAMS)
+
         info "  Processing by #{payload[:controller]}##{payload[:action]} as #{payload[:formats].first.to_s.upcase}"
-        info "  Parameters: #{payload[:params].inspect}" unless payload[:params].blank?
+        info "  Parameters: #{params.inspect}" unless params.empty?
       end
 
       def process_action(event)
diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb
index 0f4afb01d9..1958e1668d 100644
--- a/actionpack/lib/action_dispatch/http/filter_parameters.rb
+++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb
@@ -1,69 +1,30 @@
+require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/hash/keys'
 
 module ActionDispatch
   module Http
+    # Allows you to specify sensitive parameters which will be replaced from
+    # the request log by looking 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:
+    #
+    #   env["action_dispatch.parameter_filter"] = [:password]
+    #   => replaces the value to all keys matching /password/i with "[FILTERED]"
+    #
+    #   env["action_dispatch.parameter_filter"] = [:foo, "bar"]
+    #   => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
+    #
+    #   env["action_dispatch.parameter_filter"] = lambda do |k,v|
+    #     v.reverse! if k =~ /secret/i
+    #   end
+    #   => reverses the value to all keys matching /secret/i
+    #
     module FilterParameters
       extend ActiveSupport::Concern
 
-      INTERNAL_PARAMS = %w(controller action format _method only_path)
-
-      module ClassMethods
-        # Specify sensitive parameters which will be replaced 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:
-        #
-        #   ActionDispatch::Request.filter_parameters :password
-        #   => replaces the value to all keys matching /password/i with "[FILTERED]"
-        #
-        #   ActionDispatch::Request.filter_parameters :foo, "bar"
-        #   => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
-        #
-        #   ActionDispatch::Request.filter_parameters do |k,v|
-        #     v.reverse! if k =~ /secret/i
-        #   end
-        #   => reverses the value to all keys matching /secret/i
-        #
-        #   ActionDispatch::Request.filter_parameters(:foo, "bar") do |k,v|
-        #     v.reverse! if k =~ /secret/i
-        #   end
-        #   => reverses the value to all keys matching /secret/i, and
-        #      replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
-        def filter_parameters(*filter_words, &block)
-          raise "You must filter at least one word" if filter_words.empty?
-
-          parameter_filter = Regexp.new(filter_words.join('|'), true)
-
-          define_method(:process_parameter_filter) do |original_params|
-            filtered_params = {}
-
-            original_params.each do |key, value|
-              if key =~ parameter_filter
-                value = '[FILTERED]'
-              elsif value.is_a?(Hash)
-                value = process_parameter_filter(value)
-              elsif value.is_a?(Array)
-                value = value.map { |i| process_parameter_filter(i) }
-              elsif block_given?
-                key = key.dup
-                value = value.dup if value.duplicable?
-                yield key, value
-              end
-
-              filtered_params[key] = value
-            end
-
-            filtered_params.except!(*INTERNAL_PARAMS)
-          end
-
-          protected :process_parameter_filter
-        end
-      end
-
       # Return a hash of parameters with all sensitive data replaced.
       def filtered_parameters
         @filtered_parameters ||= process_parameter_filter(parameters)
@@ -71,7 +32,6 @@ module ActionDispatch
       alias :fitered_params :filtered_parameters
 
       # Return a hash of request.env with all sensitive data replaced.
-      # TODO Josh should white list env to remove stuff like rack.input and rack.errors
       def filtered_env
         filtered_env = @env.dup
         filtered_env.each do |key, value|
@@ -86,8 +46,52 @@ module ActionDispatch
 
     protected
 
-      def process_parameter_filter(original_parameters)
-        original_parameters.except(*INTERNAL_PARAMS)
+      def compile_parameter_filter #:nodoc:
+        strings, regexps, blocks = [], [], []
+
+        Array(@env["action_dispatch.parameter_filter"]).each do |item|
+          case item
+          when NilClass
+          when Proc
+            blocks << item
+          when Regexp
+            regexps << item
+          else
+            strings << item.to_s
+          end
+        end
+
+        regexps << Regexp.new(strings.join('|'), true) unless strings.empty?
+        [regexps, blocks]
+      end
+
+      def filtering_parameters? #:nodoc:
+        @env["action_dispatch.parameter_filter"].present?
+      end
+
+      def process_parameter_filter(original_params) #:nodoc:
+        return original_params.dup unless filtering_parameters?
+
+        filtered_params = {}
+        regexps, blocks = compile_parameter_filter
+
+        original_params.each do |key, value|
+          if regexps.find { |r| key =~ r }
+            value = '[FILTERED]'
+          elsif value.is_a?(Hash)
+            value = process_parameter_filter(value)
+          elsif value.is_a?(Array)
+            value = value.map { |i| process_parameter_filter(i) }
+          elsif blocks.present?
+            key = key.dup
+            value = value.dup if value.duplicable?
+            blocks.each { |b| b.call(key, value) }
+          end
+
+          filtered_params[key] = value
+        end
+
+        filtered_params
       end
     end
   end
diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb
index 1510a6a7e0..4fcfbacf4e 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -2,6 +2,9 @@ require 'abstract_unit'
 require 'logger'
 require 'pp' # require 'pp' early to prevent hidden_methods from not picking up the pretty-print methods until too late
 
+module Rails
+end
+
 # Provide some controller to run the tests on.
 module Submodule
   class ContainedEmptyController < ActionController::Base
@@ -63,7 +66,7 @@ class DefaultUrlOptionsController < ActionController::Base
   end
 end
 
-class ControllerClassTests < Test::Unit::TestCase
+class ControllerClassTests < ActiveSupport::TestCase
   def test_controller_path
     assert_equal 'empty', EmptyController.controller_path
     assert_equal EmptyController.controller_path, EmptyController.new.controller_path
@@ -74,7 +77,21 @@ class ControllerClassTests < Test::Unit::TestCase
   def test_controller_name
     assert_equal 'empty', EmptyController.controller_name
     assert_equal 'contained_empty', Submodule::ContainedEmptyController.controller_name
- end
+  end
+ 
+  def test_filter_parameter_logging
+    parameters = []
+    config = mock(:config => mock(:filter_parameters => parameters))
+    Rails.expects(:application).returns(config)
+
+    assert_deprecated do
+      Class.new(ActionController::Base) do
+        filter_parameter_logging :password
+      end
+    end
+
+    assert_equal [:password], parameters
+  end
 end
 
 class ControllerInstanceTests < Test::Unit::TestCase
diff --git a/actionpack/test/controller/subscriber_test.rb b/actionpack/test/controller/subscriber_test.rb
index bd0c83413c..152a0d0c04 100644
--- a/actionpack/test/controller/subscriber_test.rb
+++ b/actionpack/test/controller/subscriber_test.rb
@@ -97,8 +97,7 @@ class ACSubscriberTest < ActionController::TestCase
   end
 
   def test_process_action_with_filter_parameters
-    ActionDispatch::Request.class_eval "alias :safe_process_parameter_filter :process_parameter_filter"
-    ActionDispatch::Request.filter_parameters(:lifo, :amount)
+    @request.env["action_dispatch.parameter_filter"] = [:lifo, :amount]
 
     get :show, :lifo => 'Pratik', :amount => '420', :step => '1'
     wait
@@ -107,8 +106,6 @@ class ACSubscriberTest < ActionController::TestCase
     assert_match /"amount"=>"\[FILTERED\]"/, params
     assert_match /"lifo"=>"\[FILTERED\]"/, params
     assert_match /"step"=>"1"/, params
-  ensure
-    ActionDispatch::Request.class_eval "alias :process_parameter_filter :safe_process_parameter_filter"
   end
 
   def test_redirect_to
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index c5a75f5d21..2b5c19361a 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -454,18 +454,7 @@ class RequestTest < ActiveSupport::TestCase
     assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV])
   end
 
-  class FilterRequest < ActionDispatch::Request
-  end
-
-  test "filter_parameters raises error without arguments" do
-    assert_raises RuntimeError do
-      FilterRequest.filter_parameters
-    end
-  end
-  
   test "process parameter filter" do
-    request = FilterRequest.new({})
-
     test_hashes = [
     [{'foo'=>'bar'},{'foo'=>'bar'},%w'food'],
     [{'foo'=>'bar'},{'foo'=>'[FILTERED]'},%w'foo'],
@@ -473,18 +462,18 @@ class RequestTest < ActiveSupport::TestCase
     [{'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'],
-    [{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, %w(foo)]]
+    [{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, [/foo/]]]
 
     test_hashes.each do |before_filter, after_filter, filter_words|
-      FilterRequest.filter_parameters(*filter_words)
+      request = stub_request('action_dispatch.parameter_filter' => filter_words)
       assert_equal after_filter, request.send(:process_parameter_filter, before_filter)
 
-      filter_words.push('blah')
-
-      FilterRequest.filter_parameters(*filter_words) do |key, value|
+      filter_words << 'blah'
+      filter_words << lambda { |key, value|
         value.reverse! if key =~ /bargain/
-      end
+      }
 
+      request = stub_request('action_dispatch.parameter_filter' => filter_words)
       before_filter['barg'] = {'bargain'=>'gain', 'blah'=>'bar', 'bar'=>{'bargain'=>{'blah'=>'foo'}}}
       after_filter['barg']  = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}}
 
@@ -493,10 +482,9 @@ class RequestTest < ActiveSupport::TestCase
   end
 
   test "filtered_parameters returns params filtered" do
-    FilterRequest.filter_parameters(:lifo, :amount)
-
-    request = FilterRequest.new('action_dispatch.request.parameters' =>
-      { 'lifo' => 'Pratik', 'amount' => '420', 'step' => '1' })
+    request = stub_request('action_dispatch.request.parameters' =>
+      { 'lifo' => 'Pratik', 'amount' => '420', 'step' => '1' },
+      'action_dispatch.parameter_filter' => [:lifo, :amount])
 
     params = request.filtered_parameters
     assert_equal "[FILTERED]", params["lifo"]
@@ -505,12 +493,11 @@ class RequestTest < ActiveSupport::TestCase
   end
 
   test "filtered_env filters env as a whole" do
-    FilterRequest.filter_parameters(:lifo, :amount)
-
-    request = FilterRequest.new('action_dispatch.request.parameters' =>
-      { 'amount' => '420', 'step' => '1' }, "RAW_POST_DATA" => "yada yada")
+    request = stub_request('action_dispatch.request.parameters' =>
+      { 'amount' => '420', 'step' => '1' }, "RAW_POST_DATA" => "yada yada",
+      'action_dispatch.parameter_filter' => [:lifo, :amount])
 
-    request = FilterRequest.new(request.filtered_env)
+    request = stub_request(request.filtered_env)
 
     assert_equal "[FILTERED]", request.raw_post
     assert_equal "[FILTERED]", request.params["amount"]
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"
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
index 8c08fe5acc..6968e87986 100644
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@@ -125,9 +125,9 @@ module ApplicationTests
     
     test "filter_parameters should be able to set via config.filter_parameters" do
       add_to_config <<-RUBY
-        config.filter_parameters :foo, 'bar' do |key, value|
+        config.filter_parameters += [ :foo, 'bar', lambda { |key, value|
           value = value.reverse if key =~ /baz/
-        end
+        }]
       RUBY
       
       assert_nothing_raised do