Permalink
Browse files

Routing: drop semicolon and comma as route separators.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6888 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent d3017e2 commit 8139de2812a316aad41c009d03b2d1e0dfb6770c @jeremy jeremy committed May 29, 2007
Showing with 28 additions and 22 deletions.
  1. +2 −0 actionpack/CHANGELOG
  2. +2 −4 actionpack/lib/action_controller/routing.rb
  3. +24 −18 actionpack/test/controller/routing_test.rb
@@ -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]
@@ -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 @@ def requirement_for(key)
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
@@ -22,8 +22,7 @@ def setup
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 @@ def test_subpath_recognized
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 @@ def test_subpath_generated
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 @@ def test_optional_segments_preceding_required_segments
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

0 comments on commit 8139de2

Please sign in to comment.