diff options
| -rw-r--r-- | actionpack/CHANGELOG | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/routing.rb | 18 | ||||
| -rw-r--r-- | actionpack/test/controller/routing_test.rb | 40 |
3 files changed, 55 insertions, 6 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 4c4136a105..e131ec0f9e 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,8 @@ *SVN* +* Routing: better support for escaped values in route segments. #7544 [Chris +Roos] + * Introduce a cookie-based session store as the Rails default. Sessions typically contain at most a user_id and flash message; both fit within the 4K cookie size limit. A secure hash is included with the cookie to ensure data integrity (a user cannot alter his user_id without knowing the secret key included in the hash). If you have more than 4K of session data or don't want your data to be visible to the user, pick another session store. Cookie-based sessions are dramatically faster than the alternatives. [Jeremy Kemper] * Added .erb and .builder as preferred aliases to the now deprecated .rhtml and .rxml extensions [Chad Fowler]. This is done to separate the renderer from the mime type. .erb templates are often used to render emails, atom, csv, whatever. So labeling them .rhtml doesn't make too much sense. The same goes for .rxml, which can be used to build everything from HTML to Atom to whatever. .rhtml and .rxml will continue to work until Rails 3.0, though. So this is a slow phasing out. All generators and examples will start using the new aliases, though. diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 58207eed74..723aaada83 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -412,7 +412,7 @@ module ActionController def recognition_extraction next_capture = 1 extraction = segments.collect do |segment| - x = segment.match_extraction next_capture + x = segment.match_extraction(next_capture) next_capture += Regexp.new(segment.regexp_chunk).number_of_captures x end @@ -694,7 +694,7 @@ module ActionController end def interpolation_chunk - "\#{URI.escape(#{local_name}.to_s)}" + "\#{CGI.escape(#{local_name}.to_s)}" end def string_structure(prior_segments) @@ -725,10 +725,17 @@ module ActionController optional? ? Regexp.optionalize(pattern) : pattern end def match_extraction(next_capture) - hangon = (default ? "|| #{default.inspect}" : "if match[#{next_capture}]") - # All non code-related keys (such as :id, :slug) have to be unescaped as other CGI params - "params[:#{key}] = match[#{next_capture}] #{hangon}" + default_value = default ? default.inspect : nil + %[ + value = if (m = match[#{next_capture}]) + m = m.gsub('+', '%2B') + CGI.unescape(m) + else + #{default_value} + end + params[:#{key}] = value if value + ] end def optionality_implied? @@ -1292,7 +1299,6 @@ module ActionController end def recognize_path(path, environment={}) - path = URI.unescape(path) routes.each do |route| result = route.recognize(path, environment) and return result end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index f81d0393f0..5f75bf3876 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -13,6 +13,46 @@ class ROUTING::RouteBuilder end end +class UriReservedCharactersRoutingTest < Test::Unit::TestCase + # See RFC 3986, section 2.2 Reserved Characters + + def setup + ActionController::Routing.use_controllers! ['controller'] + @set = ActionController::Routing::RouteSet.new + @set.draw do |map| + map.connect ':controller/:action/:var' + end + end + + def test_should_escape_reserved_uri_characters_within_individual_path_components + assert_equal '/controller/action/p1%3Ap2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1:p2') + assert_equal '/controller/action/p1%2Fp2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1/p2') + assert_equal '/controller/action/p1%3Fp2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1?p2') + assert_equal '/controller/action/p1%23p2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1#p2') + assert_equal '/controller/action/p1%5Bp2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1[p2') + assert_equal '/controller/action/p1%5Dp2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1]p2') + assert_equal '/controller/action/p1%40p2', @set.generate(:controller => 'controller', :action => 'action', :var => 'p1@p2') + end + + def test_should_recognize_escaped_path_component_and_unescape + expected_options = {:var => "p1:p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%3Ap2') + expected_options = {:var => "p1/p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%2Fp2') + expected_options = {:var => "p1?p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%3Fp2') + expected_options = {:var => "p1#p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%23p2') + expected_options = {:var => "p1[p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%5Bp2') + expected_options = {:var => "p1]p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%5Dp2') + expected_options = {:var => "p1@p2", :controller => "controller", :action => "action"} + assert_equal expected_options, @set.recognize_path('/controller/action/p1%40p2') + end + +end + class LegacyRouteSetTests < Test::Unit::TestCase attr_reader :rs def setup |