From 0675f0ce3ac2ae5db6bd17e878e5788f0abcf1a9 Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Mon, 6 Jan 2025 02:17:09 -0500 Subject: [PATCH 01/10] create urllib3 fetcher, replace requestsFetcher with urllibFetcher in ngclient, replace requestsFecther with urllibFetcher in requestsFetcher unit tests. Signed-off-by: NicholasTanz --- tests/test_fetcher_ng.py | 23 ++-- tests/test_updater_ng.py | 4 +- tuf/ngclient/__init__.py | 4 +- tuf/ngclient/_internal/urllib3_fetcher.py | 149 ++++++++++++++++++++++ tuf/ngclient/updater.py | 4 +- 5 files changed, 167 insertions(+), 17 deletions(-) create mode 100644 tuf/ngclient/_internal/urllib3_fetcher.py diff --git a/tests/test_fetcher_ng.py b/tests/test_fetcher_ng.py index c4f924867e..5c6de0f83e 100644 --- a/tests/test_fetcher_ng.py +++ b/tests/test_fetcher_ng.py @@ -1,7 +1,7 @@ # Copyright 2021, New York University and the TUF contributors # SPDX-License-Identifier: MIT OR Apache-2.0 -"""Unit test for RequestsFetcher.""" +"""Unit test for Urllib3Fetcher.""" import io import logging @@ -14,17 +14,17 @@ from typing import Any, ClassVar from unittest.mock import Mock, patch -import requests +import urllib3 from tests import utils from tuf.api import exceptions -from tuf.ngclient import RequestsFetcher +from tuf.ngclient import Urllib3Fetcher logger = logging.getLogger(__name__) class TestFetcher(unittest.TestCase): - """Test RequestsFetcher class.""" + """Test Urllib3Fetcher class.""" server_process_handler: ClassVar[utils.TestServerProcess] @@ -58,7 +58,7 @@ def tearDownClass(cls) -> None: def setUp(self) -> None: # Instantiate a concrete instance of FetcherInterface - self.fetcher = RequestsFetcher() + self.fetcher = Urllib3Fetcher() # Simple fetch. def test_fetch(self) -> None: @@ -105,11 +105,12 @@ def test_http_error(self) -> None: self.assertEqual(cm.exception.status_code, 404) # Response read timeout error - @patch.object(requests.Session, "get") + @patch.object(urllib3.PoolManager, "request") def test_response_read_timeout(self, mock_session_get: Mock) -> None: mock_response = Mock() + mock_response.status = 200 attr = { - "iter_content.side_effect": requests.exceptions.ConnectionError( + "stream.side_effect": urllib3.exceptions.ConnectionError( "Simulated timeout" ) } @@ -118,13 +119,13 @@ def test_response_read_timeout(self, mock_session_get: Mock) -> None: with self.assertRaises(exceptions.SlowRetrievalError): next(self.fetcher.fetch(self.url)) - mock_response.iter_content.assert_called_once() + mock_response.stream.assert_called_once() # Read/connect session timeout error @patch.object( - requests.Session, - "get", - side_effect=requests.exceptions.Timeout("Simulated timeout"), + urllib3.PoolManager, + "request", + side_effect=urllib3.exceptions.TimeoutError, ) def test_session_get_timeout(self, mock_session_get: Mock) -> None: with self.assertRaises(exceptions.SlowRetrievalError): diff --git a/tests/test_updater_ng.py b/tests/test_updater_ng.py index f42e510b1e..0611d0d7cd 100644 --- a/tests/test_updater_ng.py +++ b/tests/test_updater_ng.py @@ -325,7 +325,7 @@ def test_non_existing_target_file(self) -> None: def test_user_agent(self) -> None: # test default self.updater.refresh() - session = next(iter(self.updater._fetcher._sessions.values())) + session = next(iter(self.updater._fetcher._poolManagers.values())) ua = session.headers["User-Agent"] self.assertEqual(ua[:11], "python-tuf/") @@ -338,7 +338,7 @@ def test_user_agent(self) -> None: config=UpdaterConfig(app_user_agent="MyApp/1.2.3"), ) updater.refresh() - session = next(iter(updater._fetcher._sessions.values())) + session = next(iter(updater._fetcher._poolManagers.values())) ua = session.headers["User-Agent"] self.assertEqual(ua[:23], "MyApp/1.2.3 python-tuf/") diff --git a/tuf/ngclient/__init__.py b/tuf/ngclient/__init__.py index b2c5cbfd78..0c254e195a 100644 --- a/tuf/ngclient/__init__.py +++ b/tuf/ngclient/__init__.py @@ -8,14 +8,14 @@ # requests_fetcher is public but comes from _internal for now (because # sigstore-python 1.0 still uses the module from there). requests_fetcher # can be moved out of _internal once sigstore-python 1.0 is not relevant. -from tuf.ngclient._internal.requests_fetcher import RequestsFetcher +from tuf.ngclient._internal.urllib3_fetcher import Urllib3Fetcher from tuf.ngclient.config import UpdaterConfig from tuf.ngclient.fetcher import FetcherInterface from tuf.ngclient.updater import Updater __all__ = [ # noqa: PLE0604 FetcherInterface.__name__, - RequestsFetcher.__name__, + Urllib3Fetcher.__name__, TargetFile.__name__, Updater.__name__, UpdaterConfig.__name__, diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py new file mode 100644 index 0000000000..6d43389418 --- /dev/null +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -0,0 +1,149 @@ +# Copyright 2021, New York University and the TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +"""Provides an implementation of ``FetcherInterface`` using the urllib3 HTTP +library. +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING +from urllib import parse + +# Imports +import urllib3 + +import tuf +from tuf.api import exceptions +from tuf.ngclient.fetcher import FetcherInterface + +if TYPE_CHECKING: + from collections.abc import Iterator + +# Globals +logger = logging.getLogger(__name__) + + +# Classes +class Urllib3Fetcher(FetcherInterface): + """An implementation of ``FetcherInterface`` based on the urllib3 library. + + Attributes: + socket_timeout: Timeout in seconds, used for both initial connection + delay and the maximum delay between bytes received. + chunk_size: Chunk size in bytes used when downloading. + """ + + def __init__( + self, + socket_timeout: int = 30, + chunk_size: int = 400000, + app_user_agent: str | None = None, + ) -> None: + # NOTE: We use a separate urllib3.PoolManager per scheme+hostname + # combination, in order to reuse connections to the same hostname to + # improve efficiency, but avoiding sharing state between different + # hosts-scheme combinations to minimize subtle security issues. + # Some cookies may not be HTTP-safe. + self._poolManagers: dict[tuple[str, str], urllib3.PoolManager] = {} + + # Default settings + self.socket_timeout: int = socket_timeout # seconds + self.chunk_size: int = chunk_size # bytes + self.app_user_agent = app_user_agent + + def _fetch(self, url: str) -> Iterator[bytes]: + """Fetch the contents of HTTP/HTTPS url from a remote server. + + Args: + url: URL string that represents a file location. + + Raises: + exceptions.SlowRetrievalError: Timeout occurs while receiving + data. + exceptions.DownloadHTTPError: HTTP error code is received. + + Returns: + Bytes iterator + """ + # Get a customized session for each new schema+hostname combination. + poolmanager = self._get_poolmanager(url) + + # Get the urllib3.PoolManager object for this URL. + # + # Defer downloading the response body with preload_content=False. + # Always set the timeout. This timeout value is interpreted by + # urllib3 as: + # - connect timeout (max delay before first byte is received) + # - read (gap) timeout (max delay between bytes received) + try: + response = poolmanager.request("GET", + url, preload_content=False, timeout=urllib3.Timeout(connect=self.socket_timeout) + ) + except urllib3.exceptions.TimeoutError as e: + raise exceptions.SlowRetrievalError from e + + # Check response status. + try: + if response.status >= 400: + raise urllib3.exceptions.HTTPError + except urllib3.exceptions.HTTPError as e: + response.close() + status = response.status + raise exceptions.DownloadHTTPError(str(e), status) from e + + return self._chunks(response) + + def _chunks( + self, response: urllib3.response.HTTPResponse + ) -> Iterator[bytes]: + """A generator function to be returned by fetch. + + This way the caller of fetch can differentiate between connection + and actual data download. + """ + + try: + yield from response.stream(self.chunk_size) + except ( + urllib3.exceptions.ConnectionError, + urllib3.exceptions.TimeoutError, + ) as e: + raise exceptions.SlowRetrievalError from e + + finally: + response.close() + + def _get_poolmanager(self, url: str) -> urllib3.PoolManager: + """Return a different customized urllib3.PoolManager per schema+hostname + combination. + + Raises: + exceptions.DownloadError: When there is a problem parsing the url. + """ + # Use a different urllib3.PoolManager per schema+hostname + # combination, to reuse connections while minimizing subtle + # security issues. + parsed_url = parse.urlparse(url) + + if not parsed_url.scheme: + raise exceptions.DownloadError(f"Failed to parse URL {url}") + + poolmanager_index = (parsed_url.scheme, parsed_url.hostname or "") + poolmanager = self._poolManagers.get(poolmanager_index) + + if not poolmanager: + # no default User-Agent when creating a poolManager + ua = f"python-tuf/{tuf.__version__}" + if self.app_user_agent is not None: + ua = f"{self.app_user_agent} {ua}" + + poolmanager = urllib3.PoolManager(headers={"User-Agent" : ua}) + self._poolManagers[poolmanager_index] = poolmanager + + logger.debug("Made new poolManager %s", poolmanager_index) + else: + logger.debug("Reusing poolManager %s", poolmanager_index) + + return poolmanager diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index 51bda41f26..b58ecfed39 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -49,7 +49,7 @@ from tuf.api import exceptions from tuf.api.metadata import Root, Snapshot, TargetFile, Targets, Timestamp -from tuf.ngclient._internal import requests_fetcher, trusted_metadata_set +from tuf.ngclient._internal import trusted_metadata_set, urllib3_fetcher from tuf.ngclient.config import EnvelopeType, UpdaterConfig if TYPE_CHECKING: @@ -102,7 +102,7 @@ def __init__( if fetcher is not None: self._fetcher = fetcher else: - self._fetcher = requests_fetcher.RequestsFetcher( + self._fetcher = urllib3_fetcher.Urllib3Fetcher( app_user_agent=self.config.app_user_agent ) From 20d825f04146763d937489c74ae4a3e256def1ce Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Mon, 6 Jan 2025 02:31:20 -0500 Subject: [PATCH 02/10] fix line too long linting error Signed-off-by: NicholasTanz --- tuf/ngclient/_internal/urllib3_fetcher.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py index 6d43389418..e5146044fa 100644 --- a/tuf/ngclient/_internal/urllib3_fetcher.py +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -79,7 +79,8 @@ def _fetch(self, url: str) -> Iterator[bytes]: # - read (gap) timeout (max delay between bytes received) try: response = poolmanager.request("GET", - url, preload_content=False, timeout=urllib3.Timeout(connect=self.socket_timeout) + url, preload_content=False, + timeout=urllib3.Timeout(connect=self.socket_timeout) ) except urllib3.exceptions.TimeoutError as e: raise exceptions.SlowRetrievalError from e From 031778fd8d405ae79dfb9312e230ed988b7eaa53 Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Mon, 6 Jan 2025 02:47:51 -0500 Subject: [PATCH 03/10] more linting stuff Signed-off-by: NicholasTanz --- tuf/ngclient/_internal/urllib3_fetcher.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py index e5146044fa..46aed2a03f 100644 --- a/tuf/ngclient/_internal/urllib3_fetcher.py +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -78,17 +78,19 @@ def _fetch(self, url: str) -> Iterator[bytes]: # - connect timeout (max delay before first byte is received) # - read (gap) timeout (max delay between bytes received) try: - response = poolmanager.request("GET", - url, preload_content=False, - timeout=urllib3.Timeout(connect=self.socket_timeout) + response = poolmanager.request( + "GET", + url, + preload_content=False, + timeout=urllib3.Timeout(connect=self.socket_timeout), ) except urllib3.exceptions.TimeoutError as e: raise exceptions.SlowRetrievalError from e # Check response status. try: - if response.status >= 400: - raise urllib3.exceptions.HTTPError + if response.status >= 400: + raise urllib3.exceptions.HTTPError except urllib3.exceptions.HTTPError as e: response.close() status = response.status @@ -97,7 +99,7 @@ def _fetch(self, url: str) -> Iterator[bytes]: return self._chunks(response) def _chunks( - self, response: urllib3.response.HTTPResponse + self, response: urllib3.response.BaseHTTPResponse ) -> Iterator[bytes]: """A generator function to be returned by fetch. @@ -140,7 +142,7 @@ def _get_poolmanager(self, url: str) -> urllib3.PoolManager: if self.app_user_agent is not None: ua = f"{self.app_user_agent} {ua}" - poolmanager = urllib3.PoolManager(headers={"User-Agent" : ua}) + poolmanager = urllib3.PoolManager(headers={"User-Agent": ua}) self._poolManagers[poolmanager_index] = poolmanager logger.debug("Made new poolManager %s", poolmanager_index) From 18e42cea5293c9aaddd78d1a5bf8a0d79a71607a Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Mon, 6 Jan 2025 02:55:15 -0500 Subject: [PATCH 04/10] replacing RequestsFecther with Urllib3Fetcher in .rst Signed-off-by: NicholasTanz --- docs/api/tuf.ngclient.fetcher.rst | 2 +- tuf/ngclient/updater.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/tuf.ngclient.fetcher.rst b/docs/api/tuf.ngclient.fetcher.rst index ad64b49341..5476512d99 100644 --- a/docs/api/tuf.ngclient.fetcher.rst +++ b/docs/api/tuf.ngclient.fetcher.rst @@ -5,5 +5,5 @@ Fetcher :undoc-members: :private-members: _fetch -.. autoclass:: tuf.ngclient.RequestsFetcher +.. autoclass:: tuf.ngclient.Urllib3Fetcher :no-inherited-members: diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index b58ecfed39..022d601f95 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -71,7 +71,7 @@ class Updater: target_base_url: ``Optional``; Default base URL for all remote target downloads. Can be individually set in ``download_target()`` fetcher: ``Optional``; ``FetcherInterface`` implementation used to - download both metadata and targets. Default is ``RequestsFetcher`` + download both metadata and targets. Default is ``Urllib3Fetcher`` config: ``Optional``; ``UpdaterConfig`` could be used to setup common configuration options. From 21280302e78bf0b92ef62e207a76465f7def045e Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Thu, 9 Jan 2025 20:59:56 -0500 Subject: [PATCH 05/10] utilize one pool manager Signed-off-by: NicholasTanz --- tests/test_updater_ng.py | 4 +- tuf/ngclient/_internal/urllib3_fetcher.py | 70 +++++------------------ 2 files changed, 17 insertions(+), 57 deletions(-) diff --git a/tests/test_updater_ng.py b/tests/test_updater_ng.py index 0611d0d7cd..dcb02ce86b 100644 --- a/tests/test_updater_ng.py +++ b/tests/test_updater_ng.py @@ -325,7 +325,7 @@ def test_non_existing_target_file(self) -> None: def test_user_agent(self) -> None: # test default self.updater.refresh() - session = next(iter(self.updater._fetcher._poolManagers.values())) + session = self.updater._fetcher._poolManager ua = session.headers["User-Agent"] self.assertEqual(ua[:11], "python-tuf/") @@ -338,7 +338,7 @@ def test_user_agent(self) -> None: config=UpdaterConfig(app_user_agent="MyApp/1.2.3"), ) updater.refresh() - session = next(iter(updater._fetcher._poolManagers.values())) + session = updater._fetcher._poolManager ua = session.headers["User-Agent"] self.assertEqual(ua[:23], "MyApp/1.2.3 python-tuf/") diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py index 46aed2a03f..f2c9b4f176 100644 --- a/tuf/ngclient/_internal/urllib3_fetcher.py +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -9,7 +9,6 @@ import logging from typing import TYPE_CHECKING -from urllib import parse # Imports import urllib3 @@ -41,18 +40,18 @@ def __init__( chunk_size: int = 400000, app_user_agent: str | None = None, ) -> None: - # NOTE: We use a separate urllib3.PoolManager per scheme+hostname - # combination, in order to reuse connections to the same hostname to - # improve efficiency, but avoiding sharing state between different - # hosts-scheme combinations to minimize subtle security issues. - # Some cookies may not be HTTP-safe. - self._poolManagers: dict[tuple[str, str], urllib3.PoolManager] = {} - # Default settings self.socket_timeout: int = socket_timeout # seconds self.chunk_size: int = chunk_size # bytes self.app_user_agent = app_user_agent + # Create User-Agent. + ua = f"python-tuf/{tuf.__version__}" + if self.app_user_agent is not None: + ua = f"{self.app_user_agent} {ua}" + + self._poolManager = urllib3.PoolManager(headers={"User-Agent": ua}) + def _fetch(self, url: str) -> Iterator[bytes]: """Fetch the contents of HTTP/HTTPS url from a remote server. @@ -67,34 +66,28 @@ def _fetch(self, url: str) -> Iterator[bytes]: Returns: Bytes iterator """ - # Get a customized session for each new schema+hostname combination. - poolmanager = self._get_poolmanager(url) - # Get the urllib3.PoolManager object for this URL. - # # Defer downloading the response body with preload_content=False. # Always set the timeout. This timeout value is interpreted by # urllib3 as: # - connect timeout (max delay before first byte is received) # - read (gap) timeout (max delay between bytes received) try: - response = poolmanager.request( + response = self._poolManager.request( "GET", url, preload_content=False, - timeout=urllib3.Timeout(connect=self.socket_timeout), + timeout=urllib3.Timeout(self.socket_timeout), ) except urllib3.exceptions.TimeoutError as e: raise exceptions.SlowRetrievalError from e - # Check response status. - try: - if response.status >= 400: - raise urllib3.exceptions.HTTPError - except urllib3.exceptions.HTTPError as e: + if response.status >= 400: response.close() - status = response.status - raise exceptions.DownloadHTTPError(str(e), status) from e + raise exceptions.DownloadHTTPError( + f"HTTP error occurred with status {response.status}", + response.status, + ) return self._chunks(response) @@ -116,37 +109,4 @@ def _chunks( raise exceptions.SlowRetrievalError from e finally: - response.close() - - def _get_poolmanager(self, url: str) -> urllib3.PoolManager: - """Return a different customized urllib3.PoolManager per schema+hostname - combination. - - Raises: - exceptions.DownloadError: When there is a problem parsing the url. - """ - # Use a different urllib3.PoolManager per schema+hostname - # combination, to reuse connections while minimizing subtle - # security issues. - parsed_url = parse.urlparse(url) - - if not parsed_url.scheme: - raise exceptions.DownloadError(f"Failed to parse URL {url}") - - poolmanager_index = (parsed_url.scheme, parsed_url.hostname or "") - poolmanager = self._poolManagers.get(poolmanager_index) - - if not poolmanager: - # no default User-Agent when creating a poolManager - ua = f"python-tuf/{tuf.__version__}" - if self.app_user_agent is not None: - ua = f"{self.app_user_agent} {ua}" - - poolmanager = urllib3.PoolManager(headers={"User-Agent": ua}) - self._poolManagers[poolmanager_index] = poolmanager - - logger.debug("Made new poolManager %s", poolmanager_index) - else: - logger.debug("Reusing poolManager %s", poolmanager_index) - - return poolmanager + response.release_conn() From 2aed81f0196d327b8651fc2c4e0298102f5cceb3 Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Thu, 9 Jan 2025 23:31:50 -0500 Subject: [PATCH 06/10] change error handling to MaxRetryError in _fetch() Signed-off-by: NicholasTanz --- tests/test_fetcher_ng.py | 12 ++++++------ tuf/ngclient/_internal/urllib3_fetcher.py | 10 ++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/test_fetcher_ng.py b/tests/test_fetcher_ng.py index 5c6de0f83e..434c62a233 100644 --- a/tests/test_fetcher_ng.py +++ b/tests/test_fetcher_ng.py @@ -109,11 +109,7 @@ def test_http_error(self) -> None: def test_response_read_timeout(self, mock_session_get: Mock) -> None: mock_response = Mock() mock_response.status = 200 - attr = { - "stream.side_effect": urllib3.exceptions.ConnectionError( - "Simulated timeout" - ) - } + attr = {"stream.side_effect": urllib3.exceptions.TimeoutError} mock_response.configure_mock(**attr) mock_session_get.return_value = mock_response @@ -125,7 +121,11 @@ def test_response_read_timeout(self, mock_session_get: Mock) -> None: @patch.object( urllib3.PoolManager, "request", - side_effect=urllib3.exceptions.TimeoutError, + side_effect=urllib3.exceptions.MaxRetryError( + urllib3.connectionpool.ConnectionPool("localhost"), + "", + urllib3.exceptions.TimeoutError(), + ), ) def test_session_get_timeout(self, mock_session_get: Mock) -> None: with self.assertRaises(exceptions.SlowRetrievalError): diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py index f2c9b4f176..85cc80d5fb 100644 --- a/tuf/ngclient/_internal/urllib3_fetcher.py +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -79,8 +79,9 @@ def _fetch(self, url: str) -> Iterator[bytes]: preload_content=False, timeout=urllib3.Timeout(self.socket_timeout), ) - except urllib3.exceptions.TimeoutError as e: - raise exceptions.SlowRetrievalError from e + except urllib3.exceptions.MaxRetryError as e: + if isinstance(e.reason, urllib3.exceptions.TimeoutError): + raise exceptions.SlowRetrievalError from e if response.status >= 400: response.close() @@ -102,10 +103,7 @@ def _chunks( try: yield from response.stream(self.chunk_size) - except ( - urllib3.exceptions.ConnectionError, - urllib3.exceptions.TimeoutError, - ) as e: + except urllib3.exceptions.TimeoutError as e: raise exceptions.SlowRetrievalError from e finally: From a48fca51f9b2cc6bb74acd428ee05f34f75f342b Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Thu, 9 Jan 2025 23:56:06 -0500 Subject: [PATCH 07/10] add retry error handling to _chunks() Signed-off-by: NicholasTanz --- tests/test_fetcher_ng.py | 6 +++++- tuf/ngclient/_internal/urllib3_fetcher.py | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_fetcher_ng.py b/tests/test_fetcher_ng.py index 434c62a233..03cb33f36d 100644 --- a/tests/test_fetcher_ng.py +++ b/tests/test_fetcher_ng.py @@ -109,7 +109,11 @@ def test_http_error(self) -> None: def test_response_read_timeout(self, mock_session_get: Mock) -> None: mock_response = Mock() mock_response.status = 200 - attr = {"stream.side_effect": urllib3.exceptions.TimeoutError} + attr = {"stream.side_effect": urllib3.exceptions.MaxRetryError( + urllib3.connectionpool.ConnectionPool("localhost"), + "", + urllib3.exceptions.TimeoutError(), + )} mock_response.configure_mock(**attr) mock_session_get.return_value = mock_response diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py index 85cc80d5fb..8c2a2ff6df 100644 --- a/tuf/ngclient/_internal/urllib3_fetcher.py +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -103,8 +103,9 @@ def _chunks( try: yield from response.stream(self.chunk_size) - except urllib3.exceptions.TimeoutError as e: - raise exceptions.SlowRetrievalError from e + except urllib3.exceptions.MaxRetryError as e: + if isinstance(e.reason, urllib3.exceptions.TimeoutError): + raise exceptions.SlowRetrievalError from e finally: response.release_conn() From f8b1dbd253b317c3e6730e98b1c263502cc21cf6 Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Thu, 9 Jan 2025 23:59:13 -0500 Subject: [PATCH 08/10] linting Signed-off-by: NicholasTanz --- tests/test_fetcher_ng.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_fetcher_ng.py b/tests/test_fetcher_ng.py index 03cb33f36d..a8b58ca2b0 100644 --- a/tests/test_fetcher_ng.py +++ b/tests/test_fetcher_ng.py @@ -109,11 +109,13 @@ def test_http_error(self) -> None: def test_response_read_timeout(self, mock_session_get: Mock) -> None: mock_response = Mock() mock_response.status = 200 - attr = {"stream.side_effect": urllib3.exceptions.MaxRetryError( - urllib3.connectionpool.ConnectionPool("localhost"), - "", - urllib3.exceptions.TimeoutError(), - )} + attr = { + "stream.side_effect": urllib3.exceptions.MaxRetryError( + urllib3.connectionpool.ConnectionPool("localhost"), + "", + urllib3.exceptions.TimeoutError(), + ) + } mock_response.configure_mock(**attr) mock_session_get.return_value = mock_response From 86cc7ad3ee942223fff0024b911abc5799995ae9 Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Thu, 30 Jan 2025 21:29:08 -0500 Subject: [PATCH 09/10] clarify urllib3 as requirement in pyproject.toml and add back in requestsFetcher as option. Signed-off-by: NicholasTanz --- pyproject.toml | 3 ++- tuf/ngclient/__init__.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index fae68878a6..4e829e7576 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,6 +47,7 @@ classifiers = [ dependencies = [ "requests>=2.19.1", "securesystemslib~=1.0", + "urllib3<3,>=1.21.1", ] dynamic = ["version"] @@ -156,4 +157,4 @@ exclude_also = [ ] [tool.coverage.run] branch = true -omit = [ "tests/*" ] +omit = [ "tests/*", "tuf/ngclient/_internal/requests_fetcher.py" ] diff --git a/tuf/ngclient/__init__.py b/tuf/ngclient/__init__.py index 0c254e195a..1d2084acf5 100644 --- a/tuf/ngclient/__init__.py +++ b/tuf/ngclient/__init__.py @@ -8,6 +8,7 @@ # requests_fetcher is public but comes from _internal for now (because # sigstore-python 1.0 still uses the module from there). requests_fetcher # can be moved out of _internal once sigstore-python 1.0 is not relevant. +from tuf.ngclient._internal.requests_fetcher import RequestsFetcher from tuf.ngclient._internal.urllib3_fetcher import Urllib3Fetcher from tuf.ngclient.config import UpdaterConfig from tuf.ngclient.fetcher import FetcherInterface @@ -15,6 +16,7 @@ __all__ = [ # noqa: PLE0604 FetcherInterface.__name__, + RequestsFetcher.__name__, Urllib3Fetcher.__name__, TargetFile.__name__, Updater.__name__, From d67f126233fc280f148d9b0c10d220466fd6813e Mon Sep 17 00:00:00 2001 From: NicholasTanz Date: Wed, 5 Feb 2025 17:49:02 -0500 Subject: [PATCH 10/10] remove self.app_user_agent attribute, as it's not used outside of init Signed-off-by: NicholasTanz --- tuf/ngclient/_internal/urllib3_fetcher.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tuf/ngclient/_internal/urllib3_fetcher.py b/tuf/ngclient/_internal/urllib3_fetcher.py index 8c2a2ff6df..5d4bb1cf60 100644 --- a/tuf/ngclient/_internal/urllib3_fetcher.py +++ b/tuf/ngclient/_internal/urllib3_fetcher.py @@ -43,12 +43,11 @@ def __init__( # Default settings self.socket_timeout: int = socket_timeout # seconds self.chunk_size: int = chunk_size # bytes - self.app_user_agent = app_user_agent # Create User-Agent. ua = f"python-tuf/{tuf.__version__}" - if self.app_user_agent is not None: - ua = f"{self.app_user_agent} {ua}" + if app_user_agent is not None: + ua = f"{app_user_agent} {ua}" self._poolManager = urllib3.PoolManager(headers={"User-Agent": ua})