From bd4f21fbac495fb28b6be993be808509e567239e Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Thu, 21 Jan 2010 04:37:10 +0700 Subject: Move filter_parameter_logging logic out of the controller and create ActionDispatch::ParametersFilter to handle parameter filteration instead. This will make filteration not depending on controller anymore. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/dispatch/request_test.rb | 34 ++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'actionpack/test/dispatch/request_test.rb') diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index cb95ecea50..0a6180959e 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -453,6 +453,40 @@ class RequestTest < ActiveSupport::TestCase request.expects(:parameters).at_least_once.returns({}) assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV]) end + + test "filter_parameters" do + request = stub_request + request.stubs(:request_parameters).returns({ "foo" => 1 }) + request.stubs(:query_parameters).returns({ "bar" => 2 }) + + assert_raises RuntimeError do + ActionDispatch::Http::ParametersFilter.filter_parameters + end + + test_hashes = [ + [{'foo'=>'bar'},{'foo'=>'bar'},%w'food'], + [{'foo'=>'bar'},{'foo'=>'[FILTERED]'},%w'foo'], + [{'foo'=>'bar', 'bar'=>'foo'},{'foo'=>'[FILTERED]', 'bar'=>'foo'},%w'foo baz'], + [{'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)]] + + test_hashes.each do |before_filter, after_filter, filter_words| + ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words) + assert_equal after_filter, request.__send__(:process_parameter_filter, before_filter) + + filter_words.push('blah') + ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words) do |key, value| + value.reverse! if key =~ /bargain/ + end + + before_filter['barg'] = {'bargain'=>'gain', 'blah'=>'bar', 'bar'=>{'bargain'=>{'blah'=>'foo'}}} + after_filter['barg'] = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}} + + assert_equal after_filter, request.__send__(:process_parameter_filter, before_filter) + end + end protected -- 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. --- actionpack/test/dispatch/request_test.rb | 55 ++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 13 deletions(-) (limited to 'actionpack/test/dispatch/request_test.rb') diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 0a6180959e..c5a75f5d21 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -453,16 +453,19 @@ class RequestTest < ActiveSupport::TestCase request.expects(:parameters).at_least_once.returns({}) assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV]) end - - test "filter_parameters" do - request = stub_request - request.stubs(:request_parameters).returns({ "foo" => 1 }) - request.stubs(:query_parameters).returns({ "bar" => 2 }) - + + class FilterRequest < ActionDispatch::Request + end + + test "filter_parameters raises error without arguments" do assert_raises RuntimeError do - ActionDispatch::Http::ParametersFilter.filter_parameters + 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,21 +476,47 @@ class RequestTest < ActiveSupport::TestCase [{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, %w(foo)]] test_hashes.each do |before_filter, after_filter, filter_words| - ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words) - assert_equal after_filter, request.__send__(:process_parameter_filter, before_filter) + FilterRequest.filter_parameters(*filter_words) + assert_equal after_filter, request.send(:process_parameter_filter, before_filter) filter_words.push('blah') - ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words) do |key, value| + + FilterRequest.filter_parameters(*filter_words) do |key, value| value.reverse! if key =~ /bargain/ end before_filter['barg'] = {'bargain'=>'gain', 'blah'=>'bar', 'bar'=>{'bargain'=>{'blah'=>'foo'}}} - after_filter['barg'] = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}} + after_filter['barg'] = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}} - assert_equal after_filter, request.__send__(:process_parameter_filter, before_filter) + assert_equal after_filter, request.send(:process_parameter_filter, before_filter) end 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' }) + + params = request.filtered_parameters + assert_equal "[FILTERED]", params["lifo"] + assert_equal "[FILTERED]", params["amount"] + assert_equal "1", params["step"] + 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 = FilterRequest.new(request.filtered_env) + + assert_equal "[FILTERED]", request.raw_post + assert_equal "[FILTERED]", request.params["amount"] + assert_equal "1", request.params["step"] + end + protected def stub_request(env={}) -- 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. --- actionpack/test/dispatch/request_test.rb | 39 +++++++++++--------------------- 1 file changed, 13 insertions(+), 26 deletions(-) (limited to 'actionpack/test/dispatch/request_test.rb') 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"] -- cgit v1.2.3