Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 81 additions & 4 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,44 @@
}
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 {

Check failure on line 224 in utils/utils.go

View workflow job for this annotation

GitHub Actions / Lint, Test, Build

elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic)
// 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{})
Expand Down Expand Up @@ -258,15 +289,18 @@
}

// 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"
} else {
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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here, can you do something like

basePath := path.Join(whatever)
partPath = basePath + ".part"
etagPath = basePath + ".etag"

(so that the etag isn't .part.etag)

return partPath, etagPath
}

// helper: return last item of `items` slice, or `default_` if items is empty.
Expand All @@ -288,6 +322,49 @@
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

Check failure on line 343 in utils/utils.go

View workflow job for this annotation

GitHub Actions / Lint, Test, Build

directive `//nolint:errcheck,gosec` is unused for linter "gosec" (nolintlint)
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)

Check failure on line 352 in utils/utils.go

View workflow job for this annotation

GitHub Actions / Lint, Test, Build

G304: Potential file inclusion via variable (gosec)
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 {

Check failure on line 362 in utils/utils.go

View workflow job for this annotation

GitHub Actions / Lint, Test, Build

G301: Expect directory permissions to be 0750 or less (gosec)
return errw.Wrapf(err, "creating directory for %s", etagPath)
}
return errw.Wrapf(os.WriteFile(etagPath, []byte(etag), 0o644), "writing ETag to %s", etagPath)

Check failure on line 365 in utils/utils.go

View workflow job for this annotation

GitHub Actions / Lint, Test, Build

G306: Expect WriteFile permissions to be 0600 or less (gosec)
}

// 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.
Expand Down
2 changes: 1 addition & 1 deletion utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on L395 below, can you add another line testing the etag path as well as the partial path?

maxPathLengths := map[string]int{
"linux": 4096,
"windows": 260,
Expand Down
4 changes: 2 additions & 2 deletions version_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,15 @@ 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)
test.That(t, err, test.ShouldBeNil)
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)
Expand Down
Loading