diff --git a/ckan/controllers/util.py b/ckan/controllers/util.py index b731d05a516..840c6833a04 100644 --- a/ckan/controllers/util.py +++ b/ckan/controllers/util.py @@ -12,11 +12,13 @@ class UtilController(base.BaseController): def redirect(self): ''' redirect to the url parameter. ''' url = base.request.params.get('url') + if not url: + base.abort(400, _('Missing Value') + ': url') if h.url_is_local(url): return base.redirect(url) else: - base.abort(403, _('Redirecting to external site at %s not allowed.') % url) + base.abort(403, _('Redirecting to external site is not allowed.')) def primer(self): ''' Render all html components out onto a single page. diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index f5e44bca0f0..cabee038ccc 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -230,7 +230,7 @@ def _add_i18n_to_url(url_to_amend, **kw): def url_is_local(url): '''Returns True if url is local''' - if not url or (len(url) >= 2 and url.startswith('//')): + if not url or url.startswith('//'): return False parsed = urlparse.urlparse(url) if parsed.scheme: diff --git a/ckan/new_tests/controllers/test_util.py b/ckan/new_tests/controllers/test_util.py new file mode 100644 index 00000000000..4e35e9754fc --- /dev/null +++ b/ckan/new_tests/controllers/test_util.py @@ -0,0 +1,48 @@ +from nose.tools import assert_equal +from pylons.test import pylonsapp +import paste.fixture + +import routes.url_for as url_for + + +# This is stolen from the old tests and should probably go in __init__.py +# if it is what we want. +class WsgiAppCase(object): + wsgiapp = pylonsapp + assert wsgiapp, 'You need to run nose with --with-pylons' + # Either that, or this file got imported somehow before the tests started + # running, meaning the pylonsapp wasn't setup yet (which is done in + # pylons.test.py:begin()) + app = paste.fixture.TestApp(wsgiapp) + + +class TestUtil(WsgiAppCase): + def test_redirect_ok(self): + response = self.app.get( + url=url_for(controller='util', action='redirect'), + params={'url': '/dataset'}, + status=302, + ) + assert_equal(response.header_dict.get('Location'), + 'http://localhost/dataset') + + def test_redirect_external(self): + response = self.app.get( + url=url_for(controller='util', action='redirect'), + params={'url': 'http://nastysite.com'}, + status=403, + ) + + def test_redirect_no_params(self): + response = self.app.get( + url=url_for(controller='util', action='redirect'), + params={}, + status=400, + ) + + def test_redirect_no_params_2(self): + response = self.app.get( + url=url_for(controller='util', action='redirect'), + params={'url': ''}, + status=400, + )