[#1419] Better status for when no param supplied. Added test.

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.

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:

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,
+        )