diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 736b8a734769b34bcee1143f6162b8912d58e8dc..a84487789a7041f71a1d5e51fb63de8a473815ee 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Don't let arbitrary classes match as controllers -- a potentially dangerous bug. [Nicholas Seckar] + * Fix Routing tests. Fix routing where failing to match a controller would prevent the rest of routes from being attempted. [Nicholas Seckar] * Add :builder => option to form_for and friends. [Nicholas Seckar, Rick Olson] diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index a779b443ef7a4b1a9fcd20cf2f18bcae6d86a317..af9baa54af71c3c7145eeaea17fb84269bef8ef5 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -234,9 +234,10 @@ def traverse_to_controller(segments, start_at = 0) suppress(NameError) do controller = eval("mod::#{controller_name}", nil, __FILE__, __LINE__) + expected_name = "#{mod.name}::#{controller_name}" # Detect the case when const_get returns an object from a parent namespace. - if mod == Object || controller.name == "#{mod.name}::#{controller_name}" + if controller.is_a?(Class) && controller.ancestors.include?(ActionController::Base) && (mod == Object || controller.name == expected_name) return controller, (index - start_at) end end diff --git a/actionpack/test/controller/fake_controllers.rb b/actionpack/test/controller/fake_controllers.rb index 8c626acab34874cfce580f1c7b944a5b05e14d2a..5f958b284559e2aa690148d2ff5f188b254a005b 100644 --- a/actionpack/test/controller/fake_controllers.rb +++ b/actionpack/test/controller/fake_controllers.rb @@ -2,6 +2,8 @@ class << Object; alias_method :const_available?, :const_defined?; end class ContentController < Class.new(ActionController::Base) end +class NotAController +end module Admin class << self; alias_method :const_available?, :const_defined?; end class UserController < Class.new(ActionController::Base); end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index daf2c8e5ede292c891c473c492068a70b3a4e034..b57090964107c390a135a899df0c47a389d47532 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -643,7 +643,7 @@ def test_default_setup assert_equal ['/admin/stuff', []], rs.generate({:controller => 'stuff'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) assert_equal ['/stuff', []], rs.generate({:controller => '/stuff'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) end - + def test_ignores_leading_slash @rs.draw {|m| m.connect '/:controller/:action/:id'} test_default_setup @@ -802,6 +802,13 @@ def test_paths_escaped assert results, "Recognition should have succeeded" assert_equal [], results['path'] end + + def test_non_controllers_cannot_be_matched + rs.draw do + rs.connect ':controller/:action/:id' + end + assert_nil rs.recognize_path(%w(not_a show 10)), "Shouldn't recognize non-controllers as controllers!" + end def test_paths_do_not_accept_defaults assert_raises(ActionController::RoutingError) do