Properly raise HTTP 405 (and specify Allow headers) for RestController

Change-Id: Id790efc75c8207eb61d74e9b2242b310ccd62ab1
Depends-On: Ieffa3fddc3c8d3152742455ca46d69bcc7208d69
Closes-bug: #1334690
Closes-bug: #1450109
This commit is contained in:
Ryan Petrello 2015-05-11 09:48:53 -07:00
parent 26d1d59aec
commit 3c634de566
4 changed files with 126 additions and 36 deletions

View File

@ -116,14 +116,33 @@ class RestController(object):
_lookup_result = self._handle_lookup(args, request)
if _lookup_result:
return _lookup_result
except (exc.HTTPClientError, exc.HTTPNotFound):
except (exc.HTTPClientError, exc.HTTPNotFound,
exc.HTTPMethodNotAllowed) as e:
#
# If the matching handler results in a 400 or 404, attempt to
# If the matching handler results in a 400, 404, or 405, attempt to
# handle a _lookup method (if it exists)
#
_lookup_result = self._handle_lookup(args, request)
if _lookup_result:
return _lookup_result
# Build a correct Allow: header
if isinstance(e, exc.HTTPMethodNotAllowed):
def method_iter():
for func in ('get', 'get_one', 'get_all', 'new', 'edit',
'get_delete'):
if self._find_controller(func):
yield 'GET'
break
for method in ('HEAD', 'POST', 'PUT', 'DELETE', 'TRACE',
'PATCH'):
func = method.lower()
if self._find_controller(func):
yield method
e.allow = sorted(method_iter())
raise
# return the result
@ -217,7 +236,7 @@ class RestController(object):
return lookup_controller(sub_controller, remainder[1:],
request)
abort(404)
abort(405)
def _handle_get(self, method, remainder, request=None):
'''
@ -233,7 +252,7 @@ class RestController(object):
if controller:
self._handle_bad_rest_arguments(controller, remainder, request)
return controller, []
abort(404)
abort(405)
method_name = remainder[-1]
# check for new/edit/delete GET requests
@ -258,7 +277,7 @@ class RestController(object):
self._handle_bad_rest_arguments(controller, remainder, request)
return controller, remainder
abort(404)
abort(405)
def _handle_delete(self, method, remainder, request=None):
'''
@ -291,7 +310,7 @@ class RestController(object):
return lookup_controller(sub_controller, remainder[1:],
request)
abort(404)
abort(405)
def _handle_post(self, method, remainder, request=None):
'''
@ -315,7 +334,7 @@ class RestController(object):
if controller:
return controller, remainder
abort(404)
abort(405)
def _handle_put(self, method, remainder, request=None):
return self._handle_post(method, remainder, request)

View File

@ -49,7 +49,10 @@ def lookup_controller(obj, remainder, request=None):
request)
handle_security(obj)
return obj, remainder
except (exc.HTTPNotFound, PecanNotFound):
except (exc.HTTPNotFound, exc.HTTPMethodNotAllowed,
PecanNotFound) as e:
if isinstance(e, PecanNotFound):
e = exc.HTTPNotFound()
while notfound_handlers:
name, obj, remainder = notfound_handlers.pop()
if name == '_default':
@ -67,11 +70,11 @@ def lookup_controller(obj, remainder, request=None):
remainder == [''] and
len(obj._pecan['argspec'].args) > 1
):
raise exc.HTTPNotFound
raise e
obj_, remainder_ = result
return lookup_controller(obj_, remainder_, request)
else:
raise exc.HTTPNotFound
raise e
def handle_lookup_traversal(obj, args):

View File

@ -1002,8 +1002,8 @@ class TestRestController(PecanTestCase):
assert r.status_int == 302
def test_invalid_custom_action(self):
r = self.app_.get('/things?_method=BAD', status=404)
assert r.status_int == 404
r = self.app_.get('/things?_method=BAD', status=405)
assert r.status_int == 405
def test_named_action(self):
# test custom "GET" request "length"

View File

@ -250,8 +250,8 @@ class TestRestController(PecanTestCase):
assert r.body == b_('OTHERS')
# test an invalid custom action
r = app.get('/things?_method=BAD', status=404)
assert r.status_int == 404
r = app.get('/things?_method=BAD', status=405)
assert r.status_int == 405
# test custom "GET" request "count"
r = app.get('/things/count')
@ -299,7 +299,7 @@ class TestRestController(PecanTestCase):
assert r.status_int == 200
assert r.body == b_(dumps(dict(items=ThingsController.data)))
def test_404_with_lookup(self):
def test_405_with_lookup(self):
class LookupController(RestController):
@ -322,10 +322,10 @@ class TestRestController(PecanTestCase):
# create the app
app = TestApp(make_app(RootController()))
# these should 404
# these should 405
for path in ('/things', '/things/'):
r = app.get(path, expect_errors=True)
assert r.status_int == 404
assert r.status_int == 405
r = app.get('/things/foo')
assert r.status_int == 200
@ -813,53 +813,53 @@ class TestRestController(PecanTestCase):
app = TestApp(make_app(RootController()))
# test get_all
r = app.get('/things', status=404)
assert r.status_int == 404
r = app.get('/things', status=405)
assert r.status_int == 405
# test get_one
r = app.get('/things/1', status=404)
assert r.status_int == 404
r = app.get('/things/1', status=405)
assert r.status_int == 405
# test post
r = app.post('/things', {'value': 'one'}, status=404)
assert r.status_int == 404
r = app.post('/things', {'value': 'one'}, status=405)
assert r.status_int == 405
# test edit
r = app.get('/things/1/edit', status=404)
assert r.status_int == 404
r = app.get('/things/1/edit', status=405)
assert r.status_int == 405
# test put
r = app.put('/things/1', {'value': 'ONE'}, status=404)
r = app.put('/things/1', {'value': 'ONE'}, status=405)
# test put with _method parameter and GET
r = app.get('/things/1?_method=put', {'value': 'ONE!'}, status=405)
assert r.status_int == 405
# test put with _method parameter and POST
r = app.post('/things/1?_method=put', {'value': 'ONE!'}, status=404)
assert r.status_int == 404
r = app.post('/things/1?_method=put', {'value': 'ONE!'}, status=405)
assert r.status_int == 405
# test get delete
r = app.get('/things/1/delete', status=404)
assert r.status_int == 404
r = app.get('/things/1/delete', status=405)
assert r.status_int == 405
# test delete
r = app.delete('/things/1', status=404)
assert r.status_int == 404
r = app.delete('/things/1', status=405)
assert r.status_int == 405
# test delete with _method parameter and GET
r = app.get('/things/1?_method=DELETE', status=405)
assert r.status_int == 405
# test delete with _method parameter and POST
r = app.post('/things/1?_method=DELETE', status=404)
assert r.status_int == 404
r = app.post('/things/1?_method=DELETE', status=405)
assert r.status_int == 405
# test "RESET" custom action
with warnings.catch_warnings():
warnings.simplefilter("ignore")
r = app.request('/things', method='RESET', status=404)
assert r.status_int == 404
r = app.request('/things', method='RESET', status=405)
assert r.status_int == 405
def test_nested_rest_with_missing_intermediate_id(self):
@ -1490,6 +1490,74 @@ class TestRestController(PecanTestCase):
assert r.status_int == 200
assert r.body == b_('DELETE_BAR')
def test_method_not_allowed_get(self):
class ThingsController(RestController):
@expose()
def put(self, id_, value):
response.status = 200
@expose()
def delete(self, id_):
response.status = 200
app = TestApp(make_app(ThingsController()))
r = app.get('/', status=405)
assert r.status_int == 405
assert r.headers['Allow'] == 'DELETE, PUT'
def test_method_not_allowed_post(self):
class ThingsController(RestController):
@expose()
def get_one(self):
return dict()
app = TestApp(make_app(ThingsController()))
r = app.post('/', {'foo': 'bar'}, status=405)
assert r.status_int == 405
assert r.headers['Allow'] == 'GET'
def test_method_not_allowed_put(self):
class ThingsController(RestController):
@expose()
def get_one(self):
return dict()
app = TestApp(make_app(ThingsController()))
r = app.put('/123', status=405)
assert r.status_int == 405
assert r.headers['Allow'] == 'GET'
def test_method_not_allowed_delete(self):
class ThingsController(RestController):
@expose()
def get_one(self):
return dict()
app = TestApp(make_app(ThingsController()))
r = app.delete('/123', status=405)
assert r.status_int == 405
assert r.headers['Allow'] == 'GET'
def test_proper_allow_header_multiple_gets(self):
class ThingsController(RestController):
@expose()
def get_all(self):
return dict()
@expose()
def get(self):
return dict()
app = TestApp(make_app(ThingsController()))
r = app.put('/123', status=405)
assert r.status_int == 405
assert r.headers['Allow'] == 'GET'
def test_rest_with_utf8_uri(self):
class FooController(RestController):