Repository navigation
Support suffix byte ranges and stop masking upstream 416s as 404s - #3
Draft
fangpenlin wants to merge 2 commits into
Draft
fangpenlin wants to merge 2 commits into
fangpenlin wants to merge 2 commits into
Conversation
Clients may resume or probe a blob with the RFC 9110 suffix form 'Range: bytes=-<n>', asking for the last n bytes. That form was parsed as malformed and silently downgraded to a full 200 response. Model BlobRangeRequest as a union of an offset range and a suffix range, resolve suffixes against the object size in the R2 backend, and forward them upstream verbatim. A suffix longer than the object is satisfied by the whole object, while a zero-length suffix is unsatisfiable. Co-authored-by: Fang-Pen Lin <hello@fangpenlin.com>
When a blob is not cached in R2, the blob GET handler falls back to the configured upstream registries. Any error from an upstream was treated as a miss, so an unsatisfiable range reached the client as a synthetic 404 BLOB_UNKNOWN, telling it the blob does not exist and discarding the Content-Range that carries the real object size. Report a 416 from an upstream directly, matching how a 416 from the primary R2 store is already handled. Other failures keep falling through to the next registry. Co-authored-by: Fang-Pen Lin <hello@fangpenlin.com>
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.
Follow-up to the range forwarding work in cloudflare/serverless-registry#147, fixing two range edge cases found while reviewing it.
Suffix ranges (
bytes=-<n>)Range: bytes=-1024is the RFC 9110 suffix form asking for the last 1024 bytes. The parser required at least one digit before the dash, so this valid form was treated as malformed and silently downgraded to a full200response.BlobRangeRequestis now a union of an offset range and a suffix range. The R2 backend resolves the suffix against the object size, andRegistryHTTPClientforwards it upstream verbatim asbytes=-<n>. Per RFC 9110, a suffix longer than the object is satisfied by the whole object, while a zero-length suffix is unsatisfiable and returns416.R2 supports suffixes natively (
R2Rangeaccepts{ suffix }), but resolving against the size we already have fromhead()keeps theContent-Rangeconstruction in one place alongside the existing offset path.Upstream
416reported as404When a blob is not cached in R2, the handler falls back to the configured upstream registries. The loop treated every upstream error as a miss and continued, so an upstream
416was discarded and the request bottomed out at a synthetic404 BLOB_UNKNOWN.That told the client the blob does not exist, when it does, and dropped the
Content-Range: bytes */<size>header carrying the size the client needs in order to retry. A resuming client would abort the pull instead of restarting at a valid offset, which is the failure mode #64 describes.An upstream
416is now returned directly, matching how a416from the primary R2 store is already handled. Blobs are content addressed, so every registry holding a digest holds the same bytes and would answer the same way. Other failures still fall through to the next registry, which is covered by a test.Testing
Six new tests, all driven through the worker or the real HTTP client:
206, oversized suffix returns the whole object,bytes=-0returns416, andbytes=-still serves the full object416reaches the client as416with itsContent-Rangeintact416upstream failure still falls through to a second registryRegistryHTTPClientforwards{ suffix: 4 }asRange: bytes=-4and surfaces the partial contentVerified the
416test fails without the fix (expected 404 to deeply equal 416) so it is not vacuous.pnpm typecheck,pnpm lintandpnpm format:checkall pass. The full suite is 71 passed / 2 failed; both failures are thebackground layer cachingtests exceeding the default 5s vitest timeout, which reproduce identically on unmodifiedmainand are unrelated to this change.range_tests.log
Note for a separate change
RegistryHTTPClient.getLayerhas a pre-existing quirk in its redirect retry: when the retried request fails it returns the first response rather than the retry's, so a416behind a presigned-URL redirect would still be lost. Left alone here to keep this change focused.To show artifacts inline, enable in settings.