From aa6d28fbc3a50872d51c68810de3d35892d171aa Mon Sep 17 00:00:00 2001 From: Kairo de Araujo Date: Thu, 6 Jan 2022 09:10:27 +0100 Subject: [PATCH] explicit encode role names This commit explicitly encodes role names. Mostly this encoding is already happening in ``requests`` for what is not a URL. The "/" in a role name will now be encoded. Also, a slight change in the RepositorySimulator will align with the tests. This commit partially covers issue #1634 Signed-off-by: Kairo de Araujo --- tests/repository_simulator.py | 2 ++ tests/test_updater_delegation_graphs.py | 21 +++++++++++++++------ tuf/ngclient/updater.py | 5 +++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/repository_simulator.py b/tests/repository_simulator.py index 39535ef26b..c8ddf8b13c 100644 --- a/tests/repository_simulator.py +++ b/tests/repository_simulator.py @@ -259,6 +259,8 @@ def fetch_metadata(self, role: str, version: Optional[int] = None) -> bytes: If version is None, non-versioned metadata is being requested. """ self.fetch_tracker.metadata.append((role, version)) + # decode role for the metadata + role = parse.unquote(role, encoding="utf-8") if role == Root.type: # return a version previously serialized in publish_root() diff --git a/tests/test_updater_delegation_graphs.py b/tests/test_updater_delegation_graphs.py index a5ef66291a..6e2a131798 100644 --- a/tests/test_updater_delegation_graphs.py +++ b/tests/test_updater_delegation_graphs.py @@ -328,9 +328,10 @@ def test_invalid_metadata(self, test_data: DelegationsTestCase) -> None: finally: self.teardown_subtest() - def test_fishy_rolenames(self) -> None: - # Test that delegated roles filenames are safely encoded - # when persisted to the file system. + def test_safely_encoded_rolenames(self) -> None: + """Test that delegated roles names are safely encoded in the filenames + and URLs. + """ roles_to_filenames = { "../a": "..%2Fa.json", @@ -343,16 +344,24 @@ def test_fishy_rolenames(self) -> None: for rolename in roles_to_filenames: delegations.append(TestDelegation("targets", rolename)) - fishy_rolenames = DelegationsTestCase(delegations) - self._init_repo(fishy_rolenames) + delegated_rolenames = DelegationsTestCase(delegations) + self._init_repo(delegated_rolenames) updater = self._init_updater() + updater.refresh() - # trigger updater to fetch the delegated metadata, check filenames + # trigger updater to fetch the delegated metadata + self.sim.fetch_tracker.metadata.clear() updater.get_targetinfo("anything") + + # assert that local delegated metadata filenames are expected local_metadata = os.listdir(self.metadata_dir) for fname in roles_to_filenames.values(): self.assertTrue(fname in local_metadata) + # assert that requested URLs are quoted without extension + exp_calls = [(quoted[:-5], 1) for quoted in roles_to_filenames.values()] + self.assertListEqual(self.sim.fetch_tracker.metadata, exp_calls) + class TestTargetFileSearch(TestDelegations): r""" diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index f4c5b24249..bd940425ed 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -290,10 +290,11 @@ def _download_metadata( self, rolename: str, length: int, version: Optional[int] = None ) -> bytes: """Download a metadata file and return it as bytes""" + encoded_name = parse.quote(rolename, "") if version is None: - url = f"{self._metadata_base_url}{rolename}.json" + url = f"{self._metadata_base_url}{encoded_name}.json" else: - url = f"{self._metadata_base_url}{version}.{rolename}.json" + url = f"{self._metadata_base_url}{version}.{encoded_name}.json" return self._fetcher.download_bytes(url, length) def _load_local_metadata(self, rolename: str) -> bytes: