From 148202d401c7114c8ceb69f4f9effaba38461124 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 15 Sep 2007 22:10:20 +0000 Subject: Fixed optimized route segment escaping. Closes #9562. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7487 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 + actionpack/lib/action_controller/routing.rb | 53 +++++++++++----------- .../lib/action_controller/routing_optimisation.rb | 5 +- actionpack/test/controller/routing_test.rb | 8 ++++ 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6b2f30f1b1..f8ac1237ea 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Fixed optimized route segment escaping. #9562 [wildchild, Jeremy Kemper] + * root_path returns '/' not ''. #9563 [lifofifo] * Fixed that setting request.format should also affect respond_to blocks [DHH] diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index bbf6eefd8f..adeb70235c 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -718,8 +718,8 @@ module ActionController s << "\n#{expiry_statement}" end - def interpolation_chunk - "\#{URI.escape(#{local_name}.to_s, ActionController::Routing::Segment::UNSAFE_PCHAR)}" + def interpolation_chunk(value_code = "#{local_name}.to_s") + "\#{URI.escape(#{value_code}, ActionController::Routing::Segment::UNSAFE_PCHAR)}" end def string_structure(prior_segments) @@ -776,8 +776,8 @@ module ActionController end # Don't URI.escape the controller name since it may contain slashes. - def interpolation_chunk - "\#{#{local_name}.to_s}" + def interpolation_chunk(value_code = "#{local_name}.to_s") + "\#{#{value_code}}" end # Make sure controller names like Admin/Content are correctly normalized to @@ -799,8 +799,8 @@ module ActionController RESERVED_PCHAR = "#{Segment::RESERVED_PCHAR}/" UNSAFE_PCHAR = Regexp.new("[^#{URI::REGEXP::PATTERN::UNRESERVED}#{RESERVED_PCHAR}]", false, 'N').freeze - def interpolation_chunk - "\#{URI.escape(#{local_name}.to_s, ActionController::Routing::PathSegment::UNSAFE_PCHAR)}" + def interpolation_chunk(value_code = "#{local_name}.to_s") + "\#{URI.escape(#{value_code}, ActionController::Routing::PathSegment::UNSAFE_PCHAR)}" end def default @@ -1150,34 +1150,33 @@ module ActionController @module.send(:protected, selector) helpers << selector end - + def define_url_helper(route, name, kind, options) selector = url_helper_name(name, kind) # The segment keys used for positional paramters - + hash_access_method = hash_access_name(name, kind) - + + # allow ordered parameters to be associated with corresponding + # dynamic segments, so you can do + # + # foo_url(bar, baz, bang) + # + # instead of + # + # foo_url(:bar => bar, :baz => baz, :bang => bang) + # + # Also allow options hash, so you can do + # + # foo_url(bar, baz, bang, :sort_by => 'baz') + # @module.send :module_eval, <<-end_eval # We use module_eval to avoid leaks def #{selector}(*args) - #{generate_optimisation_block(route, kind)} - + opts = if args.empty? || Hash === args.first args.first || {} else - # allow ordered parameters to be associated with corresponding - # dynamic segments, so you can do - # - # foo_url(bar, baz, bang) - # - # instead of - # - # foo_url(:bar => bar, :baz => baz, :bang => bang) - # - # Also allow options hash, so you can do - # - # foo_url(bar, baz, bang, :sort_by => 'baz') - # options = args.last.is_a?(Hash) ? args.pop : {} args = args.zip(#{route.segment_keys.inspect}).inject({}) do |h, (v, k)| h[k] = v @@ -1185,7 +1184,7 @@ module ActionController end options.merge(args) end - + url_for(#{hash_access_method}(opts)) end end_eval @@ -1193,7 +1192,7 @@ module ActionController helpers << selector end end - + attr_accessor :routes, :named_routes def initialize @@ -1445,4 +1444,4 @@ module ActionController Routes = RouteSet.new end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/routing_optimisation.rb b/actionpack/lib/action_controller/routing_optimisation.rb index 1b447b17e7..eaff1869dd 100644 --- a/actionpack/lib/action_controller/routing_optimisation.rb +++ b/actionpack/lib/action_controller/routing_optimisation.rb @@ -56,7 +56,6 @@ module ActionController elements = [] idx = 0 - if kind == :url elements << '#{request.protocol}' elements << '#{request.host_with_port}' @@ -67,10 +66,10 @@ module ActionController # we don't include the trailing slashes, so skip them. ((route.segments.size == 1 && kind == :path) ? route.segments : route.segments[0..-2]).each do |segment| if segment.is_a?(DynamicSegment) - elements << "\#{URI.escape(args[#{idx}].to_param, ActionController::Routing::Segment::UNSAFE_PCHAR)}" + elements << segment.interpolation_chunk("args[#{idx}].to_param") idx += 1 else - elements << segment.to_s + elements << segment.interpolation_chunk end end %("#{elements * ''}") diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index fee1ad3ccf..793e1d8cdf 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -273,6 +273,14 @@ class LegacyRouteSetTests < Test::Unit::TestCase assert_equal [], results[:path] end + def test_paths_slashes_unescaped_with_ordered_parameters + rs.add_named_route :path, '/file/*path', :controller => 'content' + + # No / to %2F in URI, only for query params. + x = setup_for_named_route + assert_equal("/file/hello/world", x.send(:path_path, 'hello/world')) + end + def test_non_controllers_cannot_be_matched rs.draw do |map| map.connect ':controller/:action/:id' -- cgit v1.2.3