From d39485078ec56e25a96e97d44b53498d8a1c7426 Mon Sep 17 00:00:00 2001
From: Tom Ward <tom@popdog.net>
Date: Fri, 18 Jul 2008 20:19:03 -0500
Subject: 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 [#182 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>
---
 actionpack/lib/action_controller/resources.rb       |  9 +++++----
 actionpack/lib/action_controller/routing/builder.rb | 15 +++++++++++++++
 actionpack/test/controller/resources_test.rb        | 20 ++++++++++++++++++++
 actionpack/test/controller/routing_test.rb          | 16 ++++++++++++++++
 4 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb
index b11aa5625b..0614b9a4d9 100644
--- a/actionpack/lib/action_controller/resources.rb
+++ b/actionpack/lib/action_controller/resources.rb
@@ -307,13 +307,13 @@ module ActionController
     #     map.resources :tags, :path_prefix => '/toys/:toy_id',   :name_prefix => 'toy_'
     #
     # You may also use <tt>:name_prefix</tt> to override the generic named routes in a nested resource:
-    # 
+    #
     #   map.resources :articles do |article|
     #     article.resources :comments, :name_prefix => nil
-    #   end 
-    # 
+    #   end
+    #
     # This will yield named resources like so:
-    # 
+    #
     #   comments_url(@article)
     #   comment_url(@article, @comment)
     #
@@ -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 b8323847fd..912999d845 100644
--- a/actionpack/lib/action_controller/routing/builder.rb
+++ b/actionpack/lib/action_controller/routing/builder.rb
@@ -76,6 +76,8 @@ module ActionController
         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,19 @@ 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 0f7924649a..e153b0cc98 100644
--- a/actionpack/test/controller/resources_test.rb
+++ b/actionpack/test/controller/resources_test.rb
@@ -516,6 +516,26 @@ class ResourcesTest < Test::Unit::TestCase
     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|
       set.draw do |map|
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index c5ccb71582..079189d7b3 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -1801,6 +1801,22 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
       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|
         map.connect 'page/37s', :controller => 'pages', :action => 'show', :name => /(jamis|david)/
-- 
cgit v1.2.3