From 39caa790999820483edbd8ac1ff20215e62f349b Mon Sep 17 00:00:00 2001
From: Tom Ward <tom@popdog.net>
Date: Fri, 18 Jul 2008 14:23:22 +0100
Subject: [PATCH] Raise ArgumentError if an invalid method is specified as part of a route's conditions.  Also raise an error if HEAD is specified as the method, as rails routes all HEAD requests through the equivalent GET, though doesn't return the response body (see lighthouse #182).

---
 actionpack/lib/action_controller/resources.rb      |    1 +
 .../lib/action_controller/routing/builder.rb       |   18 +++++++++++++++++-
 actionpack/test/controller/resources_test.rb       |   20 ++++++++++++++++++++
 actionpack/test/controller/routing_test.rb         |   16 ++++++++++++++++
 4 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb
index b11aa56..1fd0023 100644
--- a/actionpack/lib/action_controller/resources.rb
+++ b/actionpack/lib/action_controller/resources.rb
@@ -559,6 +559,7 @@ module ActionController
       def action_options_for(action, resource, method = nil)
         default_options = { :action => action.to_s }
         require_id = !resource.kind_of?(SingletonResource)
+        
         case default_options[:action]
           when "index", "new"; default_options.merge(add_conditions_for(resource.conditions, method || :get)).merge(resource.requirements)
           when "create";       default_options.merge(add_conditions_for(resource.conditions, method || :post)).merge(resource.requirements)
diff --git a/actionpack/lib/action_controller/routing/builder.rb b/actionpack/lib/action_controller/routing/builder.rb
index b832384..b1c0ea5 100644
--- a/actionpack/lib/action_controller/routing/builder.rb
+++ b/actionpack/lib/action_controller/routing/builder.rb
@@ -75,7 +75,9 @@ module ActionController
         requirements = (options.delete(:requirements) || {}).dup
         defaults     = (options.delete(:defaults)     || {}).dup
         conditions   = (options.delete(:conditions)   || {}).dup
-
+        
+        validate_route_conditions(conditions) 
+        
         path_keys = segments.collect { |segment| segment.key if segment.respond_to?(:key) }.compact
         options.each do |key, value|
           hash = (path_keys.include?(key) && ! value.is_a?(Regexp)) ? defaults : requirements
@@ -198,6 +200,20 @@ module ActionController
 
         route
       end
+      
+      private 
+      
+      def validate_route_conditions(conditions)
+        if method = conditions[:method]
+          if method == :head
+            raise ArgumentError, "HTTP method HEAD is invalid in route conditions. Rails processes HEAD requests the same as GETs, returning just the response headers"
+          end
+          
+          unless HTTP_METHODS.include?(method.to_sym)
+            raise ArgumentError, "Invalid HTTP method specified in route conditions: #{conditions.inspect}"
+          end
+        end
+      end
     end
   end
 end
diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb
index 0f79246..fbcf638 100644
--- a/actionpack/test/controller/resources_test.rb
+++ b/actionpack/test/controller/resources_test.rb
@@ -515,6 +515,26 @@ class ResourcesTest < Test::Unit::TestCase
       end
     end
   end
+  
+  def test_should_not_allow_invalid_head_method_for_member_routes
+    with_routing do |set|
+      set.draw do |map|
+        assert_raises(ArgumentError) do
+          map.resources :messages, :member => {:something => :head}
+        end
+      end
+    end
+  end
+
+  def test_should_not_allow_invalid_http_methods_for_member_routes
+    with_routing do |set|
+      set.draw do |map|
+        assert_raises(ArgumentError) do
+          map.resources :messages, :member => {:something => :invalid}
+        end
+      end
+    end
+  end
 
   def test_resource_action_separator
     with_routing do |set|
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index c5ccb71..c71280c 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -1800,6 +1800,22 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
         end
       end
     end
+    
+    def test_route_requirements_with_invalid_http_method_is_invalid
+      assert_raises ArgumentError do
+        set.draw do |map|
+          map.connect 'valid/route', :controller => 'pages', :action => 'show', :conditions => {:method => :invalid}
+        end
+      end
+    end
+    
+    def test_route_requirements_with_head_method_condition_is_invalid
+      assert_raises ArgumentError do
+        set.draw do |map|
+          map.connect 'valid/route', :controller => 'pages', :action => 'show', :conditions => {:method => :head}
+        end
+      end
+    end
 
     def test_non_path_route_requirements_match_all
       set.draw do |map|
-- 
1.5.5.1