From 8b41fb8bad34a08aa0484a6379e86f0f1a2a001d Mon Sep 17 00:00:00 2001 From: arpitjain099 Date: Wed, 8 Jul 2026 21:51:16 +0900 Subject: [PATCH] ngclient: propagate non-timeout urllib3 connection errors Urllib3Fetcher._fetch caught MaxRetryError and only re-raised it (as SlowRetrievalError) when the reason was a timeout. For any other reason, such as a TLS certificate error, it fell through to the response.status check with response still unbound, so the caller saw an UnboundLocalError instead of the real connection error. Re-raise the original error when the reason is not a timeout, so fetch() wraps the meaningful error in a DownloadError as documented. Add a regression test covering a non-timeout MaxRetryError. Signed-off-by: arpitjain099 --- tests/test_fetcher_ng.py | 19 +++++++++++++++++++ tuf/ngclient/urllib3_fetcher.py | 3 +++ 2 files changed, 22 insertions(+) diff --git a/tests/test_fetcher_ng.py b/tests/test_fetcher_ng.py index 7ef7c11b70..43c5fc61fc 100644 --- a/tests/test_fetcher_ng.py +++ b/tests/test_fetcher_ng.py @@ -137,6 +137,25 @@ def test_session_get_timeout(self, mock_session_get: Mock) -> None: self.fetcher.fetch(self.url) mock_session_get.assert_called_once() + # A non-timeout connection failure (e.g. TLS error) must surface as the + # original error, not be masked by an UnboundLocalError on "response". + @patch.object( + urllib3.PoolManager, + "request", + side_effect=urllib3.exceptions.MaxRetryError( + urllib3.connectionpool.ConnectionPool("localhost"), + "", + urllib3.exceptions.SSLError("certificate verify failed"), + ), + ) + def test_session_get_ssl_error(self, mock_session_get: Mock) -> None: + with self.assertRaises(exceptions.DownloadError) as cm: + self.fetcher.fetch(self.url) + mock_session_get.assert_called_once() + self.assertIsInstance( + cm.exception.__cause__, urllib3.exceptions.MaxRetryError + ) + # Simple bytes download def test_download_bytes(self) -> None: data = self.fetcher.download_bytes(self.url, self.file_length) diff --git a/tuf/ngclient/urllib3_fetcher.py b/tuf/ngclient/urllib3_fetcher.py index 88d447bd30..1e10e54dcb 100644 --- a/tuf/ngclient/urllib3_fetcher.py +++ b/tuf/ngclient/urllib3_fetcher.py @@ -82,6 +82,9 @@ def _fetch(self, url: str) -> Iterator[bytes]: except urllib3.exceptions.MaxRetryError as e: if isinstance(e.reason, urllib3.exceptions.TimeoutError): raise exceptions.SlowRetrievalError from e + # Any other reason (e.g. TLS or connection error): let the original + # error propagate instead of falling through to an unbound response. + raise if response.status >= 400: response.close()