Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
100 changes: 89 additions & 11 deletions internal/protocol/http/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
}

Expand Down
52 changes: 52 additions & 0 deletions internal/protocol/http/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down