From 52b58ae3330597551d57455765189f5706b5a1c3 Mon Sep 17 00:00:00 2001 From: 10zingpd Date: Wed, 12 Nov 2025 14:15:43 -0500 Subject: [PATCH 1/6] respect etags in resumable downloads --- utils/utils.go | 85 +++++++++++++++++++++++++++++++++++++++-- utils/utils_test.go | 2 +- version_control_test.go | 4 +- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index 1bef125..80be869 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -190,13 +190,44 @@ func DownloadFile(ctx context.Context, rawURL string, logger logging.Logger) (st } case "http", "https": // note: we shrink the hash to avoid system path length limits - partialDest := CreatePartialPath(rawURL) + partialDest, etagPath := CreatePartialPath(rawURL) + + // Do a HEAD request to get the ETag value + remoteETag, err := getRemoteETag(ctx, parsedURL.String(), logger) + if err != nil { + logger.Warnw("failed to get remote ETag, proceeding with download", "err", err) + } g := getter.HttpGetter{Client: socksClient(parsedURL.String(), logger)} g.SetClient(getterClient) if stat, err := os.Stat(partialDest); err == nil { - logger.Infow("download to existing", "dest", partialDest, "size", stat.Size()) + // File exists, load the etag file and check it against the one we just got + storedETag, err := readETag(etagPath) + if err == nil && remoteETag != "" { + if storedETag == remoteETag { + // ETag matches - allow resume (getter will handle range requests) + logger.Infow("resuming download with matching ETag", "dest", partialDest, "size", stat.Size(), "etag", remoteETag) + } else { + // If it's a mismatch, delete the old .part file and save the new .etag file + logger.Infow("ETag mismatch, deleting old file", "dest", partialDest, "stored_etag", storedETag, "remote_etag", remoteETag) + if err := os.Remove(partialDest); err != nil && !errors.Is(err, fs.ErrNotExist) { + logger.Warnw("failed to remove old partial file", "err", err) + } + if err := writeETag(etagPath, remoteETag); err != nil { + logger.Warnw("failed to save ETag", "err", err) + } + } + } else { + logger.Infow("download to existing", "dest", partialDest, "size", stat.Size()) + } + } else { + // If the file doesn't exist, save a new etag file + if remoteETag != "" { + if err := writeETag(etagPath, remoteETag); err != nil { + logger.Warnw("failed to save ETag", "err", err) + } + } } done := make(chan struct{}) @@ -258,7 +289,8 @@ func rewriteGCPDownload(orig *url.URL) (*url.URL, bool) { } // CreatePartialPath makes a path under cachedir/part. These get cleaned up by CleanPartials. -func CreatePartialPath(rawURL string) string { +// Returns both the .part path and the .etag path. +func CreatePartialPath(rawURL string) (partPath, etagPath string) { var urlPath string if parsed, err := url.Parse(rawURL); err != nil { urlPath = "UNPARSED" @@ -266,7 +298,9 @@ func CreatePartialPath(rawURL string) string { urlPath = parsed.Path } - return path.Join(ViamDirs.Partials, hashString(rawURL, 7), last(strings.Split(urlPath, "/"), "")+".part") + partPath = path.Join(ViamDirs.Partials, hashString(rawURL, 7), last(strings.Split(urlPath, "/"), "")+".part") + etagPath = partPath + ".etag" + return partPath, etagPath } // helper: return last item of `items` slice, or `default_` if items is empty. @@ -288,6 +322,49 @@ func hashString(input string, n int) string { return ret } +// getRemoteETag performs a HEAD request to get the ETag from the remote server. +// ETags are returned with quotes removed for consistent comparison. +func getRemoteETag(ctx context.Context, url string, logger logging.Logger) (string, error) { + etag, _, err := getRemoteETagAndSize(ctx, url, logger) + return etag, err +} + +// getRemoteETagAndSize performs a HEAD request to get the ETag and content length from the remote server. +// ETags are returned with quotes removed for consistent comparison. +func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger) (string, int64, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) + if err != nil { + return "", 0, errw.Wrap(err, "creating HEAD request") + } + res, err := socksClient(url, logger).Do(req) + if err != nil { + return "", 0, errw.Wrap(err, "executing HEAD request") + } + defer res.Body.Close() //nolint:errcheck,gosec + etag := res.Header.Get("ETag") + // Remove surrounding quotes if present (ETags are often returned as "value") + etag = strings.Trim(etag, `"`) + return etag, res.ContentLength, nil +} + +// readETag reads the ETag from a file. +func readETag(etagPath string) (string, error) { + data, err := os.ReadFile(etagPath) + if err != nil { + return "", err + } + return strings.TrimSpace(string(data)), nil +} + +// writeETag writes the ETag to a file. +func writeETag(etagPath string, etag string) error { + // Ensure the directory exists + if err := os.MkdirAll(path.Dir(etagPath), 0o755); err != nil { + return errw.Wrapf(err, "creating directory for %s", etagPath) + } + return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o644), "writing ETag to %s", etagPath) +} + // on windows only, create a firewall exception for the newly-downloaded file. func allowFirewall(logger logging.Logger, outPath string) error { // todo: confirm this is right; this isn't the final destination. Does the rule move when the file is renamed? Link to docs. diff --git a/utils/utils_test.go b/utils/utils_test.go index f2f1221..f707a91 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -386,7 +386,7 @@ func TestInitPaths(t *testing.T) { } func TestPartialPath(t *testing.T) { - path := CreatePartialPath("https://storage.googleapis.com/packages.viam.com/apps/viam-server/viam-server-latest-x86_64") + path, _ := CreatePartialPath("https://storage.googleapis.com/packages.viam.com/apps/viam-server/viam-server-latest-x86_64") maxPathLengths := map[string]int{ "linux": 4096, "windows": 260, diff --git a/version_control_test.go b/version_control_test.go index 0585cc7..51c3962 100644 --- a/version_control_test.go +++ b/version_control_test.go @@ -206,7 +206,7 @@ func TestCleanPartials(t *testing.T) { vc := VersionCache{logger: logging.NewTestLogger(t)} // make a part file to clean up - oldPath := utils.CreatePartialPath("https://viam.com/old.part") + oldPath, _ := utils.CreatePartialPath("https://viam.com/old.part") err := os.Mkdir(filepath.Dir(oldPath), 0o755) test.That(t, err, test.ShouldBeNil) err = os.WriteFile(oldPath, []byte("hello"), 0o600) @@ -214,7 +214,7 @@ func TestCleanPartials(t *testing.T) { os.Chtimes(oldPath, time.Now(), time.Now().Add(-time.Hour*24*4)) // make another one too new to clean up - newPath := utils.CreatePartialPath("https://viam.com/subpath/new.part") + newPath, _ := utils.CreatePartialPath("https://viam.com/subpath/new.part") err = os.Mkdir(filepath.Dir(newPath), 0o755) test.That(t, err, test.ShouldBeNil) err = os.WriteFile(newPath, []byte("hello"), 0o600) From c9481a3720ade82c2629a83e1dba69748229100e Mon Sep 17 00:00:00 2001 From: 10zingpd Date: Tue, 18 Nov 2025 15:27:34 -0500 Subject: [PATCH 2/6] Add ETag support for resumable downloads - Modified CreatePartialPath to return both .part and .etag paths - Added HEAD request to check remote ETag before downloading - Store ETag when starting downloads and check it when resuming - Delete partial file and start fresh if ETag mismatch detected - Updated tests to verify both partial and ETag path lengths --- utils/utils.go | 5 +++-- utils/utils_test.go | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index 80be869..b20367d 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -298,8 +298,9 @@ func CreatePartialPath(rawURL string) (partPath, etagPath string) { urlPath = parsed.Path } - partPath = path.Join(ViamDirs.Partials, hashString(rawURL, 7), last(strings.Split(urlPath, "/"), "")+".part") - etagPath = partPath + ".etag" + basePath := path.Join(ViamDirs.Partials, hashString(rawURL, 7), last(strings.Split(urlPath, "/"), "")) + partPath = basePath + ".part" + etagPath = basePath + ".etag" return partPath, etagPath } diff --git a/utils/utils_test.go b/utils/utils_test.go index f707a91..49979cf 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -386,13 +386,14 @@ func TestInitPaths(t *testing.T) { } func TestPartialPath(t *testing.T) { - path, _ := CreatePartialPath("https://storage.googleapis.com/packages.viam.com/apps/viam-server/viam-server-latest-x86_64") + partPath, etagPath := CreatePartialPath("https://storage.googleapis.com/packages.viam.com/apps/viam-server/viam-server-latest-x86_64") maxPathLengths := map[string]int{ "linux": 4096, "windows": 260, } for _, maxPath := range maxPathLengths { - test.That(t, len(path), test.ShouldBeLessThanOrEqualTo, maxPath) + test.That(t, len(partPath), test.ShouldBeLessThanOrEqualTo, maxPath) + test.That(t, len(etagPath), test.ShouldBeLessThanOrEqualTo, maxPath) } } From 891d0349de0cb33cdbf4fe08a70e55801bbd1a8b Mon Sep 17 00:00:00 2001 From: 10zingpd Date: Tue, 18 Nov 2025 15:50:46 -0500 Subject: [PATCH 3/6] Fix lint errors: permissions, nolint directives, and else-if syntax --- utils/utils.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index b20367d..a7d9107 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -221,12 +221,10 @@ func DownloadFile(ctx context.Context, rawURL string, logger logging.Logger) (st } else { logger.Infow("download to existing", "dest", partialDest, "size", stat.Size()) } - } else { + } else if remoteETag != "" { // If the file doesn't exist, save a new etag file - if remoteETag != "" { - if err := writeETag(etagPath, remoteETag); err != nil { - logger.Warnw("failed to save ETag", "err", err) - } + if err := writeETag(etagPath, remoteETag); err != nil { + logger.Warnw("failed to save ETag", "err", err) } } @@ -341,7 +339,7 @@ func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger if err != nil { return "", 0, errw.Wrap(err, "executing HEAD request") } - defer res.Body.Close() //nolint:errcheck,gosec + defer res.Body.Close() //nolint:errcheck etag := res.Header.Get("ETag") // Remove surrounding quotes if present (ETags are often returned as "value") etag = strings.Trim(etag, `"`) @@ -350,6 +348,7 @@ func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger // readETag reads the ETag from a file. func readETag(etagPath string) (string, error) { + //nolint:gosec data, err := os.ReadFile(etagPath) if err != nil { return "", err @@ -360,10 +359,12 @@ func readETag(etagPath string) (string, error) { // writeETag writes the ETag to a file. func writeETag(etagPath string, etag string) error { // Ensure the directory exists - if err := os.MkdirAll(path.Dir(etagPath), 0o755); err != nil { + //nolint:gosec + if err := os.MkdirAll(path.Dir(etagPath), 0o750); err != nil { return errw.Wrapf(err, "creating directory for %s", etagPath) } - return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o644), "writing ETag to %s", etagPath) + //nolint:gosec + return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o600), "writing ETag to %s", etagPath) } // on windows only, create a firewall exception for the newly-downloaded file. From 0ed0412256ffedd67acc9e4ebbefea6fbf2d8df5 Mon Sep 17 00:00:00 2001 From: 10zingpd Date: Tue, 18 Nov 2025 15:56:24 -0500 Subject: [PATCH 4/6] Revert "Fix lint errors: permissions, nolint directives, and else-if syntax" This reverts commit 891d0349de0cb33cdbf4fe08a70e55801bbd1a8b. --- utils/utils.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index a7d9107..b20367d 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -221,10 +221,12 @@ func DownloadFile(ctx context.Context, rawURL string, logger logging.Logger) (st } else { logger.Infow("download to existing", "dest", partialDest, "size", stat.Size()) } - } else if remoteETag != "" { + } else { // If the file doesn't exist, save a new etag file - if err := writeETag(etagPath, remoteETag); err != nil { - logger.Warnw("failed to save ETag", "err", err) + if remoteETag != "" { + if err := writeETag(etagPath, remoteETag); err != nil { + logger.Warnw("failed to save ETag", "err", err) + } } } @@ -339,7 +341,7 @@ func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger if err != nil { return "", 0, errw.Wrap(err, "executing HEAD request") } - defer res.Body.Close() //nolint:errcheck + defer res.Body.Close() //nolint:errcheck,gosec etag := res.Header.Get("ETag") // Remove surrounding quotes if present (ETags are often returned as "value") etag = strings.Trim(etag, `"`) @@ -348,7 +350,6 @@ func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger // readETag reads the ETag from a file. func readETag(etagPath string) (string, error) { - //nolint:gosec data, err := os.ReadFile(etagPath) if err != nil { return "", err @@ -359,12 +360,10 @@ func readETag(etagPath string) (string, error) { // writeETag writes the ETag to a file. func writeETag(etagPath string, etag string) error { // Ensure the directory exists - //nolint:gosec - if err := os.MkdirAll(path.Dir(etagPath), 0o750); err != nil { + if err := os.MkdirAll(path.Dir(etagPath), 0o755); err != nil { return errw.Wrapf(err, "creating directory for %s", etagPath) } - //nolint:gosec - return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o600), "writing ETag to %s", etagPath) + return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o644), "writing ETag to %s", etagPath) } // on windows only, create a firewall exception for the newly-downloaded file. From 91331cb3313f67a0c532a683cfbe545072153ec8 Mon Sep 17 00:00:00 2001 From: 10zingpd Date: Tue, 25 Nov 2025 13:15:39 -0500 Subject: [PATCH 5/6] Fix lint errors: permissions, nolint directives, and else-if syntax --- utils/utils.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index b20367d..e7dbee3 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -221,12 +221,10 @@ func DownloadFile(ctx context.Context, rawURL string, logger logging.Logger) (st } else { logger.Infow("download to existing", "dest", partialDest, "size", stat.Size()) } - } else { + } else if remoteETag != "" { // If the file doesn't exist, save a new etag file - if remoteETag != "" { - if err := writeETag(etagPath, remoteETag); err != nil { - logger.Warnw("failed to save ETag", "err", err) - } + if err := writeETag(etagPath, remoteETag); err != nil { + logger.Warnw("failed to save ETag", "err", err) } } @@ -341,7 +339,7 @@ func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger if err != nil { return "", 0, errw.Wrap(err, "executing HEAD request") } - defer res.Body.Close() //nolint:errcheck,gosec + defer res.Body.Close() //nolint:errcheck etag := res.Header.Get("ETag") // Remove surrounding quotes if present (ETags are often returned as "value") etag = strings.Trim(etag, `"`) @@ -350,7 +348,7 @@ func getRemoteETagAndSize(ctx context.Context, url string, logger logging.Logger // readETag reads the ETag from a file. func readETag(etagPath string) (string, error) { - data, err := os.ReadFile(etagPath) + data, err := os.ReadFile(etagPath) //nolint:gosec if err != nil { return "", err } @@ -360,10 +358,10 @@ func readETag(etagPath string) (string, error) { // writeETag writes the ETag to a file. func writeETag(etagPath string, etag string) error { // Ensure the directory exists - if err := os.MkdirAll(path.Dir(etagPath), 0o755); err != nil { + if err := os.MkdirAll(path.Dir(etagPath), 0o750); err != nil { return errw.Wrapf(err, "creating directory for %s", etagPath) } - return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o644), "writing ETag to %s", etagPath) + return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o600), "writing ETag to %s", etagPath) } // on windows only, create a firewall exception for the newly-downloaded file. From 071b96191b367fc1fba05c061a3a52f1b595d038 Mon Sep 17 00:00:00 2001 From: 10zingpd Date: Tue, 25 Nov 2025 13:21:26 -0500 Subject: [PATCH 6/6] Apply linter fix: combine string parameters in writeETag function --- utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/utils.go b/utils/utils.go index e7dbee3..53d3f6c 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -356,7 +356,7 @@ func readETag(etagPath string) (string, error) { } // writeETag writes the ETag to a file. -func writeETag(etagPath string, etag string) error { +func writeETag(etagPath, etag string) error { // Ensure the directory exists if err := os.MkdirAll(path.Dir(etagPath), 0o750); err != nil { return errw.Wrapf(err, "creating directory for %s", etagPath)