Permalink
Browse files

Merge branch 'master' into nested_has_many_through

  • Loading branch information...
jonleighton committed Mar 12, 2011
2 parents 02a43f9 + acd4bfb commit 17ea20426057aac43abcc0735534df31c577b918
View
@@ -1,5 +1,7 @@
*Rails 3.1.0 (unreleased)*
+* Sensitive query string parameters (specified in config.filter_parameters) will now be filtered out from the request paths in the log file. [Prem Sichanugrist, fxn]
+
* URL parameters which return false for to_param now appear in the query string (previously they were removed) [Andrew White]
* URL parameters which return nil for to_param are now removed from the query string [Andrew White]
@@ -5,10 +5,10 @@
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.
+ # the request log by looking in the query string of the request and all
+ # subhashes of the params hash to filter. If a block is given, each key and
+ # value of the params hash and all subhashes is passed to it, the value
+ # or key can be replaced using String#replace or similar method.
#
# Examples:
#
@@ -38,6 +38,11 @@ def filtered_env
@filtered_env ||= env_filter.filter(@env)
end
+ # Reconstructed a path with all sensitive GET parameters replaced.
+ def filtered_path
+ @filtered_path ||= query_string.empty? ? path : "#{path}?#{filtered_query_string}"
+ end
+
protected
def parameter_filter
@@ -52,6 +57,14 @@ def parameter_filter_for(filters)
@@parameter_filter_for[filters] ||= ParameterFilter.new(filters)
end
+ KV_RE = '[^&;=]+'
+ PAIR_RE = %r{(#{KV_RE})=(#{KV_RE})}
+ def filtered_query_string
+ query_string.gsub(PAIR_RE) do |_|
+ parameter_filter.filter([[$1, $2]]).first.join("=")
+ end
+ end
+
end
end
end
@@ -107,7 +107,7 @@ def normalize_path(path)
if @options[:format] == false
@options.delete(:format)
path
- elsif path.include?(":format")
+ elsif path.include?(":format") || path.end_with?('/')
path
else
"#{path}(.:format)"
@@ -243,10 +243,6 @@ def self.normalize_name(name)
end
module Base
- def initialize(set) #:nodoc:
- @set = set
- end
-
# You can specify what Rails should route "/" to with the root method:
#
# root :to => 'pages#main'
@@ -558,11 +554,6 @@ def map_method(method, *args, &block)
# PUT /admin/posts/1
# DELETE /admin/posts/1
module Scoping
- def initialize(*args) #:nodoc:
- @scope = {}
- super
- end
-
# Scopes a set of routes to the given default options.
#
# Take the following route definition as an example:
@@ -956,11 +947,6 @@ def singular
alias :nested_scope :path
end
- def initialize(*args) #:nodoc:
- super
- @scope[:path_names] = @set.resources_path_names
- end
-
def resources_path_names(options)
@scope[:path_names].merge!(options)
end
@@ -1473,6 +1459,11 @@ def match(*args)
end
end
+ def initialize(set) #:nodoc:
+ @set = set
+ @scope = { :path_names => @set.resources_path_names }
+ end
+
include Base
include HttpHelpers
include Redirection
@@ -0,0 +1,51 @@
+require 'abstract_unit'
+
+module ActionDispatch
+ module Routing
+ class MapperTest < ActiveSupport::TestCase
+ class FakeSet
+ attr_reader :routes
+
+ def initialize
+ @routes = []
+ end
+
+ def resources_path_names
+ {}
+ end
+
+ def request_class
+ ActionDispatch::Request
+ end
+
+ def add_route(*args)
+ routes << args
+ end
+
+ def conditions
+ routes.map { |x| x[1] }
+ end
+ end
+
+ def test_initialize
+ Mapper.new FakeSet.new
+ end
+
+ def test_map_slash
+ fakeset = FakeSet.new
+ mapper = Mapper.new fakeset
+ mapper.match '/', :to => 'posts#index', :as => :main
+ assert_equal '/', fakeset.conditions.first[:path_info]
+ end
+
+ def test_map_more_slashes
+ fakeset = FakeSet.new
+ mapper = Mapper.new fakeset
+
+ # FIXME: is this a desired behavior?
+ mapper.match '/one/two/', :to => 'posts#index', :as => :main
+ assert_equal '/one/two(.:format)', fakeset.conditions.first[:path_info]
+ end
+ end
+ end
+end
@@ -518,6 +518,44 @@ def url_for(options = {})
assert_equal "1", request.params["step"]
end
+ test "filtered_path returns path with filtered query string" do
+ %w(; &).each do |sep|
+ request = stub_request('QUERY_STRING' => %w(username=sikachu secret=bd4f21f api_key=b1bc3b3cd352f68d79d7).join(sep),
+ 'PATH_INFO' => '/authenticate',
+ 'action_dispatch.parameter_filter' => [:secret, :api_key])
+
+ path = request.filtered_path
+ assert_equal %w(/authenticate?username=sikachu secret=[FILTERED] api_key=[FILTERED]).join(sep), path
+ end
+ end
+
+ test "filtered_path should not unescape a genuine '[FILTERED]' value" do
+ request = stub_request('QUERY_STRING' => "secret=bd4f21f&genuine=%5BFILTERED%5D",
+ 'PATH_INFO' => '/authenticate',
+ 'action_dispatch.parameter_filter' => [:secret])
+
+ path = request.filtered_path
+ assert_equal "/authenticate?secret=[FILTERED]&genuine=%5BFILTERED%5D", path
+ end
+
+ test "filtered_path should preserve duplication of keys in query string" do
+ request = stub_request('QUERY_STRING' => "username=sikachu&secret=bd4f21f&username=fxn",
+ 'PATH_INFO' => '/authenticate',
+ 'action_dispatch.parameter_filter' => [:secret])
+
+ path = request.filtered_path
+ assert_equal "/authenticate?username=sikachu&secret=[FILTERED]&username=fxn", path
+ end
+
+ test "filtered_path should ignore searchparts" do
+ request = stub_request('QUERY_STRING' => "secret",
+ 'PATH_INFO' => '/authenticate',
+ 'action_dispatch.parameter_filter' => [:secret])
+
+ path = request.filtered_path
+ assert_equal "/authenticate?secret", path
+ end
+
protected
def stub_request(env = {})
@@ -1,3 +1,4 @@
+require 'thread'
require 'active_support/core_ext/class/attribute_accessors'
module ActiveSupport
@@ -1,5 +1,6 @@
require 'active_support/core_ext/hash/slice'
require "rails/generators/rails/app/app_generator"
+require 'date'
module Rails
class PluginBuilder
@@ -19,7 +19,7 @@ def call(env)
def before_dispatch(env)
request = ActionDispatch::Request.new(env)
- path = request.fullpath
+ path = request.filtered_path
info "\n\nStarted #{request.request_method} \"#{path}\" " \
"for #{request.ip} at #{Time.now.to_default_s}"

0 comments on commit 17ea204

Please sign in to comment.