Skip to content

client: handle non-206 response to Range requests and improve response stats - #116

Open
mazkev wants to merge 1 commit into
cavaliergopher:mainfrom
mazkev:fix/resume-206-check
Open

mazkev wants to merge 1 commit into
cavaliergopher:mainfrom
mazkev:fix/resume-206-check

Conversation

@mazkev

@mazkev mazkev commented Nov 12, 2026

Copy link
Copy Markdown

Summary

This PR addresses an HTTP specification edge case where a server ignores or does not honor a Range request and returns 200 OK (with the entire file) instead of 206 Partial Content, and resolves minor edge cases in Response statistics.

Problem Details & Fixes

  1. Resume fallback when server returns HTTP 200 OK instead of 206 Partial Content:

    • Specification: According to RFC 9110 §14.2 / RFC 7233 §3.1, an origin server or cache proxy may ignore a Range header and respond with 200 OK containing the entire representation from byte 0.
    • Bug: grab previously assumed that if a Range header was sent, any 2xx response was a successful resume. If the server returned 200 OK, DidResume remained true and the destination file was opened with O_APPEND seeking to io.SeekEnd. The full 200 OK body was then appended to the existing partial file, corrupting the downloaded file and failing checksum verification.
    • Fix: In readResponse, if resp.requestMethod() == "GET" && resp.DidResume and the status code is not 206 Partial Content, DidResume is set to false and bytesResumed is reset to 0. This allows openWriter and copyFile to safely truncate the existing partial file and cleanly download the full representation without corruption.
    • In pkg/grabtest/handler.go, mock range requests now properly return 206 Partial Content with Content-Range headers when serving partial ranges.
  2. Response edge cases:

    • Response.ETA(): Returns time.Time{} if transfer size is unknown (Size() <= 0) or bps <= 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(): Returns 0 if Start.IsZero().

Tests Added

  • TestResumeFallsBackToFullWhenServerReturns200OK: Verifies that when a local partial file exists and the remote server responds to a ranged GET with 200 OK, DidResume is set to false, 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.
  • All existing tests pass (go test ./... and go vet ./... across amd64 and 386).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant