Changeset 6862

Timestamp:

05/26/07 20:07:34 (1 year ago)

Author:

bitsweat

Message:

Routing: respond with 405 Method Not Allowed status when the route path matches but the HTTP method does not. Closes #6953.

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/base.rb (modified) (1 diff)
• trunk/actionpack/lib/action_controller/rescue.rb (modified) (2 diffs)
• trunk/actionpack/lib/action_controller/routing.rb (modified) (2 diffs)
• trunk/actionpack/test/controller/rescue_test.rb (modified) (3 diffs)
• trunk/actionpack/test/controller/resources_test.rb (modified) (2 diffs)
• trunk/actionpack/test/controller/routing_test.rb (modified) (2 diffs)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Routing: respond with 405 Method Not Allowed status when the route path matches but the HTTP method does not.  #6953 [Josh Peek, defeated, Dan Kubb, Coda Hale]
-
-* Add support for assert_select_rjs with :show and :hide. #7780 [dchelimsky]

trunk/actionpack/lib/action_controller/base.rb

@@ -22 +22 @@
-      @failures = failures
-    end
+  end
+  class MethodNotAllowed < ActionControllerError #:nodoc:
+    attr_reader :allowed_methods
+
+    def initialize(*allowed_methods)
+      super("Only #{allowed_methods.to_sentence} requests are allowed.")
+      @allowed_methods = allowed_methods
+    end
+
+    def allowed_methods_header
+      allowed_methods.map { |method_symbol| method_symbol.to_s.upcase } * ', '
+    end
+
+    def handle_response!(response)
+      response.headers['Allow'] ||= allowed_methods_header
+    end
+  end
+  class NotImplemented < MethodNotAllowed #:nodoc:
-  end
-  class UnknownController < ActionControllerError #:nodoc:

trunk/actionpack/lib/action_controller/rescue.rb

@@ -14 +14 @@
-    DEFAULT_RESCUE_RESPONSE = :internal_server_error
-    DEFAULT_RESCUE_RESPONSES = {
-
-
-
-
-
-
+
+
+
+
+
+
+
+
-    }
-
@@ -56 +58 @@
-        log_error(exception) if logger
-        erase_results if performed?
+
+        # Let the exception alter the response if it wants.
+        # For example, MethodNotAllowed sets the Allow header.
+        if exception.respond_to?(:handle_response!)
+          exception.handle_response!(response)
+        end
-
-        if consider_all_requests_local || local_request?

trunk/actionpack/lib/action_controller/routing.rb

@@ -251 +251 @@
-    # TODO: , (comma) should be an allowed path character.
-    SEPARATORS = %w( / ; . , ? )
+
+    HTTP_METHODS = [:get, :head, :post, :put, :delete]
-
-    # The root paths which may contain controller files
@@ -1346 +1348 @@
-          result = route.recognize(path, environment) and return result
-        end
-
+
+
+
+
+
+
+
+
+
+
-      end
-  

trunk/actionpack/test/controller/rescue_test.rb

@@ -7 +7 @@
-    render :text => 'already rendered'
-    raise "don't panic!"
+  end
+
+  def method_not_allowed
+    raise ActionController::MethodNotAllowed.new(:get, :head, :put)
+  end
+  
+  def not_implemented
+    raise ActionController::NotImplemented.new(:get, :put)
-  end
-end
@@ -145 +153 @@
-    assert_equal :unprocessable_entity, responses['ActiveRecord::RecordInvalid']
-    assert_equal :unprocessable_entity, responses['ActiveRecord::RecordNotSaved']
+    assert_equal :method_not_allowed, responses['ActionController::MethodNotAllowed']
+    assert_equal :not_implemented, responses['ActionController::NotImplemented']
-  end
-
@@ -176 +186 @@
-      assert_nil @controller.send(:clean_backtrace, Exception.new)
-    end
+  end
+  
+  def test_not_implemented
+    with_all_requests_local false do
+      head :not_implemented
+    end
+    assert_response :not_implemented
+    assert_equal "GET, PUT", @response.headers['Allow']
+  end
+
+  def test_method_not_allowed
+    with_all_requests_local false do
+      get :method_not_allowed
+    end
+    assert_response :method_not_allowed
+    assert_equal "GET, HEAD, PUT", @response.headers['Allow']
-  end
-

trunk/actionpack/test/controller/resources_test.rb

@@ -189 +189 @@
-      assert_restful_routes_for :messages do |options|
-        assert_recognizes(options.merge(:action => "new"), :path => "/messages/new", :method => :get)
-RoutingError
+MethodNotAllowed
-          ActionController::Routing::Routes.recognize_path("/messages/new", :method => :post)
-        end
@@ -385 +385 @@
-      collection_path = "/#{controller_name}"
-
-RoutingError
+MethodNotAllowed
-        assert_recognizes(options.merge(:action => 'update'), :path => collection_path, :method => :put)
-      end
-
-RoutingError
+MethodNotAllowed
-        assert_recognizes(options.merge(:action => 'destroy'), :path => collection_path, :method => :delete)
-      end

trunk/actionpack/test/controller/routing_test.rb

@@ -1591 +1591 @@
-    assert_equal("update", request.path_parameters[:action])
-
-
-
+
+
+
+
+
+
+
-
-    request.path = "/people/5"
@@ -1609 +1614 @@
-    assert_equal("destroy", request.path_parameters[:action])
-    assert_equal("5", request.path_parameters[:id])
-
-
-
-
+
+
+
+
+
+
+
+
+
-  ensure
-    Object.send(:remove_const, :PeopleController)