From edeb59e2002640365d05d24b2fcaae47f614f2ad Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 23:44:24 +0100 Subject: [PATCH] [#4801] Remove catch-all template controller Pylons used to have a catch-all route defined that tried to look for the path in the provided URL in the templates folder. With changed in the multiple static folders introduced in f9775a1 this route is never reached, as we now set the catch-all route to look for the path in the public folder(s) (which makes more sense to be honest). Modified the middleware tests to reflect this. --- ckan/config/routing.py | 1 - ckan/controllers/template.py | 45 ------------------------- ckan/tests/config/test_middleware.py | 42 ++++++++--------------- ckan/tests/controllers/test_template.py | 16 --------- 4 files changed, 14 insertions(+), 90 deletions(-) delete mode 100644 ckan/controllers/template.py delete mode 100644 ckan/tests/controllers/test_template.py diff --git a/ckan/config/routing.py b/ckan/config/routing.py index f08b4a440ae..f4632a2643a 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -153,6 +153,5 @@ def make_map(): map.redirect('/favicon.ico', config.get('ckan.favicon')) map.redirect('/*(url)/', '/{url}', _redirect_code='301 Moved Permanently') - map.connect('/*url', controller='template', action='view', ckan_core=True) return map diff --git a/ckan/controllers/template.py b/ckan/controllers/template.py deleted file mode 100644 index d51f891d446..00000000000 --- a/ckan/controllers/template.py +++ /dev/null @@ -1,45 +0,0 @@ -# encoding: utf-8 - -import ckan.lib.base as base -import ckan.lib.render -from ckan.common import response - - -class TemplateController(base.BaseController): - - def view(self, url): - u"""By default, the final controller tried to fulfill the request - when no other routes match. It may be used to display a template - when all else fails, e.g.:: - - def view(self, url): - return render('/%s' % url) - - Or if you're using Mako and want to explicitly send a 404 (Not - Found) response code when the requested template doesn't exist:: - - import mako.exceptions - - def view(self, url): - try: - return render('/%s' % url) - except mako.exceptions.TopLevelLookupException: - abort(404) - - By default this controller aborts the request with a 404 (Not - Found) - """ - if url.endswith(u'.txt'): - response.headers[b'Content-Type'] = b'text/plain; charset=utf-8' - # Default content-type is text/html - try: - return base.render(url) - except ckan.lib.render.TemplateNotFound: - if url.endswith(u'.html'): - base.abort(404) - url += u'.html' - response.headers[b'Content-Type'] = b'text/html; charset=utf-8' - try: - return base.render(url) - except ckan.lib.render.TemplateNotFound: - base.abort(404) diff --git a/ckan/tests/config/test_middleware.py b/ckan/tests/config/test_middleware.py index 1f2c7effdf4..28cbf97b206 100644 --- a/ckan/tests/config/test_middleware.py +++ b/ckan/tests/config/test_middleware.py @@ -137,24 +137,6 @@ def test_view(): return app -def test_ask_around_pylons_core_route_get(patched_app): - environ = {u"PATH_INFO": u"/tag", u"REQUEST_METHOD": u"GET"} - wsgiref.util.setup_testing_defaults(environ) - - answers = patched_app.app.ask_around(environ) - - assert answers == [(False, u"flask_app"), (True, u"pylons_app", u"core")] - - -def test_ask_around_pylons_core_route_post(patched_app): - environ = {u"PATH_INFO": u"/tag", u"REQUEST_METHOD": u"POST"} - wsgiref.util.setup_testing_defaults(environ) - - answers = patched_app.app.ask_around(environ) - - assert answers == [(False, u"flask_app"), (True, u"pylons_app", u"core")] - - def test_flask_core_route_is_served_by_flask(patched_app): res = patched_app.get(u"/") @@ -184,8 +166,11 @@ def test_ask_around_pylons_extension_route_get_before_map( answers = patched_app.app.ask_around(environ) + # Even though this route is defined in Pylons, there is catch all route + # in Flask for all requests to serve static files with the same name, + # so we get two positive answers assert answers == [ - (False, u"flask_app"), + (True, u"flask_app", u"core"), (True, u"pylons_app", u"extension"), ] @@ -214,11 +199,12 @@ def test_ask_around_pylons_extension_route_post_using_get( answers = patched_app.app.ask_around(environ) - # We are going to get an answer from Pylons, but just because it will - # match the catch-all template route, hence the `core` origin. + # Even though this route is defined in Pylons, there is catch all route + # in Flask for all requests to serve static files with the same name, + # so we get two positive answers assert answers == [ - (False, u"flask_app"), - (True, u"pylons_app", u"core"), + (True, u"flask_app", u"core"), + (False, u"pylons_app"), ] def test_ask_around_pylons_extension_route_get_after_map( @@ -232,8 +218,11 @@ def test_ask_around_pylons_extension_route_get_after_map( answers = patched_app.app.ask_around(environ) + # Even though this route is defined in Pylons, there is catch all route + # in Flask for all requests to serve static files with the same name, + # so we get two positive answers assert answers == [ - (False, u"flask_app"), + (True, u"flask_app", u"core"), (True, u"pylons_app", u"extension"), ] @@ -453,10 +442,7 @@ def test_ask_around_flask_core_route_post(app): answers = ckan_app.ask_around(environ) - # Even though this route is defined in Flask, there is catch all route - # in Pylons for all requests to point arbitrary urls to templates with - # the same name, so we get two positive answers assert answers == [ (True, u"flask_app", u"core"), - (True, u"pylons_app", u"core"), + (False, u"pylons_app"), ] diff --git a/ckan/tests/controllers/test_template.py b/ckan/tests/controllers/test_template.py deleted file mode 100644 index eb17c607b14..00000000000 --- a/ckan/tests/controllers/test_template.py +++ /dev/null @@ -1,16 +0,0 @@ -# encoding: utf-8 - -import pytest - - -@pytest.mark.parametrize( - u"url,expected", - [ - (u"/robots.txt", u"text/plain; charset=utf-8"), - (u"/page", u"text/html; charset=utf-8"), - (u"/page.html", u"text/html; charset=utf-8"), - ], -) -def test_content_type(url, expected, app): - response = app.get(url, status=200) - assert response.headers.get(u"Content-Type") == expected