diff --git a/internal/protocol/http/fetcher.go b/internal/protocol/http/fetcher.go index 709cd70f8..1b45cf843 100644 --- a/internal/protocol/http/fetcher.go +++ b/internal/protocol/http/fetcher.go @@ -872,9 +872,9 @@ func (f *Fetcher) expandConnections() { f.connMu.Unlock() if len(newConns) == 0 { - // No new connections could be created, stop expansion + // No new connections could be created, stop expansion. + // The downloadLoop will check stateSteady and call waitForCompletion. f.setState(stateSteady) - go f.waitForCompletion() return } @@ -987,6 +987,14 @@ func (f *Fetcher) downloadChunkOnce(conn *connection, client *http.Client, buf [ f.connMu.Unlock() return nil } + // For non-range downloads, reset chunk state since server will send from beginning. + // Without Range support, each new request restarts the entire transfer. + if !f.meta.Res.Range { + conn.Chunk.Begin = 0 + conn.Chunk.End = 0 + conn.Chunk.Downloaded = 0 + conn.Downloaded = 0 + } rangeStart := conn.Chunk.Begin + conn.Chunk.Downloaded rangeEnd := conn.Chunk.End f.connMu.Unlock() @@ -1080,6 +1088,17 @@ func (f *Fetcher) downloadChunkOnce(conn *connection, client *http.Client, buf [ n = int(remain) finished = true } + } else if f.meta.Res.Size > 0 { + // For non-range downloads with known size, enforce size limit + remaining := f.meta.Res.Size - conn.Downloaded + if remaining <= 0 { + f.connMu.Unlock() + return nil + } + if remaining < int64(n) { + n = int(remaining) + finished = true + } } writeOffset = conn.Chunk.Begin + conn.Chunk.Downloaded f.connMu.Unlock() @@ -1174,6 +1193,22 @@ func (f *Fetcher) runConnectionWithResolveResp(conn *connection) { n, err := reader.Read(buf) if n > 0 { + // For non-range downloads with known size, enforce size limit + if !f.meta.Res.Range && f.meta.Res.Size > 0 { + f.connMu.Lock() + remaining := f.meta.Res.Size - conn.Downloaded + if remaining <= 0 { + conn.Completed = true + conn.State = connCompleted + f.connMu.Unlock() + return + } + if remaining < int64(n) { + n = int(remaining) + } + f.connMu.Unlock() + } + f.fileMu.Lock() if f.file != nil { _, writeErr := f.file.WriteAt(buf[:n], conn.Chunk.Downloaded) @@ -1191,8 +1226,18 @@ func (f *Fetcher) runConnectionWithResolveResp(conn *connection) { } f.fileMu.Unlock() + f.connMu.Lock() conn.Chunk.Downloaded += int64(n) conn.Downloaded += int64(n) + + // Check if size limit reached after updating counters + if !f.meta.Res.Range && f.meta.Res.Size > 0 && conn.Downloaded >= f.meta.Res.Size { + conn.Completed = true + conn.State = connCompleted + f.connMu.Unlock() + return + } + f.connMu.Unlock() } if err != nil { if err == io.EOF { @@ -1232,6 +1277,14 @@ func (f *Fetcher) runConnectionFallback(conn *connection) { f.connMu.Lock() conn.State = connConnecting + // For non-range downloads, reset chunk state since server will send from beginning. + // Without Range support, each new request restarts the entire transfer. + if !f.meta.Res.Range { + conn.Chunk.Begin = 0 + conn.Chunk.End = 0 + conn.Chunk.Downloaded = 0 + conn.Downloaded = 0 + } f.connMu.Unlock() err := func() error { @@ -1276,6 +1329,17 @@ func (f *Fetcher) runConnectionFallback(conn *connection) { n, err := reader.Read(buf) if n > 0 { + // For non-range downloads with known size, enforce size limit + if !f.meta.Res.Range && f.meta.Res.Size > 0 { + remaining := f.meta.Res.Size - conn.Downloaded + if remaining <= 0 { + return nil + } + if remaining < int64(n) { + n = int(remaining) + } + } + f.fileMu.Lock() if f.file != nil { _, writeErr := f.file.WriteAt(buf[:n], conn.Chunk.Downloaded) @@ -1475,17 +1539,31 @@ func (f *Fetcher) onDownloadComplete() { // Check if all chunks are complete (no remaining bytes) allChunksComplete := true for _, conn := range f.connections { - if conn.Chunk != nil && conn.Chunk.remain() > 0 && !conn.Completed && conn.State != connCompleted { - // This connection has remaining work and isn't done - // Check if it failed with 403 (server limit) - these can be ignored if other connections completed the work - if conn.State == connFailed && conn.failed { - if re := extractRequestError(conn.lastErr); re != nil && re.Code == 403 { - // 403 is server connection limit, check if other connections will complete this chunk - continue + if !conn.Completed && conn.State != connCompleted { + if f.meta.Res.Range { + // For range downloads, use chunk remain() to check completion + if conn.Chunk != nil && conn.Chunk.remain() > 0 { + // This connection has remaining work and isn't done + // Check if it failed with 403 (server limit) - these can be ignored if other connections completed the work + if conn.State == connFailed && conn.failed { + if re := extractRequestError(conn.lastErr); re != nil && re.Code == 403 { + continue + } + } + allChunksComplete = false + break } + } else { + // For non-range downloads, completion is determined solely by EOF (conn.Completed). + // chunk.remain() is unreliable for non-range chunks (Begin=0, End=0). + if conn.State == connFailed && conn.failed { + if re := extractRequestError(conn.lastErr); re != nil && re.Code == 403 { + continue + } + } + allChunksComplete = false + break } - allChunksComplete = false - break } } diff --git a/internal/protocol/http/fetcher_test.go b/internal/protocol/http/fetcher_test.go index ec28b6a37..c49502108 100644 --- a/internal/protocol/http/fetcher_test.go +++ b/internal/protocol/http/fetcher_test.go @@ -307,6 +307,58 @@ func TestFetcher_DownloadChunked(t *testing.T) { downloadNormal(listener, 2, t) } +// TestFetcher_DownloadNoRangePauseResume targets a specific bug where pausing +// and resuming a download from a server that doesn't return Content-Length +// (and doesn't support Range) causes the downloaded file to grow beyond the +// actual file size. Root cause: on resume the server resends from byte 0, but +// the old code wrote at the previously accumulated offset, appending a second +// copy instead of overwriting. The fix resets the write offset on each non-range +// retry so data is always written from the beginning. +func TestFetcher_DownloadNoRangePauseResume(t *testing.T) { + listener := test.StartTestCustomServer() + defer listener.Close() + + fetcher := downloadReady(listener, 1, t) + err := fetcher.Start() + if err != nil { + t.Fatal(err) + } + // Let it download some data + time.Sleep(time.Millisecond * 50) + if err := fetcher.Pause(); err != nil { + t.Fatal(err) + } + time.Sleep(time.Millisecond * 50) + if err := fetcher.Start(); err != nil { + t.Fatal(err) + } + err = fetcher.Wait() + if err != nil { + t.Fatal(err) + } + + // The critical assertion: the downloaded file must not be larger than + // the source file. Before the fix, each resume appended extra data, + // causing the file to exceed the actual size (the bug shown in the issue). + downloadInfo, err := os.Stat(test.DownloadFile) + if err != nil { + t.Fatal(err) + } + sourceInfo, err := os.Stat(test.BuildFile) + if err != nil { + t.Fatal(err) + } + if downloadInfo.Size() > sourceInfo.Size() { + t.Errorf("Downloaded file size %d exceeds source file size %d", downloadInfo.Size(), sourceInfo.Size()) + } + + want := test.FileMd5(test.BuildFile) + got := test.FileMd5(test.DownloadFile) + if want != got { + t.Errorf("Download() got = %v, want %v", got, want) + } +} + func TestFetcher_DownloadPost(t *testing.T) { listener := test.StartTestPostServer() defer listener.Close()