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:
parent
26d1d59aec
commit
3c634de566
|
@ -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)
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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"
|
||||
|
|
|
@ -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):
|
||||
|
|
Loading…
Reference in New Issue