Merge pull request #471 from gratipay/bad-location-protection

bad location protection

aspen/exceptions.py

@@ -62,3 +62,12 @@ class UnknownBodyType(Response):
     """
     def __init__(self, ctype):
         Response.__init__(self, code=415, body="Unknown body Content-Type: %s" % ctype)
+
+
+class BadLocation(Response):
+    """
+    A 500 Response raised if the user tries to redirect with base_url and a
+    relative path.
+    """
+    def __init__(self, msg):
+        Response.__init__(self, code=500, body="Bad redirect location: %s" % msg)

aspen/website.py

@@ -14,6 +14,7 @@
 from aspen.configuration import Configurable
 from aspen.http.response import Response
 from aspen.utils import to_rfc822, utc
+from aspen.exceptions import BadLocation
 
 # 2006-11-17 was the first release of aspen - v0.3
 THE_PAST = to_rfc822(datetime.datetime(2006, 11, 17, tzinfo=utc))
@@ -68,15 +69,20 @@ def redirect(self, location, code=None, permanent=False, base_url=None, response
         permanent is True and 302 (Found) if it is False. If url doesn't start
         with base_url (defaulting to self.base_url), then we prefix it with
         base_url before redirecting. This is a protection against open
-        redirects. If you provide your own response we will set .code and
+        redirects. If you wish to use a relative path or full URL as location,
+        then base_url must be the empty string; if it's not, we raise
+        BadLocation. If you provide your own response we will set .code and
         .headers['Location'] on it.
 
         """
         response = response if response else Response()
         response.code = code if code else (301 if permanent else 302)
         base_url = base_url if base_url is not None else self.base_url
         if not location.startswith(base_url):
-            location = base_url + location
+            newloc = base_url + location
+            if not location.startswith('/'):
+                raise BadLocation(newloc)
+            location = newloc
         response.headers['Location'] = location
         raise response
 

tests/test_website.py

@@ -9,6 +9,7 @@
 from pytest import raises
 from aspen.website import Website
 from aspen.http.response import Response
+from aspen.exceptions import BadLocation
 
 
 simple_error_spt = """
@@ -360,6 +361,28 @@ def test_redirect_can_override_base_url_per_call(website):
     website.base_url = 'foo'
     assert raises(Response, website.redirect, '/', base_url='b').value.headers['Location'] == 'b/'
 
+def test_redirect_declines_to_construct_bad_urls(website):
+    raised = raises(BadLocation, website.redirect, '../foo', base_url='http://www.example.com')
+    assert raised.value.body == 'Bad redirect location: http://www.example.com../foo'
+
+def test_redirect_declines_to_construct_more_bad_urls(website):
+    raised = raises(BadLocation, website.redirect, 'http://www.example.org/foo',
+                                                                 base_url='http://www.example.com')
+    assert raised.value.body == 'Bad redirect location: '\
+                                                 'http://www.example.comhttp://www.example.org/foo'
+
+def test_redirect_will_construct_a_good_absolute_url(website):
+    response = raises(Response, website.redirect, '/foo', base_url='http://www.example.com').value
+    assert response.headers['Location'] == 'http://www.example.com/foo'
+
+def test_redirect_will_allow_a_relative_path(website):
+    response = raises(Response, website.redirect, '../foo', base_url='').value
+    assert response.headers['Location'] == '../foo'
+
+def test_redirect_will_allow_an_absolute_url(website):
+    response = raises(Response, website.redirect, 'http://www.example.org/foo', base_url='').value
+    assert response.headers['Location'] == 'http://www.example.org/foo'
+
 def test_redirect_can_use_given_response(website):
     response = Response(65, 'Greetings, program!', {'Location': 'A Town'})
     response = raises(Response, website.redirect, '/flah', response=response).value