Merge pull request #3633 from bdarnell/curl-reset-65

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

Author: bdarnell

docs/releases.rst

@@ -4,6 +4,7 @@ Release notes
 .. toctree::
    :maxdepth: 2
 
+   releases/v6.5.7
    releases/v6.5.6
    releases/v6.5.5
    releases/v6.5.4

docs/releases/v6.5.7.rst

@@ -0,0 +1,13 @@
+What's new in Tornado 6.5.7
+===========================
+
+Jun 8, 2026
+-----------
+
+Security fixes
+~~~~~~~~~~~~~~
+
+- ``CurlAsyncHTTPClient`` now fully resets the curl object before reusing it. This prevents
+  incorrectly reusing options from a previous request, specifically including client SSL and
+  credentials used for accessing proxies. Thanks to `Koh Jun Sheng <https://github.com/seankohjs>`_
+  for reporting this issue.

tornado/__init__.py

@@ -22,8 +22,8 @@
 # is zero for an official release, positive for a development branch,
 # or negative for a release candidate or beta (after the base version
 # number has been incremented)
-version = "6.5.6"
-version_info = (6, 5, 6, 0)
+version = "6.5.7"
+version_info = (6, 5, 7, 0)
 
 import importlib
 import typing

tornado/curl_httpclient.py

@@ -225,6 +225,7 @@ def _process_queue(self) -> None:
                     # _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:
@@ -242,7 +243,6 @@ def _finish(
         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
@@ -286,12 +286,22 @@ def _finish(
             )
         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)
@@ -300,15 +310,7 @@ def _curl_create(self) -> pycurl.Curl:
         ):  # 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

tornado/test/curl_httpclient_test.py

@@ -1,12 +1,14 @@
 from hashlib import md5
+import os
+import ssl
 import unittest
 
 from tornado.escape import utf8
-from tornado.testing import AsyncHTTPTestCase
+from tornado.netutil import ssl_options_to_context
 from tornado.test import httpclient_test
+from tornado.testing import AsyncHTTPSTestCase, AsyncHTTPTestCase
 from tornado.web import Application, RequestHandler
 
-
 try:
     import pycurl
 except ImportError:
@@ -123,3 +125,98 @@ def test_digest_auth_non_ascii(self):
             auth_password="barユ£",
         )
         self.assertEqual(response.body, b"ok")
+
+
+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")