From b77b48f2f2a3e6ca3e51619279494e3a02392b44 Mon Sep 17 00:00:00 2001 From: Mark McClain Date: Sat, 19 Feb 2011 18:28:48 -0500 Subject: [PATCH] adding opt support for requiring slashes on index --- pecan/core.py | 18 +++-- pecan/default_config.py | 1 + pecan/routing.py | 12 ++-- pecan/templates/project/+package+/app.py_tmpl | 3 +- tests/test_base.py | 68 +++++++++++++++++-- tests/test_hooks.py | 6 +- tests/test_rest.py | 16 +++-- tests/test_secure.py | 6 +- 8 files changed, 101 insertions(+), 29 deletions(-) diff --git a/pecan/core.py b/pecan/core.py index 77d7a93..b44077e 100644 --- a/pecan/core.py +++ b/pecan/core.py @@ -1,6 +1,5 @@ -from configuration import _runtime_conf from templating import RendererFactory -from routing import lookup_controller +from routing import lookup_controller, NonCanonicalPath from util import _cfg from webob import Request, Response, exc @@ -112,14 +111,16 @@ class Pecan(object): template_path = 'templates', hooks = [], custom_renderers = {}, - extra_template_vars = {} + extra_template_vars = {}, + force_canonical = True ): - + self.root = root self.renderers = RendererFactory(custom_renderers, extra_template_vars) self.default_renderer = default_renderer self.hooks = hooks self.template_path = template_path + self.force_canonical = force_canonical def get_content_type(self, format): return { @@ -131,8 +132,13 @@ class Pecan(object): def route(self, node, path): path = path.split('/')[1:] - node, remainder = lookup_controller(node, path) - return node, remainder + try: + node, remainder = lookup_controller(node, path) + return node, remainder + except NonCanonicalPath, e: + if self.force_canonical: + raise exc.HTTPFound(add_slash=True) + return e.controller, e.remainder def determine_hooks(self, controller=None): controller_hooks = [] diff --git a/pecan/default_config.py b/pecan/default_config.py index 4f05458..bff8a2c 100644 --- a/pecan/default_config.py +++ b/pecan/default_config.py @@ -11,6 +11,7 @@ app = { 'static_root' : 'public', 'template_path' : '', 'debug' : False, + 'force_canonical' : True, 'errors' : { '__force_dict__' : True } diff --git a/pecan/routing.py b/pecan/routing.py index fa7a163..8f22bc9 100644 --- a/pecan/routing.py +++ b/pecan/routing.py @@ -6,6 +6,11 @@ from util import iscontroller __all__ = ['lookup_controller', 'find_object'] +class NonCanonicalPath(Exception): + def __init__(self, controller, remainder): + self.controller = controller + self.remainder = remainder + def lookup_controller(obj, url_path): remainder = url_path notfound_handlers = [] @@ -52,11 +57,10 @@ def find_object(obj, remainder, notfound_handlers): index = getattr(obj, 'index', None) if iscontroller(index): return index, remainder[1:] elif not remainder: + # the URL has hit an index method without a trailing slash index = getattr(obj, 'index', None) - if iscontroller(index): - return index, remainder[1:] # TODO: why did I have to do this instead? - #raise exc.HTTPFound(add_slash=True) - + if iscontroller(index): + raise NonCanonicalPath(index, remainder[1:]) default = getattr(obj, '_default', None) if iscontroller(default): notfound_handlers.append(('_default', default, remainder)) diff --git a/pecan/templates/project/+package+/app.py_tmpl b/pecan/templates/project/+package+/app.py_tmpl index c367313..8d0b713 100644 --- a/pecan/templates/project/+package+/app.py_tmpl +++ b/pecan/templates/project/+package+/app.py_tmpl @@ -9,5 +9,6 @@ def setup_app(config): config.app.root, static_root = config.app.static_root, template_path = config.app.template_path, - debug = config.app.debug + debug = config.app.debug, + force_canonical = config.app.force_canonical ) diff --git a/tests/test_base.py b/tests/test_base.py index 765be47..b2215be 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -34,7 +34,7 @@ class TestBase(TestCase): class SubSubController(object): @expose() def index(self): - return '/sub/sub' + return '/sub/sub/' @expose() def deeper(self): @@ -43,7 +43,7 @@ class TestBase(TestCase): class SubController(object): @expose() def index(self): - return '/sub' + return '/sub/' @expose() def deeper(self): @@ -63,7 +63,7 @@ class TestBase(TestCase): sub = SubController() app = TestApp(Pecan(RootController())) - for path in ('/', '/deeper', '/sub', '/sub/deeper', '/sub/sub', '/sub/sub/deeper'): + for path in ('/', '/deeper', '/sub/', '/sub/deeper', '/sub/sub/', '/sub/sub/deeper'): r = app.get(path) assert r.status_int == 200 assert r.body == path @@ -95,7 +95,7 @@ class TestBase(TestCase): assert r.status_int == 200 assert r.body == '/' - r = app.get('/100') + r = app.get('/100/') assert r.status_int == 200 assert r.body == '/100' @@ -618,8 +618,6 @@ class TestEngines(object): if error_msg: break assert error_msg is not None - - def test_json(self): from json import loads @@ -649,3 +647,61 @@ class TestEngines(object): assert r.status_int == 200 assert 'Override' in r.body assert r.content_type == 'text/plain' + + def test_canonical_index(self): + class ArgSubController(object): + @expose() + def index(self, arg): + return arg + class SubController(object): + @expose() + def index(self): + return 'subindex' + class RootController(object): + @expose() + def index(self): + return 'index' + + sub = SubController() + arg = ArgSubController() + + app = TestApp(Pecan(RootController())) + + r = app.get('/') + assert r.status_int == 200 + assert 'index' in r.body + + r = app.get('/index') + assert r.status_int == 200 + assert 'index' in r.body + + r = app.get('/sub/') + assert r.status_int == 200 + assert 'subindex' in r.body + + r = app.get('/sub', expect_errors=True) + assert r.status_int == 302 + + r = app.get('/arg/index/foo') + assert r.status_int == 200 + assert r.body == 'foo' + + app = TestApp(Pecan(RootController(), force_canonical=False)) + r = app.get('/') + assert r.status_int == 200 + assert 'index' in r.body + + r = app.get('/sub') + assert r.status_int == 200 + assert 'subindex' in r.body + + r = app.get('/sub/') + assert r.status_int == 200 + assert 'subindex' in r.body + + + + + + + diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 232d2a3..2261983 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -574,7 +574,7 @@ class TestHooks(object): run_hook = [] - response = app.get('/sub') + response = app.get('/sub/') assert response.status_int == 200 assert response.body == 'Inside here!' @@ -584,7 +584,7 @@ class TestHooks(object): assert run_hook[2] == 'after' run_hook = [] - response = app.get('/sub/sub') + response = app.get('/sub/sub/') assert response.status_int == 200 assert response.body == 'Deep inside here!' @@ -641,7 +641,7 @@ class TestHooks(object): run_hook = [] - response = app.get('/sub') + response = app.get('/sub/') assert response.status_int == 200 assert response.body == 'Inside here!' diff --git a/tests/test_rest.py b/tests/test_rest.py index 56b3e3b..bdfe0d7 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -212,12 +212,16 @@ class TestRestController(object): assert r.status_int == 405 # test the "others" custom action - r = app.request('/things/others', method='MISC') + r = app.request('/things/others/', method='MISC') assert r.status_int == 200 assert r.body == 'OTHERS' + + # test the "others" custom action missing trailing slash + r = app.request('/things/others', method='MISC', status=302) + assert r.status_int == 302 # test the "others" custom action with the _method parameter - r = app.get('/things/others?_method=MISC') + r = app.get('/things/others/?_method=MISC') assert r.status_int == 200 assert r.body == 'OTHERS' @@ -619,16 +623,16 @@ class TestRestController(object): assert r.status_int == 405 # test custom delete without ID - r = app.delete('/things/others') + r = app.delete('/things/others/') assert r.status_int == 200 assert r.body == 'DELETE' # test custom delete without ID with _method parameter and GET - r = app.get('/things/others?_method=delete', status=405) + r = app.get('/things/others/?_method=delete', status=405) assert r.status_int == 405 # test custom delete without ID with _method parameter and POST - r = app.post('/things/others', {'_method':'delete'}) + r = app.post('/things/others/', {'_method':'delete'}) assert r.status_int == 200 assert r.body == 'DELETE' @@ -674,7 +678,7 @@ class TestRestController(object): assert r.body == 'one, two, three' # test nested get request - r = app.get('/things/one/two/three/others') + r = app.get('/things/one/two/three/others/') assert r.status_int == 200 assert r.body == 'NESTED: one, two, three' diff --git a/tests/test_secure.py b/tests/test_secure.py index 2bb337b..e430ab6 100644 --- a/tests/test_secure.py +++ b/tests/test_secure.py @@ -57,7 +57,7 @@ class TestSecure(object): response = app.get('/locked', expect_errors=True) assert response.status_int == 401 - response = app.get('/secret', expect_errors=True) + response = app.get('/secret/', expect_errors=True) assert response.status_int == 401 response = app.get('/secret/allowed') @@ -116,14 +116,14 @@ class TestSecure(object): response = app.get('/locked', expect_errors=True) assert response.status_int == 401 - response = app.get('/secret', expect_errors=True) + response = app.get('/secret/', expect_errors=True) assert response.status_int == 401 response = app.get('/secret/allowed') assert response.status_int == 200 assert response.body == 'Allowed!' - response = app.get('/secret/authorized') + response = app.get('/secret/authorized/') assert response.status_int == 200 assert response.body == 'Index'