mirror of
https://github.com/GopeedLab/gopeed.git
synced 2026-11-05 16:15:43 +00:00
Fix non-range download bug: downloaded bytes exceeding total file size
When HTTP server doesn't return Content-Length or doesn't support Range requests, the download can accumulate extra bytes on pause/resume because: 1. Each retry/resume re-downloads from the beginning but writes at the accumulated offset 2. No size limit enforcement for non-range downloads 3. onDownloadComplete incorrectly uses chunk.remain() for non-range downloads, which gives negative values for chunk(0,0) Fixes: - Reset chunk state in downloadChunkOnce and runConnectionFallback for non-range downloads since server will send from beginning - Add size limit checks in download loops when Size is known - Fix onDownloadComplete to use conn.Completed for non-range downloads instead of unreliable chunk.remain() - Add test cases for non-range download, pause/resume, and size limit Co-authored-by: monkeyWie <[email protected]>
This commit is contained in:
1 parent
27c0f81a1e
commit
2daa9c45df
3 files changed
+197
-9
No files matched your search
@@ -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,21 @@ 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 {
|
||||
remaining := f.meta.Res.Size - conn.Downloaded
|
||||
if remaining <= 0 {
|
||||
f.connMu.Lock()
|
||||
conn.Completed = true
|
||||
conn.State = connCompleted
|
||||
f.connMu.Unlock()
|
||||
return
|
||||
}
|
||||
if remaining < int64(n) {
|
||||
n = int(remaining)
|
||||
}
|
||||
}
|
||||
|
||||
f.fileMu.Lock()
|
||||
if f.file != nil {
|
||||
_, writeErr := f.file.WriteAt(buf[:n], conn.Chunk.Downloaded)
|
||||
@@ -1193,6 +1227,15 @@ func (f *Fetcher) runConnectionWithResolveResp(conn *connection) {
|
||||
|
||||
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 {
|
||||
f.connMu.Lock()
|
||||
conn.Completed = true
|
||||
conn.State = connCompleted
|
||||
f.connMu.Unlock()
|
||||
return
|
||||
}
|
||||
}
|
||||
if err != nil {
|
||||
if err == io.EOF {
|
||||
@@ -1232,6 +1275,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 +1327,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 +1537,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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -307,6 +307,99 @@ func TestFetcher_DownloadChunked(t *testing.T) {
|
||||
downloadNormal(listener, 2, t)
|
||||
}
|
||||
|
||||
func TestFetcher_DownloadChunkedContinue(t *testing.T) {
|
||||
listener := test.StartTestCustomServer()
|
||||
defer listener.Close()
|
||||
|
||||
downloadContinue(listener, 1, t)
|
||||
}
|
||||
|
||||
func TestFetcher_DownloadNoRangeWithSize(t *testing.T) {
|
||||
listener := test.StartTestNoRangeServer()
|
||||
defer listener.Close()
|
||||
|
||||
downloadNormal(listener, 1, t)
|
||||
}
|
||||
|
||||
func TestFetcher_DownloadNoRangeContinue(t *testing.T) {
|
||||
listener := test.StartTestNoRangeServer()
|
||||
defer listener.Close()
|
||||
|
||||
downloadContinue(listener, 1, t)
|
||||
}
|
||||
|
||||
// TestFetcher_DownloadNoRangeNotExceedSize verifies that when a server returns
|
||||
// Content-Length but doesn't support Range requests, the downloaded bytes never
|
||||
// exceed the total file size after pause and resume.
|
||||
func TestFetcher_DownloadNoRangeNotExceedSize(t *testing.T) {
|
||||
listener := test.StartTestNoRangeServer()
|
||||
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)
|
||||
}
|
||||
|
||||
// Verify total downloaded does not exceed file size
|
||||
totalDownloaded := fetcher.Progress().TotalDownloaded()
|
||||
if totalDownloaded > test.BuildSize {
|
||||
t.Errorf("Downloaded %d bytes exceeds file size %d", totalDownloaded, test.BuildSize)
|
||||
}
|
||||
|
||||
want := test.FileMd5(test.BuildFile)
|
||||
got := test.FileMd5(test.DownloadFile)
|
||||
if want != got {
|
||||
t.Errorf("Download() got = %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFetcher_DownloadChunkedNotExceedSize verifies that when a server doesn't
|
||||
// return Content-Length (chunked transfer), the download properly handles
|
||||
// pause/resume without accumulating extra bytes.
|
||||
func TestFetcher_DownloadChunkedNotExceedSize(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)
|
||||
}
|
||||
|
||||
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()
|
||||
|
||||
@@ -636,6 +636,25 @@ func StartTestRangeBugServer() net.Listener {
|
||||
})
|
||||
}
|
||||
|
||||
// StartTestNoRangeServer starts a server that returns Content-Length but does NOT support Range requests.
|
||||
// This simulates servers where file size is known but range downloads are not supported.
|
||||
func StartTestNoRangeServer() net.Listener {
|
||||
return startTestServer(func(sl *shutdownListener) http.Handler {
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/"+BuildName, func(writer http.ResponseWriter, request *http.Request) {
|
||||
file, err := os.Open(BuildFile)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
defer file.Close()
|
||||
writer.Header().Set("Content-Length", fmt.Sprintf("%d", BuildSize))
|
||||
writer.WriteHeader(200)
|
||||
io.Copy(writer, file)
|
||||
})
|
||||
return mux
|
||||
})
|
||||
}
|
||||
|
||||
func rangeFileHandle(writer http.ResponseWriter, request *http.Request, modifyEnd func(end int64) int64, iocpN func(file *os.File, n int64)) {
|
||||
r := request.Header.Get("Range")
|
||||
|
||||
|
||||
Reference in new issue
Block a user