Repository: tornadoweb/tornado

Issue:
redirect requests starting with '//' to '/' leading to wrong place
Tornado uses `urljoin` to join `self.request.uri` and the destination but when `self.request.uri` starts with '//' it generates locations still start with '//' because this behaviour of `urljoin`:

```
>>> from urllib.parse import urljoin                                                                                                                   
>>> urljoin('//abc', '/abc')
'//abc/abc'
```

I suggest using `self.request.full_url()` instead. Also, the HTTP specification says that the Location header should include the host part.

PS: `self.request.full_url()` doesn't work for proxy requests that have full urls in their request line.

Discussion:

Patch:
diff --git a/tornado/httpserver.py b/tornado/httpserver.py
index 226f966a..13a6e92f 100644
--- a/tornado/httpserver.py
+++ b/tornado/httpserver.py
@@ -37,11 +37,9 @@ from tornado import httputil
 from tornado import iostream
 from tornado import netutil
 from tornado.tcpserver import TCPServer
-from tornado.util import Configurable
 
 
-class HTTPServer(TCPServer, Configurable,
-                 httputil.HTTPServerConnectionDelegate):
+class HTTPServer(TCPServer, httputil.HTTPServerConnectionDelegate):
     r"""A non-blocking, single-threaded HTTP server.
 
     A server is defined by a subclass of `.HTTPServerConnectionDelegate`,
@@ -122,20 +120,12 @@ class HTTPServer(TCPServer, Configurable,
        two arguments ``(server_conn, request_conn)`` (in accordance with the
        documentation) instead of one ``(request_conn)``.
     """
-    def __init__(self, *args, **kwargs):
-        # Ignore args to __init__; real initialization belongs in
-        # initialize since we're Configurable. (there's something
-        # weird in initialization order between this class,
-        # Configurable, and TCPServer so we can't leave __init__ out
-        # completely)
-        pass
-
-    def initialize(self, request_callback, no_keep_alive=False, io_loop=None,
-                   xheaders=False, ssl_options=None, protocol=None,
-                   decompress_request=False,
-                   chunk_size=None, max_header_size=None,
-                   idle_connection_timeout=None, body_timeout=None,
-                   max_body_size=None, max_buffer_size=None):
+    def __init__(self, request_callback, no_keep_alive=False, io_loop=None,
+                 xheaders=False, ssl_options=None, protocol=None,
+                 decompress_request=False,
+                 chunk_size=None, max_header_size=None,
+                 idle_connection_timeout=None, body_timeout=None,
+                 max_body_size=None, max_buffer_size=None):
         self.request_callback = request_callback
         self.no_keep_alive = no_keep_alive
         self.xheaders = xheaders
@@ -152,14 +142,6 @@ class HTTPServer(TCPServer, Configurable,
                            read_chunk_size=chunk_size)
         self._connections = set()
 
-    @classmethod
-    def configurable_base(cls):
-        return HTTPServer
-
-    @classmethod
-    def configurable_default(cls):
-        return HTTPServer
-
     @gen.coroutine
     def close_all_connections(self):
         while self._connections:
diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py
index 6321a81d..f3cb1b86 100644
--- a/tornado/simple_httpclient.py
+++ b/tornado/simple_httpclient.py
@@ -135,14 +135,10 @@ class SimpleAsyncHTTPClient(AsyncHTTPClient):
                 release_callback = functools.partial(self._release_fetch, key)
                 self._handle_request(request, release_callback, callback)
 
-    def _connection_class(self):
-        return _HTTPConnection
-
     def _handle_request(self, request, release_callback, final_callback):
-        self._connection_class()(
-            self.io_loop, self, request, release_callback,
-            final_callback, self.max_buffer_size, self.tcp_client,
-            self.max_header_size)
+        _HTTPConnection(self.io_loop, self, request, release_callback,
+                        final_callback, self.max_buffer_size, self.tcp_client,
+                        self.max_header_size)
 
     def _release_fetch(self, key):
         del self.active[key]
@@ -352,7 +348,14 @@ class _HTTPConnection(httputil.HTTPMessageDelegate):
             self.request.headers["Accept-Encoding"] = "gzip"
         req_path = ((self.parsed.path or '/') +
                     (('?' + self.parsed.query) if self.parsed.query else ''))
-        self.connection = self._create_connection(stream)
+        self.stream.set_nodelay(True)
+        self.connection = HTTP1Connection(
+            self.stream, True,
+            HTTP1ConnectionParameters(
+                no_keep_alive=True,
+                max_header_size=self.max_header_size,
+                decompress=self.request.decompress_response),
+            self._sockaddr)
         start_line = httputil.RequestStartLine(self.request.method,
                                                req_path, '')
         self.connection.write_headers(start_line, self.request.headers)
@@ -361,20 +364,10 @@ class _HTTPConnection(httputil.HTTPMessageDelegate):
         else:
             self._write_body(True)
 
-    def _create_connection(self, stream):
-        stream.set_nodelay(True)
-        connection = HTTP1Connection(
-            stream, True,
-            HTTP1ConnectionParameters(
-                no_keep_alive=True,
-                max_header_size=self.max_header_size,
-                decompress=self.request.decompress_response),
-            self._sockaddr)
-        return connection
-
     def _write_body(self, start_read):
         if self.request.body is not None:
             self.connection.write(self.request.body)
+            self.connection.finish()
         elif self.request.body_producer is not None:
             fut = self.request.body_producer(self.connection.write)
             if is_future(fut):
@@ -385,7 +378,7 @@ class _HTTPConnection(httputil.HTTPMessageDelegate):
                         self._read_response()
                 self.io_loop.add_future(fut, on_body_written)
                 return
-        self.connection.finish()
+            self.connection.finish()
         if start_read:
             self._read_response()
 
diff --git a/tornado/util.py b/tornado/util.py
index 606ced19..d943ce2b 100644
--- a/tornado/util.py
+++ b/tornado/util.py
@@ -198,21 +198,21 @@ class Configurable(object):
     __impl_class = None
     __impl_kwargs = None
 
-    def __new__(cls, *args, **kwargs):
+    def __new__(cls, **kwargs):
         base = cls.configurable_base()
-        init_kwargs = {}
+        args = {}
         if cls is base:
             impl = cls.configured_class()
             if base.__impl_kwargs:
-                init_kwargs.update(base.__impl_kwargs)
+                args.update(base.__impl_kwargs)
         else:
             impl = cls
-        init_kwargs.update(kwargs)
+        args.update(kwargs)
         instance = super(Configurable, cls).__new__(impl)
         # initialize vs __init__ chosen for compatibility with AsyncHTTPClient
         # singleton magic.  If we get rid of that we can switch to __init__
         # here too.
-        instance.initialize(*args, **init_kwargs)
+        instance.initialize(**args)
         return instance
 
     @classmethod
@@ -233,9 +233,6 @@ class Configurable(object):
         """Initialize a `Configurable` subclass instance.
 
         Configurable classes should use `initialize` instead of ``__init__``.
-
-        .. versionchanged:: 4.2
-           Now accepts positional arguments in addition to keyword arguments.
         """
 
     @classmethod
diff --git a/tornado/web.py b/tornado/web.py
index 62f3779d..155da550 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -650,8 +650,7 @@ class RequestHandler(object):
         else:
             assert isinstance(status, int) and 300 <= status <= 399
         self.set_status(status)
-        self.set_header("Location", urlparse.urljoin(utf8(self.request.uri),
-                                                     utf8(url)))
+        self.set_header("Location", utf8(url))
         self.finish()
 
     def write(self, chunk):


Test patch:
diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py
index c1ba831c..62ef6ca3 100644
--- a/tornado/test/httpserver_test.py
+++ b/tornado/test/httpserver_test.py
@@ -162,22 +162,19 @@ class BadSSLOptionsTest(unittest.TestCase):
         application = Application()
         module_dir = os.path.dirname(__file__)
         existing_certificate = os.path.join(module_dir, 'test.crt')
-        existing_key = os.path.join(module_dir, 'test.key')
 
-        self.assertRaises((ValueError, IOError),
-                          HTTPServer, application, ssl_options={
-                              "certfile": "/__mising__.crt",
+        self.assertRaises(ValueError, HTTPServer, application, ssl_options={
+                          "certfile": "/__mising__.crt",
                           })
-        self.assertRaises((ValueError, IOError),
-                          HTTPServer, application, ssl_options={
-                              "certfile": existing_certificate,
-                              "keyfile": "/__missing__.key"
+        self.assertRaises(ValueError, HTTPServer, application, ssl_options={
+                          "certfile": existing_certificate,
+                          "keyfile": "/__missing__.key"
                           })
 
         # This actually works because both files exist
         HTTPServer(application, ssl_options={
                    "certfile": existing_certificate,
-                   "keyfile": existing_key,
+                   "keyfile": existing_certificate
                    })
 
 
diff --git a/tornado/test/runtests.py b/tornado/test/runtests.py
index cb9969d3..20133d4e 100644
--- a/tornado/test/runtests.py
+++ b/tornado/test/runtests.py
@@ -8,7 +8,6 @@ import operator
 import textwrap
 import sys
 from tornado.httpclient import AsyncHTTPClient
-from tornado.httpserver import HTTPServer
 from tornado.ioloop import IOLoop
 from tornado.netutil import Resolver
 from tornado.options import define, options, add_parse_callback
@@ -124,8 +123,6 @@ def main():
     define('httpclient', type=str, default=None,
            callback=lambda s: AsyncHTTPClient.configure(
                s, defaults=dict(allow_ipv6=False)))
-    define('httpserver', type=str, default=None,
-           callback=HTTPServer.configure)
     define('ioloop', type=str, default=None)
     define('ioloop_time_monotonic', default=False)
     define('resolver', type=str, default=None,
diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py
index 0936c89a..a0fbae43 100644
--- a/tornado/test/util_test.py
+++ b/tornado/test/util_test.py
@@ -46,15 +46,13 @@ class TestConfigurable(Configurable):
 
 
 class TestConfig1(TestConfigurable):
-    def initialize(self, pos_arg=None, a=None):
+    def initialize(self, a=None):
         self.a = a
-        self.pos_arg = pos_arg
 
 
 class TestConfig2(TestConfigurable):
-    def initialize(self, pos_arg=None, b=None):
+    def initialize(self, b=None):
         self.b = b
-        self.pos_arg = pos_arg
 
 
 class ConfigurableTest(unittest.TestCase):
@@ -104,10 +102,9 @@ class ConfigurableTest(unittest.TestCase):
         self.assertIsInstance(obj, TestConfig1)
         self.assertEqual(obj.a, 3)
 
-        obj = TestConfigurable(42, a=4)
+        obj = TestConfigurable(a=4)
         self.assertIsInstance(obj, TestConfig1)
         self.assertEqual(obj.a, 4)
-        self.assertEqual(obj.pos_arg, 42)
 
         self.checkSubclasses()
         # args bound in configure don't apply when using the subclass directly
@@ -120,10 +117,9 @@ class ConfigurableTest(unittest.TestCase):
         self.assertIsInstance(obj, TestConfig2)
         self.assertEqual(obj.b, 5)
 
-        obj = TestConfigurable(42, b=6)
+        obj = TestConfigurable(b=6)
         self.assertIsInstance(obj, TestConfig2)
         self.assertEqual(obj.b, 6)
-        self.assertEqual(obj.pos_arg, 42)
 
         self.checkSubclasses()
         # args bound in configure don't apply when using the subclass directly
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index f3c8505a..a52f1667 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -597,6 +597,7 @@ class WSGISafeWebTest(WebTestCase):
             url("/redirect", RedirectHandler),
             url("/web_redirect_permanent", WebRedirectHandler, {"url": "/web_redirect_newpath"}),
             url("/web_redirect", WebRedirectHandler, {"url": "/web_redirect_newpath", "permanent": False}),
+            url("//web_redirect_double_slash", WebRedirectHandler, {"url": '/web_redirect_newpath'}),
             url("/header_injection", HeaderInjectionHandler),
             url("/get_argument", GetArgumentHandler),
             url("/get_arguments", GetArgumentsHandler),
@@ -730,6 +731,11 @@ js_embed()
         self.assertEqual(response.code, 302)
         self.assertEqual(response.headers['Location'], '/web_redirect_newpath')
 
+    def test_web_redirect_double_slash(self):
+        response = self.fetch("//web_redirect_double_slash", follow_redirects=False)
+        self.assertEqual(response.code, 301)
+        self.assertEqual(response.headers['Location'], '/web_redirect_newpath')
+
     def test_header_injection(self):
         response = self.fetch("/header_injection")
         self.assertEqual(response.body, b"ok")
diff --git a/tornado/testing.py b/tornado/testing.py
index 93f0dbe1..3d3bcf72 100644
--- a/tornado/testing.py
+++ b/tornado/testing.py
@@ -417,8 +417,10 @@ class AsyncHTTPSTestCase(AsyncHTTPTestCase):
     Interface is generally the same as `AsyncHTTPTestCase`.
     """
     def get_http_client(self):
-        return AsyncHTTPClient(io_loop=self.io_loop, force_instance=True,
-                               defaults=dict(validate_cert=False))
+        # Some versions of libcurl have deadlock bugs with ssl,
+        # so always run these tests with SimpleAsyncHTTPClient.
+        return SimpleAsyncHTTPClient(io_loop=self.io_loop, force_instance=True,
+                                     defaults=dict(validate_cert=False))
 
     def get_httpserver_options(self):
         return dict(ssl_options=self.get_ssl_options())