From 8139de2812a316aad41c009d03b2d1e0dfb6770c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 29 May 2007 06:40:33 +0000 Subject: Routing: drop semicolon and comma as route separators. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6888 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/routing.rb | 6 ++--- actionpack/test/controller/routing_test.rb | 42 ++++++++++++++++------------- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index f655ecbede..7bd1a71d7c 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Routing: drop semicolon and comma as route separators. [Jeremy Kemper] + * request.remote_ip understands X-Forwarded-For addresses with nonstandard whitespace. #7386 [moses] * Don't prepare response when rendering a component. #8493 [jsierles] diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 34041cc0fc..74f09f815c 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -247,8 +247,7 @@ module ActionController # end # module Routing - # TODO: , (comma) should be an allowed path character. - SEPARATORS = %w( / ; . , ? ) + SEPARATORS = %w( / . ? ) HTTP_METHODS = [:get, :head, :post, :put, :delete] @@ -549,8 +548,7 @@ module ActionController end class Segment #:nodoc: - # TODO: , (comma) should be an allowed path character. - RESERVED_PCHAR = ':@&=+$' + RESERVED_PCHAR = ':@&=+$,;' UNSAFE_PCHAR = Regexp.new("[^#{URI::REGEXP::PATTERN::UNRESERVED}#{RESERVED_PCHAR}]", false, 'N').freeze attr_accessor :is_optional diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 7efb6b8a6e..951bca6b0d 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -22,8 +22,7 @@ class UriReservedCharactersRoutingTest < Test::Unit::TestCase map.connect ':controller/:action/:variable' end - # TODO: perhaps , (comma) shouldn't be a route separator. - safe, unsafe = %w(: @ & = + $), %w(, ^ / ? # [ ] ;) + safe, unsafe = %w(: @ & = + $ , ;), %w(^ / ? # [ ]) hex = unsafe.map { |char| '%' + char.unpack('H2').first.upcase } @segment = "#{safe}#{unsafe}".freeze @@ -532,21 +531,21 @@ class LegacyRouteSetTests < Test::Unit::TestCase Object.const_set(:SubpathBooksController, Class.new(ActionController::Base)) rs.draw do |r| - r.connect '/books/:id;edit', :controller => 'subpath_books', :action => 'edit' - r.connect '/items/:id;:action', :controller => 'subpath_books' - r.connect '/posts/new;:action', :controller => 'subpath_books' + r.connect '/books/:id/edit', :controller => 'subpath_books', :action => 'edit' + r.connect '/items/:id/:action', :controller => 'subpath_books' + r.connect '/posts/new/:action', :controller => 'subpath_books' r.connect '/posts/:id', :controller => 'subpath_books', :action => "show" end - hash = rs.recognize_path "/books/17;edit" + hash = rs.recognize_path "/books/17/edit" assert_not_nil hash assert_equal %w(subpath_books 17 edit), [hash[:controller], hash[:id], hash[:action]] - hash = rs.recognize_path "/items/3;complete" + hash = rs.recognize_path "/items/3/complete" assert_not_nil hash assert_equal %w(subpath_books 3 complete), [hash[:controller], hash[:id], hash[:action]] - hash = rs.recognize_path "/posts/new;preview" + hash = rs.recognize_path "/posts/new/preview" assert_not_nil hash assert_equal %w(subpath_books preview), [hash[:controller], hash[:action]] @@ -561,14 +560,14 @@ class LegacyRouteSetTests < Test::Unit::TestCase Object.const_set(:SubpathBooksController, Class.new(ActionController::Base)) rs.draw do |r| - r.connect '/books/:id;edit', :controller => 'subpath_books', :action => 'edit' - r.connect '/items/:id;:action', :controller => 'subpath_books' - r.connect '/posts/new;:action', :controller => 'subpath_books' + r.connect '/books/:id/edit', :controller => 'subpath_books', :action => 'edit' + r.connect '/items/:id/:action', :controller => 'subpath_books' + r.connect '/posts/new/:action', :controller => 'subpath_books' end - assert_equal "/books/7;edit", rs.generate(:controller => "subpath_books", :id => 7, :action => "edit") - assert_equal "/items/15;complete", rs.generate(:controller => "subpath_books", :id => 15, :action => "complete") - assert_equal "/posts/new;preview", rs.generate(:controller => "subpath_books", :action => "preview") + assert_equal "/books/7/edit", rs.generate(:controller => "subpath_books", :id => 7, :action => "edit") + assert_equal "/items/15/complete", rs.generate(:controller => "subpath_books", :id => 15, :action => "complete") + assert_equal "/posts/new/preview", rs.generate(:controller => "subpath_books", :action => "preview") ensure Object.send(:remove_const, :SubpathBooksController) rescue nil end @@ -1178,12 +1177,19 @@ class RouteBuilderTest < Test::Unit::TestCase assert_equal nil, builder.warn_output # should only warn on the :person segment end - def test_segmentation_of_semicolon_path + def test_comma_isnt_a_route_separator + segments = builder.segments_for_route_path '/books/:id,:action' + defaults = { :action => 'show' } + assert_raise(ArgumentError) do + builder.assign_route_options(segments, defaults, {}) + end + end + + def test_semicolon_isnt_a_route_separator segments = builder.segments_for_route_path '/books/:id;:action' defaults = { :action => 'show' } - assert builder.assign_route_options(segments, defaults, {}).empty? - segments.each do |segment| - assert ! segment.optional? || segment.key == :action + assert_raise(ArgumentError) do + builder.assign_route_options(segments, defaults, {}) end end -- cgit v1.2.3