diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rwxr-xr-x | actionpack/lib/action_controller/base.rb | 18 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/rescue.rb | 20 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/routing.rb | 13 | ||||
| -rw-r--r-- | actionpack/test/controller/rescue_test.rb | 26 | ||||
| -rw-r--r-- | actionpack/test/controller/resources_test.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/controller/routing_test.rb | 22 |
7 files changed, 91 insertions, 16 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 0258d02b54..356c88a42a 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -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] diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index dc5a1d18cd..8bff41d38d 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -22,6 +22,24 @@ module ActionController #:nodoc: @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: diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 225acd28f3..3ab10107bf 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -13,12 +13,14 @@ module ActionController #:nodoc: 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 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 00d5050b7e..4a1eda82bf 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -251,6 +251,8 @@ module ActionController # 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 @@ module ActionController 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 diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 885ac0b8c0..100fee8a6e 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -7,6 +7,14 @@ class RescueController < ActionController::Base 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 @@ class RescueTest < Test::Unit::TestCase 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 @@ class RescueTest < Test::Unit::TestCase 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) diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 60d94120af..8149c02a9d 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -188,7 +188,7 @@ class ResourcesTest < Test::Unit::TestCase 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 @@ class ResourcesTest < Test::Unit::TestCase 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 diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 2abccd7402..7efb6b8a6e 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1590,8 +1590,13 @@ class RouteSetTest < Test::Unit::TestCase 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 @@ class RouteSetTest < Test::Unit::TestCase 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 |