Skip to content

Commit 48ce8f6

Browse files
committed
fix(software): retry truncated/empty downloads and report real checksum algorithm
Software downloads failed the whole workflow on a transient CDN/network hiccup: a 200-with-empty-body, a truncated copy, or a mid-stream reset wrote a corrupt file that the checksum verifier correctly rejected, but with no retry the entire run aborted. - Download now rejects zero-byte bodies and, when Content-Length is advertised, fails on a byte-count mismatch instead of persisting a truncated file. - Add Downloader.DownloadAndVerify, which retries download+verify with capped exponential backoff (default 3 attempts), deleting the bad file between attempts; both download errors and checksum mismatches are retryable. The three base_installer call sites now use it. - Thread the real algorithm through the checksum helper so mismatch errors report e.g. 'sha256' instead of the misleading 'unknown'. Closes #786 Signed-off-by: alex-au <alex.w.aus@gmail.com>
1 parent 8db66a6 commit 48ce8f6

5 files changed

Lines changed: 304 additions & 36 deletions

File tree

pkg/software/base_installer.go

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,8 @@ func (b *baseInstaller) downloadAndVerifyArchive(archive ArchiveDetail) error {
201201
}
202202
}
203203

204-
// Download the archive
205-
err = b.downloader.Download(downloadURL, destinationFile)
206-
if err != nil {
207-
return err
208-
}
209-
210-
// Verify the downloaded archive's checksum
211-
err = VerifyChecksum(destinationFile, checksum.Value, checksum.Algorithm)
204+
// Download the archive and verify its checksum, retrying transient corruption
205+
err = b.downloader.DownloadAndVerify(downloadURL, destinationFile, checksum.Value, checksum.Algorithm)
212206
if err != nil {
213207
return err
214208
}
@@ -285,14 +279,8 @@ func (b *baseInstaller) downloadAndVerifyBinary(binary BinaryDetail) error {
285279
}
286280
}
287281

288-
// Download the binary
289-
err = b.downloader.Download(downloadURL, destinationFile)
290-
if err != nil {
291-
return err
292-
}
293-
294-
// Verify the downloaded binary's checksum
295-
err = VerifyChecksum(destinationFile, checksum.Value, checksum.Algorithm)
282+
// Download the binary and verify its checksum, retrying transient corruption
283+
err = b.downloader.DownloadAndVerify(downloadURL, destinationFile, checksum.Value, checksum.Algorithm)
296284
if err != nil {
297285
return err
298286
}
@@ -328,14 +316,8 @@ func (b *baseInstaller) downloadConfigs() error {
328316
}
329317
}
330318

331-
// Download the config file
332-
err = b.downloader.Download(config.URL, configFile)
333-
if err != nil {
334-
return err
335-
}
336-
337-
// Verify the downloaded config file's checksum
338-
err = VerifyChecksum(configFile, config.Value, config.Algorithm)
319+
// Download the config file and verify its checksum, retrying transient corruption
320+
err = b.downloader.DownloadAndVerify(config.URL, configFile, config.Value, config.Algorithm)
339321
if err != nil {
340322
return err
341323
}

pkg/software/downloader.go

Lines changed: 125 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,22 @@ import (
2020
"github.com/hashgraph/solo-weaver/pkg/sanity"
2121
)
2222

23+
// Default retry settings for DownloadAndVerify.
24+
const (
25+
defaultMaxAttempts = 3 // initial attempt + 2 retries
26+
defaultRetryDelay = 1 * time.Second // base delay; grows exponentially per attempt
27+
maxRetryDelay = 30 * time.Second
28+
)
29+
2330
// Downloader is responsible for downloading a software package and checking its integrity.
2431
type Downloader struct {
2532
client *http.Client
2633
timeout time.Duration
27-
basePath string // Base directory for validating download/extraction paths
28-
allowedDomains []string // List of allowed domains for SSRF protection
29-
insecureTLS bool // Skip TLS certificate verification (for local dev with self-signed certs)
34+
basePath string // Base directory for validating download/extraction paths
35+
allowedDomains []string // List of allowed domains for SSRF protection
36+
insecureTLS bool // Skip TLS certificate verification (for local dev with self-signed certs)
37+
maxAttempts int // Number of download+verify attempts before giving up
38+
retryDelay time.Duration // Base backoff delay between attempts (0 disables sleeping)
3039
}
3140

3241
// DownloaderOption is a function that configures a Downloader
@@ -63,6 +72,25 @@ func WithHTTPClient(client *http.Client) DownloaderOption {
6372
}
6473
}
6574

75+
// WithMaxAttempts sets the number of download+verify attempts before giving up.
76+
// Values below 1 are clamped to 1.
77+
func WithMaxAttempts(attempts int) DownloaderOption {
78+
return func(d *Downloader) {
79+
if attempts < 1 {
80+
attempts = 1
81+
}
82+
d.maxAttempts = attempts
83+
}
84+
}
85+
86+
// WithRetryDelay sets the base backoff delay between download attempts.
87+
// A zero delay disables sleeping (useful for tests).
88+
func WithRetryDelay(delay time.Duration) DownloaderOption {
89+
return func(d *Downloader) {
90+
d.retryDelay = delay
91+
}
92+
}
93+
6694
// WithInsecureTLS skips TLS certificate verification.
6795
// WARNING: Only use this for local development with self-signed certificates!
6896
// This option is ignored in release builds for security.
@@ -97,6 +125,8 @@ func NewDownloader(opts ...DownloaderOption) *Downloader {
97125
basePath: models.Paths().HomeDir,
98126
allowedDomains: sanity.AllowedDomains(),
99127
insecureTLS: false,
128+
maxAttempts: defaultMaxAttempts,
129+
retryDelay: defaultRetryDelay,
100130
}
101131

102132
// Apply options
@@ -160,14 +190,105 @@ func (fd *Downloader) Download(url, destination string) error {
160190
}
161191
defer out.Close()
162192

163-
_, err = io.Copy(out, resp.Body)
193+
written, err := io.Copy(out, resp.Body)
164194
if err != nil {
195+
_ = os.Remove(cleanDest)
165196
return NewDownloadError(err, url, 0)
166197
}
167198

199+
// Reject a zero-byte body: a 200-with-empty-response is a corrupt download,
200+
// not a valid file. This is treated as a retryable download error.
201+
if written == 0 {
202+
_ = os.Remove(cleanDest)
203+
return NewDownloadError(errorx.ExternalError.New("downloaded file is empty (0 bytes written)"), url, 0)
204+
}
205+
206+
// When the response advertises a Content-Length, fail fast on a size mismatch
207+
// (a truncated copy or mid-stream reset) rather than persisting a partial file.
208+
// ContentLength is -1 when unknown (e.g. chunked transfer), so only check when >= 0.
209+
if resp.ContentLength >= 0 && written != resp.ContentLength {
210+
_ = os.Remove(cleanDest)
211+
return NewDownloadError(
212+
errorx.ExternalError.New("download truncated: wrote %d of %d advertised bytes", written, resp.ContentLength),
213+
url, 0)
214+
}
215+
168216
return nil
169217
}
170218

219+
// DownloadAndVerify downloads a file from url to destination and verifies its
220+
// checksum, retrying transient corruption. Both download errors (network reset,
221+
// empty/truncated body) and a subsequent checksum mismatch are treated as
222+
// retryable: on failure the bad file is removed and the download is re-attempted
223+
// with capped exponential backoff, up to maxAttempts. The last error is returned
224+
// once attempts are exhausted.
225+
func (fd *Downloader) DownloadAndVerify(url, destination, expectedValue, algorithm string) error {
226+
attempts := fd.maxAttempts
227+
if attempts < 1 {
228+
attempts = 1
229+
}
230+
231+
// Sanitize the destination once so the download, verification, and cleanup all
232+
// operate on the exact same path (Download sanitizes internally, so a caller
233+
// passing a relative/unclean path would otherwise write to one path and verify
234+
// or remove another).
235+
cleanDest, err := sanity.ValidatePathWithinBase(fd.basePath, destination)
236+
if err != nil {
237+
return NewDownloadError(err, url, 0)
238+
}
239+
240+
// Fail fast on deterministic errors that no retry can fix: an invalid/unsafe URL
241+
// and an unsupported checksum algorithm will fail identically on every attempt,
242+
// so retrying them only adds backoff delay and log noise.
243+
if err := sanity.ValidateURL(url, &sanity.ValidateURLOptions{AllowedDomains: fd.allowedDomains}); err != nil {
244+
return NewInvalidURLError(err, url)
245+
}
246+
if !IsSupportedAlgorithm(algorithm) {
247+
return NewChecksumError(cleanDest, algorithm, expectedValue, "")
248+
}
249+
250+
var lastErr error
251+
for attempt := 1; attempt <= attempts; attempt++ {
252+
lastErr = fd.Download(url, cleanDest)
253+
if lastErr == nil {
254+
lastErr = VerifyChecksum(cleanDest, expectedValue, algorithm)
255+
if lastErr == nil {
256+
return nil
257+
}
258+
}
259+
260+
// Remove the corrupt/partial file so a bad download never persists and the
261+
// next attempt (or a later run) starts clean.
262+
_ = os.Remove(cleanDest)
263+
264+
if attempt < attempts {
265+
logx.As().Warn().
266+
Str("url", url).
267+
Int("attempt", attempt).
268+
Int("maxAttempts", attempts).
269+
Err(lastErr).
270+
Msg("Download or checksum verification failed; retrying after backoff")
271+
fd.sleepBackoff(attempt)
272+
}
273+
}
274+
275+
return lastErr
276+
}
277+
278+
// sleepBackoff sleeps for a capped exponential delay before the next attempt.
279+
// A zero base retryDelay disables sleeping.
280+
func (fd *Downloader) sleepBackoff(attempt int) {
281+
if fd.retryDelay <= 0 {
282+
return
283+
}
284+
285+
delay := fd.retryDelay << (attempt - 1)
286+
if delay > maxRetryDelay || delay < fd.retryDelay {
287+
delay = maxRetryDelay
288+
}
289+
time.Sleep(delay)
290+
}
291+
171292
// ExtractTarGz extracts a tar.gz file
172293
func (fd *Downloader) Extract(compressedFilePath string, destDir string) error {
173294
// Validate and sanitize the compressed file path

pkg/software/downloader_test.go

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ package software
55
import (
66
"archive/tar"
77
"compress/gzip"
8+
"crypto/sha256"
89
"fmt"
910
"net/http"
1011
"net/http/httptest"
1112
"os"
1213
"path/filepath"
14+
"sync/atomic"
1315
"testing"
1416
"time"
1517

@@ -106,6 +108,155 @@ func Test_Downloader_Timeout(t *testing.T) {
106108
require.True(t, errorx.IsOfType(err, DownloadError), "Error should be of type DownloadError")
107109
}
108110

111+
func Test_Downloader_Download_ZeroByte(t *testing.T) {
112+
// Server returns 200 with an empty body — a corrupt download.
113+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
114+
w.WriteHeader(http.StatusOK)
115+
}))
116+
defer server.Close()
117+
118+
tmpFile, err := os.CreateTemp("", "test_zerobyte_*.txt")
119+
require.NoError(t, err, "Failed to create temp file")
120+
_ = tmpFile.Close()
121+
defer func() { _ = os.Remove(tmpFile.Name()) }()
122+
123+
downloader := NewDownloader(
124+
WithHTTPClient(server.Client()),
125+
WithBasePath(filepath.Dir(tmpFile.Name())),
126+
WithAllowedDomains([]string{"localhost", "127.0.0.1"}),
127+
)
128+
129+
err = downloader.Download(server.URL, tmpFile.Name())
130+
require.Error(t, err, "Download should reject a zero-byte body")
131+
require.True(t, errorx.IsOfType(err, DownloadError), "Error should be of type DownloadError")
132+
}
133+
134+
func Test_Downloader_Download_ContentLengthMismatch(t *testing.T) {
135+
// Server advertises a larger Content-Length than the body it actually writes,
136+
// simulating a truncated/reset transfer.
137+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
138+
w.Header().Set("Content-Length", "1024")
139+
w.WriteHeader(http.StatusOK)
140+
_, _ = w.Write([]byte("short body"))
141+
}))
142+
defer server.Close()
143+
144+
tmpFile, err := os.CreateTemp("", "test_truncated_*.txt")
145+
require.NoError(t, err, "Failed to create temp file")
146+
_ = tmpFile.Close()
147+
defer func() { _ = os.Remove(tmpFile.Name()) }()
148+
149+
downloader := NewDownloader(
150+
WithHTTPClient(server.Client()),
151+
WithBasePath(filepath.Dir(tmpFile.Name())),
152+
WithAllowedDomains([]string{"localhost", "127.0.0.1"}),
153+
)
154+
155+
err = downloader.Download(server.URL, tmpFile.Name())
156+
require.Error(t, err, "Download should reject a Content-Length mismatch")
157+
require.True(t, errorx.IsOfType(err, DownloadError), "Error should be of type DownloadError")
158+
}
159+
160+
func Test_Downloader_DownloadAndVerify_RetriesThenSucceeds(t *testing.T) {
161+
goodContent := "the real payload"
162+
goodSum := fmt.Sprintf("%x", sha256.Sum256([]byte(goodContent)))
163+
164+
var calls int32
165+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
166+
n := atomic.AddInt32(&calls, 1)
167+
w.WriteHeader(http.StatusOK)
168+
if n == 1 {
169+
// First attempt: empty body (transient corruption).
170+
return
171+
}
172+
_, _ = w.Write([]byte(goodContent))
173+
}))
174+
defer server.Close()
175+
176+
tmpFile, err := os.CreateTemp("", "test_retry_ok_*.txt")
177+
require.NoError(t, err, "Failed to create temp file")
178+
_ = tmpFile.Close()
179+
defer func() { _ = os.Remove(tmpFile.Name()) }()
180+
181+
downloader := NewDownloader(
182+
WithHTTPClient(server.Client()),
183+
WithBasePath(filepath.Dir(tmpFile.Name())),
184+
WithAllowedDomains([]string{"localhost", "127.0.0.1"}),
185+
WithRetryDelay(0), // no sleeping in tests
186+
)
187+
188+
err = downloader.DownloadAndVerify(server.URL, tmpFile.Name(), goodSum, "sha256")
189+
require.NoError(t, err, "DownloadAndVerify should recover from a transient empty download")
190+
require.Equal(t, int32(2), atomic.LoadInt32(&calls), "Expected exactly one retry")
191+
192+
content, err := os.ReadFile(tmpFile.Name())
193+
require.NoError(t, err, "Failed to read downloaded file")
194+
require.Equal(t, goodContent, string(content), "Downloaded content mismatch")
195+
}
196+
197+
func Test_Downloader_DownloadAndVerify_ExhaustsAttempts(t *testing.T) {
198+
// Server always returns a non-empty body that never matches the expected checksum.
199+
var calls int32
200+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
201+
atomic.AddInt32(&calls, 1)
202+
w.WriteHeader(http.StatusOK)
203+
_, _ = w.Write([]byte("persistently wrong content"))
204+
}))
205+
defer server.Close()
206+
207+
tmpFile, err := os.CreateTemp("", "test_retry_fail_*.txt")
208+
require.NoError(t, err, "Failed to create temp file")
209+
_ = tmpFile.Close()
210+
defer func() { _ = os.Remove(tmpFile.Name()) }()
211+
212+
downloader := NewDownloader(
213+
WithHTTPClient(server.Client()),
214+
WithBasePath(filepath.Dir(tmpFile.Name())),
215+
WithAllowedDomains([]string{"localhost", "127.0.0.1"}),
216+
WithMaxAttempts(3),
217+
WithRetryDelay(0),
218+
)
219+
220+
expectedSum := fmt.Sprintf("%x", sha256.Sum256([]byte("the content we wanted")))
221+
err = downloader.DownloadAndVerify(server.URL, tmpFile.Name(), expectedSum, "sha256")
222+
require.Error(t, err, "DownloadAndVerify should fail after exhausting attempts")
223+
require.True(t, errorx.IsOfType(err, ChecksumError), "Final error should be the checksum mismatch")
224+
require.Equal(t, int32(3), atomic.LoadInt32(&calls), "Expected exactly maxAttempts attempts")
225+
226+
// The corrupt file must not persist after the retries are exhausted.
227+
_, statErr := os.Stat(tmpFile.Name())
228+
require.True(t, os.IsNotExist(statErr), "Corrupt download should be removed")
229+
}
230+
231+
func Test_Downloader_DownloadAndVerify_FailsFastOnUnsupportedAlgorithm(t *testing.T) {
232+
// An unsupported algorithm can never verify, so DownloadAndVerify must fail
233+
// before ever contacting the server or entering the retry loop.
234+
var calls int32
235+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
236+
atomic.AddInt32(&calls, 1)
237+
w.WriteHeader(http.StatusOK)
238+
_, _ = w.Write([]byte("content"))
239+
}))
240+
defer server.Close()
241+
242+
tmpFile, err := os.CreateTemp("", "test_failfast_*.txt")
243+
require.NoError(t, err, "Failed to create temp file")
244+
_ = tmpFile.Close()
245+
defer func() { _ = os.Remove(tmpFile.Name()) }()
246+
247+
downloader := NewDownloader(
248+
WithHTTPClient(server.Client()),
249+
WithBasePath(filepath.Dir(tmpFile.Name())),
250+
WithAllowedDomains([]string{"localhost", "127.0.0.1"}),
251+
WithRetryDelay(0),
252+
)
253+
254+
err = downloader.DownloadAndVerify(server.URL, tmpFile.Name(), "abc123", "sha1")
255+
require.Error(t, err, "DownloadAndVerify should reject an unsupported algorithm")
256+
require.True(t, errorx.IsOfType(err, ChecksumError), "Error should be of type ChecksumError")
257+
require.Equal(t, int32(0), atomic.LoadInt32(&calls), "No download should be attempted for an unsupported algorithm")
258+
}
259+
109260
func Test_Downloader_Extract(t *testing.T) {
110261
// Create a temporary directory for test files
111262
tempDir, err := os.MkdirTemp("", "test_extract_*")

0 commit comments

Comments
 (0)