Merge pull request #113 from jku/add-download-test-with-subdir

Add download test using targetpath with subdirs

Author: jku

clients/go-tuf/cmd/download.go

@@ -13,6 +13,7 @@ package cmd
 
 import (
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 
@@ -103,7 +104,8 @@ func RefreshAndDownloadCmd(targetName string,
 	}
 
 	// target is available, so let's see if the target is already present locally
-	path, _, err := up.FindCachedTarget(targetInfo, "")
+	localPath := filepath.Join(targetDownloadDir, url.QueryEscape(targetName))
+	path, _, err := up.FindCachedTarget(targetInfo, localPath)
 	if err != nil {
 		return fmt.Errorf("failed while finding a cached target: %w", err)
 	}
@@ -114,7 +116,8 @@ func RefreshAndDownloadCmd(targetName string,
 	}
 
 	// target is not present locally, so let's try to download it
-	path, _, err = up.DownloadTarget(targetInfo, filepath.Join(targetDownloadDir, targetName), targetBaseUrl)
+	//
+	path, _, err = up.DownloadTarget(targetInfo, localPath, targetBaseUrl)
 	if err != nil {
 		return fmt.Errorf("failed to download target file %s - %w", targetName, err)
 	}

tuf_conformance/client_runner.py

@@ -23,19 +23,24 @@ def __init__(
         self._server = server
         self._cmd = client_cmd.split(" ")
         self._tempdir = TemporaryDirectory()
-        self._target_dir = TemporaryDirectory()
-        self._remote_target_dir = TemporaryDirectory(dir=os.getcwd())
         # TODO: cleanup tempdir
         self.metadata_dir = os.path.join(self._tempdir.name, "metadata")
+        self.artifact_dir = os.path.join(self._tempdir.name, "targets")
         os.mkdir(self.metadata_dir)
+        os.mkdir(self.artifact_dir)
         self.test_name = test_name
 
-    def get_last_downloaded_target(self) -> str:
-        list_of_files = glob.glob(self._target_dir.name + "/*")
-        if len(list_of_files) == 0:
-            return ""
-        latest_file = max(list_of_files, key=os.path.getctime)
-        return latest_file
+    def get_downloaded_target_bytes(self) -> list[bytes]:
+        """Returns list of downloaded artifact contents in order of modification time"""
+        artifacts = glob.glob(f"{self.artifact_dir}/**", recursive=True)
+        artifact_bytes = []
+        for artifact in sorted(artifacts, key=os.path.getmtime):
+            if not os.path.isfile(artifact):
+                continue
+            with open(artifact, "rb") as f:
+                artifact_bytes.append(f.read())
+
+        return artifact_bytes
 
     def _run(self, cmd: list[str]) -> int:
         popen = subprocess.Popen(cmd)
@@ -77,7 +82,7 @@ def download_target(self, data: ClientInitData, target_name: str) -> int:
             "--target-name",
             target_name,
             "--target-dir",
-            self._target_dir.name,
+            self.artifact_dir,
             "--target-base-url",
             data.targets_url,
             "download",

tuf_conformance/test_file_download.py

@@ -20,15 +20,40 @@ def test_client_downloads_expected_file(
     target_content = b"target file contents"
     repo.add_target(Targets.type, target_content, target_path)
 
-    # Client updates, sanity update that nothing was downloaded
+    # Client updates, sanity check that nothing was downloaded
     assert client.refresh(init_data) == 0
-    assert client.get_last_downloaded_target() == ""
+    assert client.get_downloaded_target_bytes() == []
 
     assert client.download_target(init_data, target_path) == 0
 
     # assert that the downloaded file contents are correct
-    with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-        assert last_download_file.read() == target_content
+    assert client.get_downloaded_target_bytes() == [target_content]
+
+
+def test_client_downloads_expected_file_in_sub_dir(
+    client: ClientRunner, server: SimulatorServer
+) -> None:
+    """This test adds a file to the repository targets metadata with a targetpath that
+    contains a directory. Client then refreshes, and downloads the target. Finally,
+    the test asserts that the client downloaded the file and that the contents of the
+    file are correct
+    """
+    init_data, repo = server.new_test(client.test_name)
+    assert client.init_client(init_data) == 0
+
+    # Create a test artifact, add it to the repository
+    target_path = "path/to/a/target_file.txt"
+    target_content = b"target file contents"
+    repo.add_target(Targets.type, target_content, target_path)
+
+    # Client updates, sanity check that nothing was downloaded
+    assert client.refresh(init_data) == 0
+    assert client.get_downloaded_target_bytes() == []
+
+    assert client.download_target(init_data, target_path) == 0
+
+    # assert that the downloaded file contents are correct
+    assert client.get_downloaded_target_bytes() == [target_content]
 
 
 def test_repository_substitutes_target_file(
@@ -54,8 +79,7 @@ def test_repository_substitutes_target_file(
 
     # Download one of the artifacts
     assert client.download_target(init_data, target_path_1) == 0
-    with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-        assert last_download_file.read() == target_content_1
+    assert client.get_downloaded_target_bytes() == [target_content_1]
 
     # Change both artifact contents that repository serves
     malicious_content = b"malicious data - should not download"
@@ -68,15 +92,13 @@ def test_repository_substitutes_target_file(
     client.download_target(init_data, target_path_1)
 
     # assert the client did not accept the malicious artifact
-    with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-        assert last_download_file.read() == target_content_1
+    assert client.get_downloaded_target_bytes() == [target_content_1]
 
     # ask client to download the other artifact and expect failure
-    assert client.download_target(init_data, target_path_1) == 1
+    assert client.download_target(init_data, target_path_2) == 1
 
     # assert the client did not store the malicious artifact
-    with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-        assert last_download_file.read() == target_content_1
+    assert client.get_downloaded_target_bytes() == [target_content_1]
 
 
 def test_multiple_changes_to_target(
@@ -109,18 +131,22 @@ def test_multiple_changes_to_target(
     # Client downloads the file
     client.download_target(init_data, target_path)
     # check file contents
-    with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-        assert last_download_file.read() == target_content
+    expected_downloads = [target_content]
+    previous_content = target_content
+    assert client.get_downloaded_target_bytes() == expected_downloads
 
     # Now the repository makes 10 changes to the targets metadata.
     # It modifies the targets file in the targets metadata including
     # the hashes and length, and it also makes the corresponding
-    # content changes in the file itself. As such, the client
-    # should download the file
+    # content changes in the file itself.
     for i in range(10):
-        # Modify the artifact legitimately:
-        new_file_contents = f"target file contents {i}".encode()
-        repo.add_target(Targets.type, new_file_contents, target_path)
+        # Modify the existing artifact legitimately:
+        modified_contents = f"modified file contents {i}".encode()
+        repo.add_target(Targets.type, modified_contents, target_path)
+        # Add a completely new artifact
+        new_file_contents = f"new file contents {i}".encode()
+        new_target_path = f"new-target-{i}"
+        repo.add_target(Targets.type, new_file_contents, new_target_path)
         repo.targets.version += 1
 
         # Bump repo snapshot
@@ -129,13 +155,22 @@ def test_multiple_changes_to_target(
         # Client updates
         assert client.refresh(init_data) == 0
 
-        # Client downloads the new file
+        # Client downloads the modified artifact
         assert client.download_target(init_data, target_path) == 0
-        # check file contents
-        with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-            assert last_download_file.read() == new_file_contents
+        # the previous content is no longer there, modified content is there
+        expected_downloads.remove(previous_content)
+        expected_downloads.append(modified_contents)
+        previous_content = modified_contents
+        assert client.get_downloaded_target_bytes() == expected_downloads
+
+        # Client downloads the new artifact
+        assert client.download_target(init_data, new_target_path) == 0
+        expected_downloads.append(new_file_contents)
+
+        # check downloaded contents
+        assert client.get_downloaded_target_bytes() == expected_downloads
 
-        # Modify the artifact content without updating the hashes/length in metadata.
+        # Modify the original artifact content without updating the hashes/length in metadata.
         malicious_file_contents = f"malicious contents {i}".encode()
         repo.artifacts[target_path].data = malicious_file_contents
         repo.targets.version += 1
@@ -147,5 +182,4 @@ def test_multiple_changes_to_target(
         client.download_target(init_data, target_path)
 
         # Check that the client did not download the malicious file
-        with open(client.get_last_downloaded_target(), "rb") as last_download_file:
-            assert new_file_contents == last_download_file.read()
+        assert client.get_downloaded_target_bytes() == expected_downloads