From 7732baff8b3b98e7c1e3898815392efdaa602cab Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 20 Jan 2022 17:45:18 +0200 Subject: [PATCH 1/2] Remove LengthOrHashMismatchError catch and reraise We no longer need to catch LengthOrHashMismatchError and reraise a RepositoryError as LengthOrHashMismatchError is changed to inherit RepositoryError. Signed-off-by: Martin Vrachev --- tuf/ngclient/updater.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index 7fe48a11c7..3a42617a81 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -248,12 +248,7 @@ def download_target( with self._fetcher.download_file( full_url, targetinfo.length ) as target_file: - try: - targetinfo.verify_length_and_hashes(target_file) - except exceptions.LengthOrHashMismatchError as e: - raise exceptions.RepositoryError( - f"{target_filepath} length or hashes do not match" - ) from e + targetinfo.verify_length_and_hashes(target_file) sslib_util.persist_temp_file(target_file, filepath) From 9f7866db0a8309dd1286f7f35e28d3fee98c094a Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 20 Jan 2022 18:38:55 +0200 Subject: [PATCH 2/2] Additions to the client exceptions I made a review on all files inside tuf/ngclient to see which of them needs additions or changes in their function docstrings regarding exceptions. I didn't find any changes required inside the request_fetcher.py and of course inside the config module. Other than that multiple additions had to be made. For trusted_metadata_set we had a discussion with Jussi that there is no need to list each of the specific RepositoryErrors one by one as this is an internal module and this will only create a bigger maintenance burden. For updater.py we had discussions with Jussi and Lukas that we want to document only those exceptions that could be potentially handled. This means there is no point in documenting each of the RepositoryErrors or DownloadErrors separately. Finally, I added a little documentation for download_bytes() inside fetcher.py, as it's naming, suggests it's not an internal function. Signed-off-by: Martin Vrachev --- tuf/ngclient/_internal/trusted_metadata_set.py | 5 +++++ tuf/ngclient/fetcher.py | 11 +++++++++++ tuf/ngclient/updater.py | 10 ++++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/tuf/ngclient/_internal/trusted_metadata_set.py b/tuf/ngclient/_internal/trusted_metadata_set.py index e23fa45950..ad91c8479e 100644 --- a/tuf/ngclient/_internal/trusted_metadata_set.py +++ b/tuf/ngclient/_internal/trusted_metadata_set.py @@ -150,6 +150,7 @@ def update_root(self, data: bytes) -> Metadata[Root]: data: unverified new root metadata as bytes Raises: + RuntimeError: This function is called after updating timestamp. RepositoryError: Metadata failed to load or verify. The actual error type and content will contain more details. @@ -198,6 +199,7 @@ def update_timestamp(self, data: bytes) -> Metadata[Timestamp]: data: unverified new timestamp metadata as bytes Raises: + RuntimeError: This function is called after updating snapshot. RepositoryError: Metadata failed to load or verify as final timestamp. The actual error type and content will contain more details. @@ -281,6 +283,8 @@ def update_snapshot( match data. Default is False. Raises: + RuntimeError: This function is called before updating timestamp + or after updating targets. RepositoryError: data failed to load or verify as final snapshot. The actual error type and content will contain more details. @@ -385,6 +389,7 @@ def update_delegated_targets( delegator_name: The name of the role delegating to the new metadata Raises: + RuntimeError: This function is called before updating snapshot. RepositoryError: Metadata failed to load or verify. The actual error type and content will contain more details. diff --git a/tuf/ngclient/fetcher.py b/tuf/ngclient/fetcher.py index 1ab2e5363a..a56c66dc78 100644 --- a/tuf/ngclient/fetcher.py +++ b/tuf/ngclient/fetcher.py @@ -90,6 +90,17 @@ def download_bytes(self, url: str, max_length: int) -> bytes: """Download bytes from given url Returns the downloaded bytes, otherwise like download_file() + + Args: + url: a URL string that represents the location of the file. + max_length: upper bound of data size in bytes. + + Raises: + exceptions.DownloadLengthMismatchError: downloaded bytes exceed + 'max_length'. + + Returns: + The content of the file in bytes. """ with self.download_file(url, max_length) as dl_file: return dl_file.read() diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index 3a42617a81..08153dce22 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -121,7 +121,7 @@ def refresh(self) -> None: Raises: OSError: New metadata could not be written to disk RepositoryError: Metadata failed to verify in some way - TODO: download-related errors + DownloadError: Download of a metadata file failed in some way """ self._load_root() @@ -157,7 +157,7 @@ def get_targetinfo(self, target_path: str) -> Optional[TargetFile]: Raises: OSError: New metadata could not be written to disk RepositoryError: Metadata failed to verify in some way - TODO: download-related errors + DownloadError: Download of a metadata file failed in some way Returns: A TargetFile instance or None. @@ -216,8 +216,10 @@ def download_target( Raises: ValueError: Invalid arguments - TODO: download-related errors - TODO: file write errors + DownloadError: Download of the target file failed in some way + RepositoryError: Downloaded target failed to be verified in some way + exceptions.StorageError: Downloaded target could not be written + to disk Returns: Local path to downloaded file