simple_httpclient: Start the connection timeout before the Resolver call.

Timeouts were previously unenforced during DNS resolution.

diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py
index 271b25be54bb14d8781284e3dd2da92c042b126f..faf75cec6d8bcf16ab6d42482d15c0bb574b2331 100644 (file)
--- a/tornado/simple_httpclient.py
+++ b/tornado/simple_httpclient.py
@@ -190,15 +190,18 @@ class _HTTPConnection(object):
                 # so restrict to ipv4 by default.
                 af = socket.AF_INET
 
+            timeout = min(self.request.connect_timeout, self.request.request_timeout)
+            if timeout:
+                self._timeout = self.io_loop.add_timeout(
+                    self.start_time + timeout,
+                    stack_context.wrap(self._on_timeout))
             self.resolver.resolve(host, port, af, callback=self._on_resolve)
 
     def _on_resolve(self, addrinfo):
+        if self.final_callback is None:
+            # final_callback is cleared if we've hit our timeout
+            return
         self.stream = self._create_stream(addrinfo)
-        timeout = min(self.request.connect_timeout, self.request.request_timeout)
-        if timeout:
-            self._timeout = self.io_loop.add_timeout(
-                self.start_time + timeout,
-                stack_context.wrap(self._on_timeout))
         self.stream.set_close_callback(self._on_close)
         # ipv6 addresses are broken (in self.parsed.hostname) until
         # 2.7, here is correctly parsed value calculated in __init__
@@ -261,6 +264,8 @@ class _HTTPConnection(object):
 
     def _on_connect(self):
         self._remove_timeout()
+        if self.final_callback is None:
+            return
         if self.request.request_timeout:
             self._timeout = self.io_loop.add_timeout(
                 self.start_time + self.request.request_timeout,

diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py
index 407275314cc13e5b0eccd6d19871c54aebb68fff..ac98aaae4846c4edadc280a698dc868b7a65d60b 100644 (file)
--- a/tornado/test/simple_httpclient_test.py
+++ b/tornado/test/simple_httpclient_test.py
@@ -14,6 +14,7 @@ from tornado.httpclient import AsyncHTTPClient
 from tornado.httputil import HTTPHeaders
 from tornado.ioloop import IOLoop
 from tornado.log import gen_log
+from tornado.netutil import Resolver
 from tornado.simple_httpclient import SimpleAsyncHTTPClient, _DEFAULT_CA_CERTS
 from tornado.test.httpclient_test import ChunkHandler, CountdownHandler, HelloWorldHandler
 from tornado.test import httpclient_test
@@ -412,3 +413,23 @@ class HostnameMappingTestCase(AsyncHTTPTestCase):
         response = self.wait()
         response.rethrow()
         self.assertEqual(response.body, b'Hello world!')
+
+
+class ResolveTimeoutTestCase(AsyncHTTPTestCase):
+    def setUp(self):
+        # Dummy Resolver subclass that never invokes its callback.
+        class BadResolver(Resolver):
+            def resolve(self, *args, **kwargs):
+                pass
+
+        super(ResolveTimeoutTestCase, self).setUp()
+        self.http_client = SimpleAsyncHTTPClient(
+            self.io_loop,
+            resolver=BadResolver())
+
+    def get_app(self):
+        return Application([url("/hello", HelloWorldHandler), ])
+
+    def test_resolve_timeout(self):
+        response = self.fetch('/hello', connect_timeout=0.1)
+        self.assertEqual(response.code, 599)