diff --git a/tests/test_updater_with_simulator.py b/tests/test_updater_with_simulator.py index 5e9e786ace..f0e08f4680 100644 --- a/tests/test_updater_with_simulator.py +++ b/tests/test_updater_with_simulator.py @@ -80,32 +80,37 @@ def test_refresh(self): self._run_refresh() def test_targets(self): - # target does not exist yet - updater = self._run_refresh() - self.assertIsNone(updater.get_one_valid_targetinfo("file")) + targets = { + "targetpath": b"content", + "åäö": b"more content" + } + # Add targets to repository self.sim.targets.version += 1 - self.sim.add_target("targets", b"content", "file") + for targetpath, content in targets.items(): + self.sim.add_target("targets", content, targetpath) self.sim.update_snapshot() - # target now exists, is not in cache yet updater = self._run_refresh() - file_info = updater.get_one_valid_targetinfo("file") - self.assertIsNotNone(file_info) - self.assertEqual( - updater.updated_targets([file_info], self.targets_dir), [file_info] - ) - - # download target, assert it is in cache and content is correct - updater.download_target(file_info, self.targets_dir) - self.assertEqual( - updater.updated_targets([file_info], self.targets_dir), [] - ) - with open(os.path.join(self.targets_dir, "file"), "rb") as f: - self.assertEqual(f.read(), b"content") - - # TODO: run the same download tests for - # self.sim.add_target("targets", b"more content", "dir/file2") + for targetpath, content in targets.items(): + # target now exists, is not in cache yet + file_info = updater.get_one_valid_targetinfo(targetpath) + self.assertIsNotNone(file_info) + self.assertEqual( + updater.updated_targets([file_info], self.targets_dir), + [file_info] + ) + + # download target, assert it is in cache and content is correct + local_path = updater.download_target(file_info, self.targets_dir) + self.assertEqual( + updater.updated_targets([file_info], self.targets_dir), [] + ) + self.assertTrue(local_path.startswith(self.targets_dir)) + with open(local_path, "rb") as f: + self.assertEqual(f.read(), content) + + # TODO: run the same download tests for target paths like "dir/file2") # This currently fails because issue #1576 def test_keys_and_signatures(self): diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index c616b7cd6d..3e8667bada 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -188,31 +188,27 @@ def updated_targets( returned in a list. The list items can be downloaded with 'download_target()'. """ - # Keep track of the target objects and filepaths of updated targets. - # Return 'updated_targets' and use 'updated_targetpaths' to avoid - # duplicates. - updated_targets = [] - updated_targetpaths = [] + # Keep track of TargetFiles and local paths. Return 'updated_targets' + # and use 'local_paths' to avoid duplicates. + updated_targets: List[TargetFile] = [] + local_paths: List[str] = [] for target in targets: - # Prepend 'destination_directory' to the target's relative filepath - # (as stored in metadata.) Verify the hash of 'target_filepath' - # against each hash listed for its fileinfo. Note: join() discards - # 'destination_directory' if 'filepath' contains a leading path - # separator (i.e., is treated as an absolute path). - target_filepath = os.path.join(destination_directory, target.path) - - if target_filepath in updated_targetpaths: + # URL encode to get local filename like download_target() does + filename = parse.quote(target.path, "") + local_path = os.path.join(destination_directory, filename) + + if local_path in local_paths: continue try: - with open(target_filepath, "rb") as target_file: + with open(local_path, "rb") as target_file: target.verify_length_and_hashes(target_file) # If the file does not exist locally or length and hashes # do not match, append to updated targets. except (OSError, exceptions.LengthOrHashMismatchError): updated_targets.append(target) - updated_targetpaths.append(target_filepath) + local_paths.append(local_path) return updated_targets @@ -221,7 +217,7 @@ def download_target( targetinfo: TargetFile, destination_directory: str, target_base_url: Optional[str] = None, - ) -> None: + ) -> str: """Downloads the target file specified by 'targetinfo'. Args: @@ -236,6 +232,9 @@ def download_target( Raises: TODO: download-related errors TODO: file write errors + + Returns: + Path to downloaded file """ if target_base_url is None: @@ -266,12 +265,13 @@ def download_target( f"{target_filepath} length or hashes do not match" ) from e - # Store the target file name without the HASH prefix. - local_filepath = os.path.join( - destination_directory, targetinfo.path - ) + # Use a URL encoded targetpath as the local filename + filename = parse.quote(targetinfo.path, "") + local_filepath = os.path.join(destination_directory, filename) sslib_util.persist_temp_file(target_file, local_filepath) + return local_filepath + def _download_metadata( self, rolename: str, length: int, version: Optional[int] = None ) -> bytes: