curl_httpclient: Reset the curl object before putting it on the freelist

We previously did a piecemeal reset of the curl object, but missed some
options (and in fact many of these options cannot be unset without
fully resetting the object). This allowed some options to leak between
requests.

Now we use the curl reset method to ensure we catch everything.

diff --git a/tornado/curl_httpclient.py b/tornado/curl_httpclient.py
index b7f482033d9583407434f4ad1356ec9bd0c2cd0d..98da8543cc904c157f6e1df1c2e7b0b7a46d6d22 100644 (file)
--- a/tornado/curl_httpclient.py
+++ b/tornado/curl_httpclient.py
@@ -221,6 +221,7 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
                     # _process_queue() is called from
                     # _finish_pending_requests the exceptions have
                     # nowhere to go.
+                    curl.reset()
                     self._free_list.append(curl)
                     callback(HTTPResponse(request=request, code=599, error=e))
                 else:
@@ -238,7 +239,6 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
         info = curl.info  # type: ignore
         curl.info = None  # type: ignore
         self._multi.remove_handle(curl)
-        self._free_list.append(curl)
         buffer = info["buffer"]
         if curl_error:
             assert curl_message is not None
@@ -282,12 +282,22 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
             )
         except Exception:
             self.handle_callback_exception(info["callback"])
+        curl.reset()
+        self._free_list.append(curl)
 
     def handle_callback_exception(self, callback: Any) -> None:
         app_log.error("Exception in callback %r", callback, exc_info=True)
 
     def _curl_create(self) -> pycurl.Curl:
-        curl = pycurl.Curl()
+        return pycurl.Curl()
+
+    def _curl_setup_request(
+        self,
+        curl: pycurl.Curl,
+        request: HTTPRequest,
+        buffer: BytesIO,
+        headers: httputil.HTTPHeaders,
+    ) -> None:
         if curl_log.isEnabledFor(logging.DEBUG):
             curl.setopt(pycurl.VERBOSE, 1)
             curl.setopt(pycurl.DEBUGFUNCTION, self._curl_debug)
@@ -296,15 +306,7 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
         ):  # PROTOCOLS first appeared in pycurl 7.19.5 (2014-07-12)
             curl.setopt(pycurl.PROTOCOLS, pycurl.PROTO_HTTP | pycurl.PROTO_HTTPS)
             curl.setopt(pycurl.REDIR_PROTOCOLS, pycurl.PROTO_HTTP | pycurl.PROTO_HTTPS)
-        return curl
 
-    def _curl_setup_request(
-        self,
-        curl: pycurl.Curl,
-        request: HTTPRequest,
-        buffer: BytesIO,
-        headers: httputil.HTTPHeaders,
-    ) -> None:
         curl.setopt(pycurl.URL, native_str(request.url))
 
         # libcurl's magic "Expect: 100-continue" behavior causes delays

diff --git a/tornado/test/curl_httpclient_test.py b/tornado/test/curl_httpclient_test.py
index 0c4766fe34f3bc11a0366ca2ba16bb8adb5a5391..dfbf69a1717a2decf25ba02b94042cad84436269 100644 (file)
--- a/tornado/test/curl_httpclient_test.py
+++ b/tornado/test/curl_httpclient_test.py
@@ -1,10 +1,13 @@
-import unittest
 from hashlib import md5
+import os
+import ssl
+import unittest
 
 from tornado import gen
 from tornado.escape import utf8
+from tornado.netutil import ssl_options_to_context
 from tornado.test import httpclient_test
-from tornado.testing import AsyncHTTPTestCase
+from tornado.testing import AsyncHTTPSTestCase, AsyncHTTPTestCase
 from tornado.web import Application, RequestHandler
 
 try:
@@ -139,3 +142,98 @@ class CurlHTTPClientTestCase(AsyncHTTPTestCase):
 
         with self.assertRaises(TypeError):
             self.fetch("/digest", streaming_callback=_async_recv_chunk)
+
+
+class ProxyAuthEchoHandler(RequestHandler):
+    def get(self):
+        if self.request.headers.get("Proxy-Authorization", None) is not None:
+            self.write(f"proxy auth: {self.request.headers['Proxy-Authorization']}")
+        else:
+            self.write("no proxy auth")
+
+
+@unittest.skipIf(pycurl is None, "pycurl module not present")
+class CurlHTTPClientReuseProxyAuthTestCase(AsyncHTTPTestCase):
+    def get_app(self):
+        # Note that we don't properly support proxy-style requests, but it works well enough
+        # for this test if we start the url matcher with a wildcard.
+        return Application([(".*/proxy_auth", ProxyAuthEchoHandler)])
+
+    def get_http_client(self):
+        # max_clients=1 forces us to reuse curl "easy handles". This is a regression test for
+        # a bug in which proxy credentials were not cleared between requests.
+        return CurlAsyncHTTPClient(
+            force_instance=True,
+            defaults=dict(
+                allow_ipv6=False,
+            ),
+            max_clients=1,
+        )
+
+    def test_reuse_proxy_credentials(self):
+        # Proxy credentials used on one request should not be automatically reused
+        # by another request.
+        response = self.fetch(
+            "/proxy_auth",
+            proxy_host="127.0.0.1",
+            proxy_port=self.get_http_port(),
+            proxy_username="foo",
+            proxy_password="bar",
+        )
+        self.assertEqual(response.body, b"proxy auth: Basic Zm9vOmJhcg==")
+        response = self.fetch(
+            "/proxy_auth",
+            proxy_host="127.0.0.1",
+            proxy_port=self.get_http_port(),
+        )
+        self.assertEqual(response.body, b"no proxy auth")
+
+
+class ClientCertEchoHandler(RequestHandler):
+    def get(self):
+        cert = self.request.get_ssl_certificate()
+        if cert is not None:
+            assert isinstance(cert, dict)
+            self.write(f"client cert: {cert['subject']}")
+        else:
+            self.write("no client cert")
+
+
+@unittest.skipIf(pycurl is None, "pycurl module not present")
+class CurlHTTPClientReuseCertsTestCase(AsyncHTTPSTestCase):
+    def get_app(self):
+        return Application([(".*/client_cert", ClientCertEchoHandler)])
+
+    def get_http_client(self):
+        return CurlAsyncHTTPClient(
+            force_instance=True,
+            defaults=dict(
+                allow_ipv6=False,
+                validate_cert=False,
+            ),
+            max_clients=1,
+        )
+
+    def get_httpserver_options(self):
+        ssl_ctx = ssl_options_to_context(self.get_ssl_options(), server_side=True)
+        ssl_ctx.verify_mode = ssl.CERT_OPTIONAL
+        return dict(ssl_options=ssl_ctx)
+
+    def get_ssl_options(self):
+        opts = super().get_ssl_options()
+        opts["ca_certs"] = os.path.join(os.path.dirname(__file__), "test.crt")
+        return opts
+
+    def test_reuse_certs(self):
+        # Client certs used on one request should not be automatically reused
+        # by another request.
+        response = self.fetch(
+            self.get_url("/client_cert"),
+            client_cert=os.path.join(os.path.dirname(__file__), "test.crt"),
+            client_key=os.path.join(os.path.dirname(__file__), "test.key"),
+        )
+        self.assertEqual(
+            response.body, b"client cert: ((('commonName', 'foo.example.com'),),)"
+        )
+        response = self.fetch(self.get_url("/client_cert"))
+        self.assertEqual(response.body, b"no client cert")