Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses an HTTP specification edge case where a server ignores or does not honor a
Rangerequest and returns200 OK(with the entire file) instead of206 Partial Content, and resolves minor edge cases inResponsestatistics.Problem Details & Fixes
Resume fallback when server returns HTTP 200 OK instead of 206 Partial Content:
Rangeheader and respond with200 OKcontaining the entire representation from byte 0.grabpreviously assumed that if aRangeheader was sent, any 2xx response was a successful resume. If the server returned200 OK,DidResumeremainedtrueand the destination file was opened withO_APPENDseeking toio.SeekEnd. The full200 OKbody was then appended to the existing partial file, corrupting the downloaded file and failing checksum verification.readResponse, ifresp.requestMethod() == "GET" && resp.DidResumeand the status code is not206 Partial Content,DidResumeis set tofalseandbytesResumedis reset to0. This allowsopenWriterandcopyFileto safely truncate the existing partial file and cleanly download the full representation without corruption.pkg/grabtest/handler.go, mock range requests now properly return206 Partial ContentwithContent-Rangeheaders when serving partial ranges.Response edge cases:
Response.ETA(): Returnstime.Time{}if transfer size is unknown (Size() <= 0) orbps <= 0, preventing negative duration calculations from returning past timestamps. Also calculates sub-second remaining time with float duration precision.Response.BytesPerSecond(): Prevents division by zero (+Inf/NaN) if a completed transfer has a duration of 0.Response.Duration(): Returns0ifStart.IsZero().Tests Added
TestResumeFallsBackToFullWhenServerReturns200OK: Verifies that when a local partial file exists and the remote server responds to a ranged GET with200 OK,DidResumeis set tofalse, the file is truncated and cleanly downloaded, and checksums match.TestResponseStatsEdgeCases: Verifies zero duration, zero BPS division guard, and ETA handling for unknown size transfers.go test ./...andgo vet ./...across amd64 and 386).