From: Ben Darnell Date: Fri, 5 Jun 2026 19:33:33 +0000 (-0400) Subject: curl_httpclient: Reset the curl object before putting it on the freelist X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=45e47bbffb67abea90ae999d48147d3115b983fc;p=thirdparty%2Ftornado.git 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 b7f48203..98da8543 100644 --- 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 0c4766fe..dfbf69a1 100644 --- 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")