Repository navigation
Serve a Range request for a cached object as 206 instead of the whole object [patch] - #107
Merged
Merged
Conversation
… object [patch] ServeFromStoreAsync always set the full Content-Length and streamed the entire file with 200, so a git-lfs download resumed with Range: bytes=N- against a cached object started over from byte zero. The design spec calls for range support on a store hit; only the miss path honoured Range. Hits are now served through ASP.NET Core's range processing, which answers a satisfiable range with 206 and Content-Range, an unsatisfiable one with 416, and advertises Accept-Ranges: bytes. A request without Range still gets 200 and the whole object. Fixes #62 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WY7QGzbH6kE2oceF1hP8fh
…-hit # Conflicts: # GitLfsCache/Endpoints/ObjectRouteHandler.cs
…agree The re-queue loop brought in from main tested ticket.IsLeader in a for loop whose incrementer advanced attempt, which SonarCloud reports as a critical S1994 finding and which failed this PR's quality gate once the merge put the loop in its diff. The same loop as a while with the counter advanced in the body behaves identically. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WY7QGzbH6kE2oceF1hP8fh
|
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
2 participants
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.
Fixes #62
What was wrong
ObjectRouteHandler.ServeFromStoreAsyncalways set the fullContent-Lengthand streamed the whole file with 200. On a cache hit it sent noAccept-RangesorContent-Range, and never answered 206 or 416. When git-lfs resumed an interrupted download withRange: bytes=N-and the object was already cached, it got every byte again and started over. The design spec calls for range support on a store hit. Only the miss path honouredRange.Change
Results.Stream(cached, OctetStream, enableRangeProcessing: true). ASP.NET Core's range processing answers a satisfiable range with 206 andContent-Range, an unsatisfiable one with 416, and sendsAccept-Ranges: bytes. This follows the triage note to use the built-in processing instead of writing a parser.Rangestill gets 200, the whole object, andapplication/octet-stream.Tests
New
ProxyFlowTestscases each warm the store through a real download first, then hit it:Download_RangeRequestOnAHit_ReturnsPartialContentFromTheStorecovers a closed range (10-19), a suffix range (-5) and an open range (30-). Each checks for 206, the exact bytes, theContent-Range, and that upstream was not fetched again.Download_UnsatisfiableRangeOnAHit_Returns416checks for 416 withbytes */<length>.Download_HitWithoutARange_ReturnsTheWholeObjectAndAdvertisesRangeschecks for 200, the full body,Content-Length, the content type andAccept-Ranges: bytes.With the fix reverted, all 5 new cases fail. The full suite passes locally: 359 tests, 0 failed. The branch merges cleanly with the open #101, #102, #103 and #104.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WY7QGzbH6kE2oceF1hP8fh
Generated by Claude Code