Added Request.urlparts property and lots of redirect() tests.

This reverts 5cf06439. The result of Request.url is now quoted properly.
If you really need the URL with an unquoted path, get Request.urlparts,
urllib.unquote() the path and urlparse.urlunsplit() it again.

I do not quite remember why I changed the Request.url behavior in the
first place. An URL with special characters in the path is useless.

4 files changed, 124 insertions, 34 deletions

diff --git a/bottle.py b/bottle.py
index 0d59c3b..d6ccb26 100755
--- a/bottle.py
+++ b/bottle.py
@@ -39,8 +39,8 @@ import warnings
 from Cookie import SimpleCookie
 from tempfile import TemporaryFile
 from traceback import format_exc
-from urllib import urlencode
-from urlparse import urlunsplit, urljoin
+from urllib import urlencode, quote as urlquote, unquote as urlunquote
+from urlparse import urlunsplit, urljoin, SplitResult as UrlSplitResult
 
 try: from collections import MutableMapping as DictMixin
 except ImportError: # pragma: no cover
@@ -811,37 +811,42 @@ class Request(threading.local, DictMixin):
             if 'bottle.' + key in self.environ:
                 del self.environ['bottle.' + key]
 
-    @property
-    def query_string(self):
-        """ The part of the URL following the '?'. """
-        return self.environ.get('QUERY_STRING', '')
-
-    @property
-    def fullpath(self):
-        """ Request path including SCRIPT_NAME (if present). """
+    @DictProperty('environ', 'bottle.urlparts', read_only=True)
+    def urlparts(self):
+        ''' Return a :cls:`urlparse.SplitResult` tuple that can be used
+            to reconstruct the full URL as requested by the client.
+            The tuple contains: (scheme, host, path, query_string, fragment).
+            The fragment is always empty because it is not visible to the server.
+        '''
+        env = self.environ
+        host = env.get('HTTP_X_FORWARDED_HOST') or env.get('HTTP_HOST', '')
+        http = env.get('wsgi.url_scheme', 'http')
+        port = env.get('SERVER_PORT')
+        if ':' in host: # Overrule SERVER_POST (proxy support)
+            host, port = host.rsplit(':', 1)
+        if not host or host == '127.0.0.1':
+            host = env.get('SERVER_NAME', host)
+        if port and http+port not in ('http80', 'https443'):
+            host += ':' + port
         spath = self.environ.get('SCRIPT_NAME','').rstrip('/') + '/'
         rpath = self.path.lstrip('/')
-        return urljoin(spath, rpath)
+        path = urlquote(urljoin(spath, rpath))
+        return UrlSplitResult(http, host, path, env.get('QUERY_STRING'), '')
 
     @property
     def url(self):
-        """ Full URL as requested by the client (computed).
+        """ Full URL as requested by the client. """
+        return self.urlparts.geturl()
 
-            This value is constructed out of different environment variables
-            and includes scheme, host, port, scriptname, path and query string.
+    @property
+    def fullpath(self):
+        """ Request path including SCRIPT_NAME (if present). """
+        return urlunquote(self.urlparts[2])
 
-            Special characters are NOT escaped.
-        """
-        scheme = self.environ.get('wsgi.url_scheme', 'http')
-        host   = self.environ.get('HTTP_X_FORWARDED_HOST')
-        host   = host or self.environ.get('HTTP_HOST', None)
-        if not host:
-            host = self.environ.get('SERVER_NAME')
-            port = self.environ.get('SERVER_PORT', '80')
-            if (scheme, port) not in (('https','443'), ('http','80')):
-                host += ':' + port
-        parts = (scheme, host, self.fullpath, self.query_string, '')
-        return urlunsplit(parts)
+    @property
+    def query_string(self):
+        """ The part of the URL following the '?'. """
+        return self.environ.get('QUERY_STRING', '')
 
     @property
     def content_length(self):
@@ -1356,9 +1361,8 @@ def abort(code=500, text='Unknown Error: Application stopped.'):
 
 
 def redirect(url, code=303):
-    """ Aborts execution and causes a 303 redirect """
-    scriptname = request.environ.get('SCRIPT_NAME', '').rstrip('/') + '/'
-    location = urljoin(request.url, urljoin(scriptname, url))
+    """ Aborts execution and causes a 303 redirect. """
+    location = urljoin(request.url, url)
     raise HTTPResponse("", status=code, header=dict(Location=location))
 
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 681f28e..8868d12 100755
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -17,6 +17,7 @@ Release 0.9
 * Support for SimpleTAL templates.
 * Better runtime exception handling for mako templates in debug mode.
 * Lots of documentation, fixes and small improvements.
+* A new :data:`Request.urlparts` property.
 
 .. rubric:: Performance improvements
 
diff --git a/test/test_environ.py b/test/test_environ.py
index f611a12..3612027 100755
--- a/test/test_environ.py
+++ b/test/test_environ.py
@@ -49,11 +49,11 @@ class TestRequest(unittest.TestCase):
         request.bind({'SERVER_NAME':'example.com', 'SERVER_PORT':'81'})
         self.assertEqual('http://example.com:81/', request.url)
         request.bind({'wsgi.url_scheme':'https', 'SERVER_NAME':'example.com'})
-        self.assertEqual('https://example.com:80/', request.url)
+        self.assertEqual('https://example.com/', request.url)
         request.bind({'HTTP_HOST':'example.com', 'PATH_INFO':'/path', 'QUERY_STRING':'1=b&c=d', 'SCRIPT_NAME':'/sp'})
         self.assertEqual('http://example.com/sp/path?1=b&c=d', request.url)
-        request.bind({'HTTP_HOST':'example.com', 'PATH_INFO':'/pa th', 'QUERY_STRING':'1=b b', 'SCRIPT_NAME':'/s p'})
-        self.assertEqual('http://example.com/s p/pa th?1=b b', request.url)
+        request.bind({'HTTP_HOST':'example.com', 'PATH_INFO':'/pa th', 'SCRIPT_NAME':'/s p'})
+        self.assertEqual('http://example.com/s%20p/pa%20th', request.url)
 
     def test_dict_access(self):
         """ Environ: request objects are environment dicts """
@@ -242,7 +242,92 @@ class TestResponse(unittest.TestCase):
                    if name.title() == 'Set-Cookie']
         self.assertTrue('name=;' in cookies[0])
 
+class TestRedirect(unittest.TestCase):
+   
+    def assertRedirect(self, target, result, query=None, status=303, **args):
+        env = {}
+        for key in args:
+            if key.startswith('wsgi'):
+                args[key.replace('_', '.', 1)] = args[key]
+                del args[key]
+        env.update(args)
+        wsgiref.util.setup_testing_defaults(env)
+        request.bind(env)
+        try:
+            bottle.redirect(target, **(query or {}))
+        except bottle.HTTPResponse, r:
+            self.assertEqual(status, r.status)
+            self.assertTrue(r.headers)
+            self.assertEqual(result, r.headers['Location'])
+                
+    def test_root(self):
+        self.assertRedirect('/', 'http://127.0.0.1/')
+        self.assertRedirect('/test.html', 'http://127.0.0.1/test.html')
+        self.assertRedirect('/test.html', 'http://127.0.0.1/test.html',
+                            PATH_INFO='/some/sub/path/')
+        self.assertRedirect('/test.html', 'http://127.0.0.1/test.html',
+                            PATH_INFO='/some/sub/file.html')
+        self.assertRedirect('/test.html', 'http://127.0.0.1/test.html',
+                            SCRIPT_NAME='/some/sub/path/')
+        self.assertRedirect('/foo/test.html', 'http://127.0.0.1/foo/test.html')
+        self.assertRedirect('/foo/test.html', 'http://127.0.0.1/foo/test.html',
+                            PATH_INFO='/some/sub/file.html')
 
+    def test_relative(self):
+        self.assertRedirect('./', 'http://127.0.0.1/')
+        self.assertRedirect('./test.html', 'http://127.0.0.1/test.html')
+        self.assertRedirect('./test.html', 'http://127.0.0.1/foo/test.html',
+                            PATH_INFO='/foo/')
+        self.assertRedirect('./test.html', 'http://127.0.0.1/foo/test.html',
+                            PATH_INFO='/foo/bar.html')
+        self.assertRedirect('./test.html', 'http://127.0.0.1/foo/test.html',
+                            SCRIPT_NAME='/foo/')
+        self.assertRedirect('./test.html', 'http://127.0.0.1/foo/bar/test.html',
+                            SCRIPT_NAME='/foo/', PATH_INFO='/bar/baz.html')
+        self.assertRedirect('./foo/test.html', 'http://127.0.0.1/foo/test.html')
+        self.assertRedirect('./foo/test.html', 'http://127.0.0.1/bar/foo/test.html',
+                            PATH_INFO='/bar/file.html')
+        self.assertRedirect('../test.html', 'http://127.0.0.1/test.html',
+                            PATH_INFO='/foo/')
+        self.assertRedirect('../test.html', 'http://127.0.0.1/foo/test.html',
+                            PATH_INFO='/foo/bar/')
+        self.assertRedirect('../test.html', 'http://127.0.0.1/test.html',
+                            PATH_INFO='/foo/bar.html')
+        self.assertRedirect('../test.html', 'http://127.0.0.1/test.html',
+                            SCRIPT_NAME='/foo/')
+        self.assertRedirect('../test.html', 'http://127.0.0.1/foo/test.html',
+                            SCRIPT_NAME='/foo/', PATH_INFO='/bar/baz.html')
+        self.assertRedirect('../baz/../test.html', 'http://127.0.0.1/foo/test.html',
+                            PATH_INFO='/foo/bar/')
+      
+    def test_sheme(self):
+        self.assertRedirect('./test.html', 'https://127.0.0.1/test.html',
+                            wsgi_url_scheme='https')
+        self.assertRedirect('./test.html', 'https://127.0.0.1:80/test.html',
+                            wsgi_url_scheme='https', SERVER_PORT='80')
+                            
+    def test_host(self):
+        self.assertRedirect('./test.html', 'http://example.com/test.html',
+                            HTTP_HOST='example.com')
+        self.assertRedirect('./test.html', 'http://example.com/test.html',
+                            HTTP_X_FORWARDED_HOST='example.com')
+        self.assertRedirect('./test.html', 'http://example.com/test.html',
+                            SERVER_NAME='example.com')
+        self.assertRedirect('./test.html', 'http://example.com/test.html',
+                            HTTP_HOST='example.com:80')
+        self.assertRedirect('./test.html', 'http://example.com:81/test.html',
+                            HTTP_HOST='example.com:81')
+        self.assertRedirect('./test.html', 'http://127.0.0.1:81/test.html',
+                            SERVER_PORT='81')
+        self.assertRedirect('./test.html', 'http://example.com:81/test.html',
+                            HTTP_HOST='example.com:81', SERVER_PORT='82')
+
+    def test_specialchars(self):
+        ''' The target URL is not quoted automatically. '''
+        self.assertRedirect('./te st.html',
+                            'http://example.com/a%20a/b%20b/te st.html',
+                            HTTP_HOST='example.com', SCRIPT_NAME='/a a/', PATH_INFO='/b b/')
+                            
 class TestMultipart(unittest.TestCase):
     def test_multipart(self):
         """ Environ: POST (multipart files and multible values per key) """
diff --git a/test/test_wsgi.py b/test/test_wsgi.py
index 5d2283b..8c3edce 100755
--- a/test/test_wsgi.py
+++ b/test/test_wsgi.py
@@ -81,10 +81,10 @@ class TestWsgi(ServerTestBase):
         """ WSGI: Exceptions within handler code (HTTP 500) """
         @bottle.route('/my/:string')
         def test(string): return string
-        self.assertBody(tob(u'urf8-öäü'), tob(u'/my/urf8-öäü'))
+        self.assertBody(tob(u'urf8-öäü'), '/my/urf8-öäü')
     
     def test_utf8_404(self):
-        self.assertStatus(404, tob(u'/not-found/urf8-öäü'))
+        self.assertStatus(404, '/not-found/urf8-öäü')
         
     def test_503(self):
         """ WSGI: Server stopped (HTTP 503) """