Skip to content

Commit 8b41fb8

Browse files
committed
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 <arpitjain099@gmail.com>
1 parent 3197848 commit 8b41fb8

2 files changed

Lines changed: 22 additions & 0 deletions

File tree

tests/test_fetcher_ng.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,25 @@ def test_session_get_timeout(self, mock_session_get: Mock) -> None:
137137
self.fetcher.fetch(self.url)
138138
mock_session_get.assert_called_once()
139139

140+
# A non-timeout connection failure (e.g. TLS error) must surface as the
141+
# original error, not be masked by an UnboundLocalError on "response".
142+
@patch.object(
143+
urllib3.PoolManager,
144+
"request",
145+
side_effect=urllib3.exceptions.MaxRetryError(
146+
urllib3.connectionpool.ConnectionPool("localhost"),
147+
"",
148+
urllib3.exceptions.SSLError("certificate verify failed"),
149+
),
150+
)
151+
def test_session_get_ssl_error(self, mock_session_get: Mock) -> None:
152+
with self.assertRaises(exceptions.DownloadError) as cm:
153+
self.fetcher.fetch(self.url)
154+
mock_session_get.assert_called_once()
155+
self.assertIsInstance(
156+
cm.exception.__cause__, urllib3.exceptions.MaxRetryError
157+
)
158+
140159
# Simple bytes download
141160
def test_download_bytes(self) -> None:
142161
data = self.fetcher.download_bytes(self.url, self.file_length)

tuf/ngclient/urllib3_fetcher.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ def _fetch(self, url: str) -> Iterator[bytes]:
8282
except urllib3.exceptions.MaxRetryError as e:
8383
if isinstance(e.reason, urllib3.exceptions.TimeoutError):
8484
raise exceptions.SlowRetrievalError from e
85+
# Any other reason (e.g. TLS or connection error): let the original
86+
# error propagate instead of falling through to an unbound response.
87+
raise
8588

8689
if response.status >= 400:
8790
response.close()

0 commit comments

Comments
 (0)