From 8d37bd08eeac6d0e94f76ac6640e288bf64595b2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 14 May 2008 13:00:09 -0500 Subject: [PATCH] =?UTF-8?q?Protect=20#filter=5Fparameters=20created=20by?= =?UTF-8?q?=20filter=5Fparameter=5Flogging=20[Jos=C3=A9=20Valim]=20[#196?= =?UTF-8?q?=20state:resolved]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- actionpack/lib/action_controller/base.rb | 51 ++++++++++--------- .../test/controller/filter_params_test.rb | 20 +++++--- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index e1bf005f39..ea55fe42ce 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -259,12 +259,12 @@ module ActionController #:nodoc: DEFAULT_RENDER_STATUS_CODE = "200 OK" include StatusCodes - + # Controller specific instance variables which will not be accessible inside views. @@protected_view_variables = %w(@assigns @performed_redirect @performed_render @variables_added @request_origin @url @parent_controller @action_name @before_filter_chain_aborted @action_cache_path @_session @_cookies @_headers @_params @_flash @_response) - + # Prepends all the URL-generating helpers from AssetHelper. This makes it possible to easily move javascripts, stylesheets, # and images to a dedicated asset server away from the main web server. Example: # ActionController::Base.asset_host = "http://assets.example.com" @@ -325,7 +325,7 @@ module ActionController #:nodoc: # Controls the default charset for all renders. @@default_charset = "utf-8" cattr_accessor :default_charset - + # The logger is used for generating information on the action run-time (including benchmarking) if available. # Can be set to nil for no logging. Compatible with both Ruby's own Logger and Log4r loggers. cattr_accessor :logger @@ -333,7 +333,7 @@ module ActionController #:nodoc: # Controls the resource action separator @@resource_action_separator = "/" cattr_accessor :resource_action_separator - + # Allow to override path names for default resources' actions @@resources_path_names = { :new => 'new', :edit => 'edit' } cattr_accessor :resources_path_names @@ -433,7 +433,7 @@ module ActionController #:nodoc: end # Adds a view_path to the front of the view_paths array. - # If the current class has no view paths, copy them from + # If the current class has no view paths, copy them from # the superclass. This change will be visible for all future requests. # # ArticleController.prepend_view_path("views/default") @@ -444,9 +444,9 @@ module ActionController #:nodoc: view_paths.unshift(*path) ActionView::TemplateFinder.process_view_paths(path) end - + # Adds a view_path to the end of the view_paths array. - # If the current class has no view paths, copy them from + # If the current class has no view paths, copy them from # the superclass. This change will be visible for all future requests. # # ArticleController.append_view_path("views/default") @@ -457,7 +457,7 @@ module ActionController #:nodoc: view_paths.push(*path) ActionView::TemplateFinder.process_view_paths(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. @@ -504,6 +504,7 @@ module ActionController #:nodoc: filtered_parameters end + protected :filter_parameters end # Don't render layouts for templates with the given extensions. @@ -643,12 +644,12 @@ module ActionController #:nodoc: end self.view_paths = [] - + # View load paths for controller. def view_paths @template.finder.view_paths end - + def view_paths=(value) @template.finder.view_paths = value # Mutex needed end @@ -662,7 +663,7 @@ module ActionController #:nodoc: def prepend_view_path(path) @template.finder.prepend_view_path(path) # Mutex needed end - + # Adds a view_path to the end of the view_paths array. # This change affects the current request only. # @@ -874,10 +875,10 @@ module ActionController #:nodoc: elsif action_name = options[:action] template = default_template_name(action_name.to_s) if options[:layout] && !template_exempt_from_layout?(template) - render_with_a_layout(:file => template, :status => options[:status], :use_full_path => true, :layout => true) + render_with_a_layout(:file => template, :status => options[:status], :use_full_path => true, :layout => true) else render_with_no_layout(:file => template, :status => options[:status], :use_full_path => true) - end + end elsif xml = options[:xml] response.content_type ||= Mime::XML @@ -895,12 +896,12 @@ module ActionController #:nodoc: if collection = options[:collection] render_for_text( - @template.send!(:render_partial_collection, partial, collection, + @template.send!(:render_partial_collection, partial, collection, options[:spacer_template], options[:locals]), options[:status] ) else render_for_text( - @template.send!(:render_partial, partial, + @template.send!(:render_partial, partial, ActionView::Base::ObjectWrapper.new(options[:object]), options[:locals]), options[:status] ) end @@ -1024,7 +1025,7 @@ module ActionController #:nodoc: # redirect_to articles_url # redirect_to :back # - # The redirection happens as a "302 Moved" header unless otherwise specified. + # The redirection happens as a "302 Moved" header unless otherwise specified. # # Examples: # redirect_to post_url(@post), :status=>:found @@ -1035,17 +1036,17 @@ module ActionController #:nodoc: # When using redirect_to :back, if there is no referrer, # RedirectBackError will be raised. You may specify some fallback # behavior for this case by rescuing RedirectBackError. - def redirect_to(options = {}, response_status = {}) #:doc: + def redirect_to(options = {}, response_status = {}) #:doc: raise ActionControllerError.new("Cannot redirect to nil!") if options.nil? - if options.is_a?(Hash) && options[:status] - status = options.delete(:status) - elsif response_status[:status] - status = response_status[:status] - else - status = 302 + if options.is_a?(Hash) && options[:status] + status = options.delete(:status) + elsif response_status[:status] + status = response_status[:status] + else + status = 302 end - + case options when %r{^\w+://.*} raise DoubleRenderError if performed? @@ -1119,7 +1120,7 @@ module ActionController #:nodoc: response.body = text.is_a?(Proc) ? text : text.to_s end end - + def initialize_template_class(response) response.template = ActionView::Base.new(self.class.view_paths, {}, self) response.template.extend self.class.master_helper_module diff --git a/actionpack/test/controller/filter_params_test.rb b/actionpack/test/controller/filter_params_test.rb index 11adacb5e3..c9688b2063 100644 --- a/actionpack/test/controller/filter_params_test.rb +++ b/actionpack/test/controller/filter_params_test.rb @@ -7,14 +7,14 @@ class FilterParamTest < Test::Unit::TestCase def setup @controller = FilterParamController.new end - + def test_filter_parameters assert FilterParamController.respond_to?(:filter_parameter_logging) assert !@controller.respond_to?(:filter_parameters) - + FilterParamController.filter_parameter_logging assert @controller.respond_to?(:filter_parameters) - + test_hashes = [[{},{},[]], [{'foo'=>nil},{'foo'=>nil},[]], [{'foo'=>'bar'},{'foo'=>'bar'},[]], @@ -24,11 +24,11 @@ class FilterParamTest < Test::Unit::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']] - + test_hashes.each do |before_filter, after_filter, filter_words| FilterParamController.filter_parameter_logging(*filter_words) - assert_equal after_filter, @controller.filter_parameters(before_filter) - + assert_equal after_filter, @controller.send!(:filter_parameters, before_filter) + filter_words.push('blah') FilterParamController.filter_parameter_logging(*filter_words) do |key, value| value.reverse! if key =~ /bargain/ @@ -37,7 +37,13 @@ class FilterParamTest < Test::Unit::TestCase 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, @controller.filter_parameters(before_filter) + assert_equal after_filter, @controller.send!(:filter_parameters, before_filter) end end + + def test_filter_parameters_is_protected + FilterParamController.filter_parameter_logging + assert !@controller.send!(:action_methods).include?(:filter_parameters) + assert (begin @controller.filter_parameters rescue true end) + end end