Skip to content

Commit dd2f935

Browse files
authored
batches: Make sure no corrupted repozips are used (#817)
When the download of a zip was aborted, it could've left behind a corrupted archive zip. This fixes it by first storing it to a temporary location and then creating the actual file atomically by doing os.Rename.
1 parent 9c8e8bc commit dd2f935

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

internal/batches/repozip/fetcher.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,15 +336,27 @@ func fetchRepositoryFile(ctx context.Context, client HTTPClient, repo RepoRevisi
336336
return false, fmt.Errorf("unable to fetch archive (HTTP %d from %s)", resp.StatusCode, req.URL.String())
337337
}
338338

339-
f, err := os.Create(dest)
339+
f, err := os.CreateTemp(filepath.Dir(dest), fmt.Sprintf("%s-*.tmp", filepath.Base(dest)))
340340
if err != nil {
341341
return false, err
342342
}
343-
defer f.Close()
343+
// Make sure we clean up the temp file in case something fails.
344+
defer func(path string) { _ = os.Remove(path) }(f.Name())
344345

345346
if _, err := io.Copy(f, resp.Body); err != nil {
347+
// Be a good citizen, attempt to close the file.
348+
_ = f.Close()
346349
return false, err
347350
}
351+
if err := f.Close(); err != nil {
352+
return false, errors.Wrap(err, "closing temp file")
353+
}
354+
355+
// Atomically create the actual file, so that there are no artifacts left behind
356+
// when this process is aborted, network errors occur, or some witchcraft goes on.
357+
if err := os.Rename(f.Name(), dest); err != nil {
358+
return false, errors.Wrap(err, "renaming temp file")
359+
}
348360

349361
return true, nil
350362
}

0 commit comments

Comments
 (0)