public
Description: Ruby on Rails
Homepage: http://rubyonrails.org
Clone URL: git://github.com/rails/rails.git
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>
tomafro (author)
Fri Jul 18 18:19:03 -0700 2008
josh (committer)
Fri Jul 18 18:19:03 -0700 2008
commit  d39485078ec56e25a96e97d44b53498d8a1c7426
tree    6c7a23d8c91536a8e79d38372d710baffc42c8d2
parent  c3d1fda555c4bd5f8821d830c685ae5d0e7e52d0
...
307
308
309
310
 
311
312
313
314
 
 
315
316
 
317
318
319
...
559
560
561
 
562
563
564
...
307
308
309
 
310
311
312
 
 
313
314
315
 
316
317
318
319
...
559
560
561
562
563
564
565
0
@@ -307,13 +307,13 @@ module ActionController
0
     #     map.resources :tags, :path_prefix => '/toys/:toy_id',   :name_prefix => 'toy_'
0
     #
0
     # You may also use <tt>:name_prefix</tt> to override the generic named routes in a nested resource:
0
-    # 
0
+    #
0
     #   map.resources :articles do |article|
0
     #     article.resources :comments, :name_prefix => nil
0
-    #   end 
0
-    # 
0
+    #   end
0
+    #
0
     # This will yield named resources like so:
0
-    # 
0
+    #
0
     #   comments_url(@article)
0
     #   comment_url(@article, @comment)
0
     #
0
@@ -559,6 +559,7 @@ module ActionController
0
       def action_options_for(action, resource, method = nil)
0
         default_options = { :action => action.to_s }
0
         require_id = !resource.kind_of?(SingletonResource)
0
+
0
         case default_options[:action]
0
           when "index", "new"; default_options.merge(add_conditions_for(resource.conditions, method || :get)).merge(resource.requirements)
0
           when "create";       default_options.merge(add_conditions_for(resource.conditions, method || :post)).merge(resource.requirements)
...
76
77
78
 
 
79
80
81
...
198
199
200
 
 
 
 
 
 
 
 
 
 
 
 
 
201
202
203
...
76
77
78
79
80
81
82
83
...
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
0
@@ -76,6 +76,8 @@ module ActionController
0
         defaults     = (options.delete(:defaults)     || {}).dup
0
         conditions   = (options.delete(:conditions)   || {}).dup
0
 
0
+        validate_route_conditions(conditions)
0
+
0
         path_keys = segments.collect { |segment| segment.key if segment.respond_to?(:key) }.compact
0
         options.each do |key, value|
0
           hash = (path_keys.include?(key) && ! value.is_a?(Regexp)) ? defaults : requirements
0
@@ -198,6 +200,19 @@ module ActionController
0
 
0
         route
0
       end
0
+
0
+      private
0
+        def validate_route_conditions(conditions)
0
+          if method = conditions[:method]
0
+            if method == :head
0
+              raise ArgumentError, "HTTP method HEAD is invalid in route conditions. Rails processes HEAD requests the same as GETs, returning just the response headers"
0
+            end
0
+
0
+            unless HTTP_METHODS.include?(method.to_sym)
0
+              raise ArgumentError, "Invalid HTTP method specified in route conditions: #{conditions.inspect}"
0
+            end
0
+          end
0
+        end
0
     end
0
   end
0
 end
...
516
517
518
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
519
520
521
...
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
0
@@ -516,6 +516,26 @@ class ResourcesTest < Test::Unit::TestCase
0
     end
0
   end
0
 
0
+  def test_should_not_allow_invalid_head_method_for_member_routes
0
+    with_routing do |set|
0
+      set.draw do |map|
0
+        assert_raises(ArgumentError) do
0
+          map.resources :messages, :member => {:something => :head}
0
+        end
0
+      end
0
+    end
0
+  end
0
+
0
+  def test_should_not_allow_invalid_http_methods_for_member_routes
0
+    with_routing do |set|
0
+      set.draw do |map|
0
+        assert_raises(ArgumentError) do
0
+          map.resources :messages, :member => {:something => :invalid}
0
+        end
0
+      end
0
+    end
0
+  end
0
+
0
   def test_resource_action_separator
0
     with_routing do |set|
0
       set.draw do |map|
...
1801
1802
1803
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1804
1805
1806
...
1801
1802
1803
1804
1805
1806
1807
1808
1809
1810
1811
1812
1813
1814
1815
1816
1817
1818
1819
1820
1821
1822
0
@@ -1801,6 +1801,22 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
0
       end
0
     end
0
 
0
+    def test_route_requirements_with_invalid_http_method_is_invalid
0
+      assert_raises ArgumentError do
0
+        set.draw do |map|
0
+          map.connect 'valid/route', :controller => 'pages', :action => 'show', :conditions => {:method => :invalid}
0
+        end
0
+      end
0
+    end
0
+
0
+    def test_route_requirements_with_head_method_condition_is_invalid
0
+      assert_raises ArgumentError do
0
+        set.draw do |map|
0
+          map.connect 'valid/route', :controller => 'pages', :action => 'show', :conditions => {:method => :head}
0
+        end
0
+      end
0
+    end
0
+
0
     def test_non_path_route_requirements_match_all
0
       set.draw do |map|
0
         map.connect 'page/37s', :controller => 'pages', :action => 'show', :name => /(jamis|david)/

Comments

matthewrudy Fri Jul 18 19:10:45 -0700 2008

@tomafro – you should go to bed instead of submitting patches.