Skip to content
Browse files

Raise ArgumentError instead of normalizing controller name when there…

… is a leading slash [#5651 state:resolved]
  • Loading branch information...
1 parent 301462c commit b79a782a05df50028c04b4336d45875352e89d12 @pixeltrix pixeltrix committed
Showing with 35 additions and 7 deletions.
  1. +5 −1 actionpack/lib/action_dispatch/routing/mapper.rb
  2. +30 −6 actionpack/test/dispatch/routing_test.rb
View
6 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -128,7 +128,7 @@ def default_controller_and_action(to_shorthand=nil)
{ }
else
if to.is_a?(String)
- controller, action = to.sub(%r{\A/}, '').split('#')
+ controller, action = to.split('#')
elsif to.is_a?(Symbol)
action = to.to_s
end
@@ -140,6 +140,10 @@ def default_controller_and_action(to_shorthand=nil)
controller = [@scope[:module], controller].compact.join("/").presence
end
+ if controller.is_a?(String) && controller =~ %r{\A/}
+ raise ArgumentError, "controller name should not start with a slash"
+ end
+
controller = controller.to_s unless controller.is_a?(Regexp)
action = action.to_s unless action.is_a?(Regexp)
View
36 actionpack/test/dispatch/routing_test.rb
@@ -457,8 +457,6 @@ def self.matches?(request)
match '/cities', :to => 'countries#cities'
end
- match '/feeds/:service', :to => '/api/feeds#show', :as => :feed
-
match '/countries/:country/(*other)', :to => redirect{ |params, req| params[:other] ? "/countries/all/#{params[:other]}" : '/countries/all' }
match '/:locale/*file.:format', :to => 'files#show', :file => /path\/to\/existing\/file/
@@ -2130,10 +2128,36 @@ def test_nested_resource_constraints
assert_raises(ActionController::RoutingError){ list_todo_path(:list_id => '2', :id => '1') }
end
- def test_controller_has_leading_slash_removed
- get '/feeds/twitter.xml'
- assert_equal 'api/feeds#show', @response.body
- assert_equal '/feeds/twitter.xml', feed_path(:service => 'twitter', :format => 'xml')
+ def test_controller_name_with_leading_slash_raise_error
+ assert_raise(ArgumentError) do
+ self.class.stub_controllers do |routes|
+ routes.draw { get '/feeds/:service', :to => '/feeds#show' }
+ end
+ end
+
+ assert_raise(ArgumentError) do
+ self.class.stub_controllers do |routes|
+ routes.draw { get '/feeds/:service', :controller => '/feeds', :action => 'show' }
+ end
+ end
+
+ assert_raise(ArgumentError) do
+ self.class.stub_controllers do |routes|
+ routes.draw { get '/api/feeds/:service', :to => '/api/feeds#show' }
+ end
+ end
+
+ assert_raise(ArgumentError) do
+ self.class.stub_controllers do |routes|
+ routes.draw { controller("/feeds") { get '/feeds/:service', :to => :show } }
+ end
+ end
+
+ assert_raise(ArgumentError) do
+ self.class.stub_controllers do |routes|
+ routes.draw { resources :feeds, :controller => '/feeds' }
+ end
+ end
end
private

2 comments on commit b79a782

@sikachu
Ruby on Rails member

This patch looks good. Thank you :)

@EmmanuelOga

Nice, thank you

Please sign in to comment.
Something went wrong with that request. Please try again.