Don't let arbitrary classes match as controllers

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3542 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

4 files changed, 14 insertions, 2 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 736b8a7347..a84487789a 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 a779b443ef..af9baa54af 100644
--- a/actionpack/lib/action_controller/routing.rb
+++ b/actionpack/lib/action_controller/routing.rb
@@ -234,9 +234,10 @@ module ActionController
             
             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 8c626acab3..5f958b2845 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 daf2c8e5ed..b570909641 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -643,7 +643,7 @@ class RouteSetTests < Test::Unit::TestCase
     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 @@ class RouteSetTests < Test::Unit::TestCase
     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