From 7916c227f7d2a6dffa334a26f34f65f5d4ae9e48 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Wed, 10 May 2023 14:27:20 +0200
Subject: [PATCH] http/fetch: improve error messages

- Return a more specific error for an empty digest, instead of a parsing
  error.
- Include the digest in the error message when parsing fails.
- Tidy the format of some other errors.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 http/fetch/archive_fetcher.go      | 16 ++++++++++------
 http/fetch/archive_fetcher_test.go |  8 ++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/http/fetch/archive_fetcher.go b/http/fetch/archive_fetcher.go
index 8b891c3b..584c0269 100644
--- a/http/fetch/archive_fetcher.go
+++ b/http/fetch/archive_fetcher.go
@@ -84,7 +84,7 @@ func (r *ArchiveFetcher) Fetch(archiveURL, digest, dir string) error {
 
 	resp, err := r.httpClient.Do(req)
 	if err != nil {
-		return fmt.Errorf("failed to download archive, error: %w", err)
+		return fmt.Errorf("failed to download archive: %w", err)
 	}
 	defer resp.Body.Close()
 
@@ -92,7 +92,7 @@ func (r *ArchiveFetcher) Fetch(archiveURL, digest, dir string) error {
 		if code == http.StatusNotFound {
 			return FileNotFoundError
 		}
-		return fmt.Errorf("failed to download archive from %s, status: %s", archiveURL, resp.Status)
+		return fmt.Errorf("failed to download archive from %s (status: %s)", archiveURL, resp.Status)
 	}
 
 	f, err := os.CreateTemp("", "fetch.*.tmp")
@@ -130,7 +130,7 @@ func (r *ArchiveFetcher) Fetch(archiveURL, digest, dir string) error {
 	// Ensure that the digest of the downloaded file matches the
 	// known digest.
 	if err := r.verifyDigest(digest, f); err != nil {
-		return err
+		return fmt.Errorf("failed to verify archive: %w", err)
 	}
 
 	// Jump back at the beginning of the file stream again.
@@ -148,15 +148,19 @@ func (r *ArchiveFetcher) Fetch(archiveURL, digest, dir string) error {
 }
 
 // verifyDigest verifies the digest of the reader, and returns an error if it
-// doesn't match.
+// doesn't match, fails to parse, or is empty.
 func (r *ArchiveFetcher) verifyDigest(dig string, reader io.Reader) error {
+	if dig == "" {
+		return fmt.Errorf("empty digest")
+	}
+
 	if !strings.Contains(dig, ":") {
 		dig = "sha256:" + dig
 	}
 
 	d, err := digest.Parse(dig)
 	if err != nil {
-		return fmt.Errorf("failed to parse digest: %w", err)
+		return fmt.Errorf("failed to parse digest '%s': %w", dig, err)
 	}
 
 	// Verify reader's data.
@@ -165,7 +169,7 @@ func (r *ArchiveFetcher) verifyDigest(dig string, reader io.Reader) error {
 		return err
 	}
 	if !verifier.Verified() {
-		return fmt.Errorf("failed to verify archive: computed digest doesn't match provided '%s' (check whether file size exceeds max download size)", dig)
+		return fmt.Errorf("computed digest doesn't match provided '%s' (check whether file size exceeds max download size)", dig)
 	}
 	return nil
 }
diff --git a/http/fetch/archive_fetcher_test.go b/http/fetch/archive_fetcher_test.go
index cbc112c8..af7ad77c 100644
--- a/http/fetch/archive_fetcher_test.go
+++ b/http/fetch/archive_fetcher_test.go
@@ -85,6 +85,14 @@ func TestArchiveFetcher_Fetch(t *testing.T) {
 			maxUntarSize:    1,
 			wantErr:         true,
 		},
+		{
+			name:            "fails with empty digest error",
+			url:             artifactURL,
+			digest:          "",
+			maxDownloadSize: -1,
+			maxUntarSize:    -1,
+			wantErr:         true,
+		},
 		{
 			name:            "fails with digest parsing error",
 			url:             artifactURL,