Permalink
Browse files

Routing: respond with 405 Method Not Allowed status when the route pa…

…th matches but the HTTP method does not. Closes #6953.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6862 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 42ebf55 commit dcaa074abf5691a933b9c55159cc7d98a02b3b2f @jeremy jeremy committed May 26, 2007
@@ -1,5 +1,7 @@
*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]
* Make assert_select's failure messages clearer about what failed. #7779 [dchelimsky]
@@ -22,6 +22,24 @@ def initialize(message, failures=[])
@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:
end
class UnknownAction < ActionControllerError #:nodoc:
@@ -13,12 +13,14 @@ module Rescue
DEFAULT_RESCUE_RESPONSE = :internal_server_error
DEFAULT_RESCUE_RESPONSES = {
- 'ActionController::RoutingError' => :not_found,
- 'ActionController::UnknownAction' => :not_found,
- 'ActiveRecord::RecordNotFound' => :not_found,
- 'ActiveRecord::StaleObjectError' => :conflict,
- 'ActiveRecord::RecordInvalid' => :unprocessable_entity,
- 'ActiveRecord::RecordNotSaved' => :unprocessable_entity
+ 'ActionController::RoutingError' => :not_found,
+ 'ActionController::UnknownAction' => :not_found,
+ 'ActiveRecord::RecordNotFound' => :not_found,
+ 'ActiveRecord::StaleObjectError' => :conflict,
+ 'ActiveRecord::RecordInvalid' => :unprocessable_entity,
+ 'ActiveRecord::RecordNotSaved' => :unprocessable_entity,
+ 'ActionController::MethodNotAllowed' => :method_not_allowed,
+ 'ActionController::NotImplemented' => :not_implemented
}
DEFAULT_RESCUE_TEMPLATE = 'diagnostics'
@@ -56,6 +58,12 @@ def rescue_action(exception)
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?
rescue_action_locally(exception)
else
@@ -251,6 +251,8 @@ module Routing
# 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
mattr_accessor :controller_paths
self.controller_paths = []
@@ -1345,7 +1347,16 @@ def recognize_path(path, environment={})
routes.each do |route|
result = route.recognize(path, environment) and return result
end
- raise RoutingError, "no route found to match #{path.inspect} with #{environment.inspect}"
+
+ allows = HTTP_METHODS.select { |verb| routes.find { |r| r.recognize(path, :method => verb) } }
+
+ if environment[:method] && !HTTP_METHODS.include?(environment[:method])
+ raise NotImplemented.new(*allows)
+ elsif !allows.empty?
+ raise MethodNotAllowed.new(*allows)
+ else
+ raise RoutingError, "No route matches #{path.inspect} with #{environment.inspect}"
+ end
end
def routes_by_controller
@@ -7,6 +7,14 @@ def raises
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
@@ -144,6 +152,8 @@ def test_rescue_responses
assert_equal :conflict, responses['ActiveRecord::StaleObjectError']
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
def test_rescue_templates
@@ -176,6 +186,22 @@ def test_clean_backtrace
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
protected
def with_all_requests_local(local = true)
@@ -188,7 +188,7 @@ def test_override_new_method
with_restful_routing :messages do
assert_restful_routes_for :messages do |options|
assert_recognizes(options.merge(:action => "new"), :path => "/messages/new", :method => :get)
- assert_raises(ActionController::RoutingError) do
+ assert_raises(ActionController::MethodNotAllowed) do
ActionController::Routing::Routes.recognize_path("/messages/new", :method => :post)
end
end
@@ -384,11 +384,11 @@ def test_should_not_allow_delete_or_put_on_collection_path
options = { :controller => controller_name.to_s }
collection_path = "/#{controller_name}"
- assert_raises(ActionController::RoutingError) do
+ assert_raises(ActionController::MethodNotAllowed) do
assert_recognizes(options.merge(:action => 'update'), :path => collection_path, :method => :put)
end
- assert_raises(ActionController::RoutingError) do
+ assert_raises(ActionController::MethodNotAllowed) do
assert_recognizes(options.merge(:action => 'destroy'), :path => collection_path, :method => :delete)
end
end
@@ -1590,8 +1590,13 @@ def test_recognize_with_conditions
assert_nothing_raised { set.recognize(request) }
assert_equal("update", request.path_parameters[:action])
- request.method = :update
- assert_raises(ActionController::RoutingError) { set.recognize(request) }
+ begin
+ request.method = :bacon
+ set.recognize(request)
+ flunk 'Should have raised NotImplemented'
+ rescue ActionController::NotImplemented => e
+ assert_equal [:get, :post, :put, :delete], e.allowed_methods
+ end
request.path = "/people/5"
request.method = :get
@@ -1608,10 +1613,15 @@ def test_recognize_with_conditions
assert_nothing_raised { set.recognize(request) }
assert_equal("destroy", request.path_parameters[:action])
assert_equal("5", request.path_parameters[:id])
-
- request.method = :post
- assert_raises(ActionController::RoutingError) { set.recognize(request) }
-
+
+ begin
+ request.method = :post
+ set.recognize(request)
+ flunk 'Should have raised MethodNotAllowed'
+ rescue ActionController::MethodNotAllowed => e
+ assert_equal [:get, :put, :delete], e.allowed_methods
+ end
+
ensure
Object.send(:remove_const, :PeopleController)
end

0 comments on commit dcaa074

Please sign in to comment.