[miniflare] R2 local public bucket fixes#14323
Open
tahmid-23 wants to merge 8 commits into
Open
Conversation
… R2 public endpoint Write methods get 401 (not 405): r2.dev has no authenticated mode, writes go through the S3 API or bindings. Failed preconditions return a bare 412 with no object headers, like r2.dev (whose error responses carry Cloudflare's HTML error pages; locally only the status code is mimicked). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on the local R2 public endpoint R2's HTTP endpoints only accept a single range with start <= end; anything else (including multiple ranges) is rejected with 400 rather than ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
r2.dev answers ranged HEAD requests with a bodyless 206 and Content-Range; previously the range was ignored for HEAD and a 200 with the full length was returned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…c endpoint `object.range` may carry all keys with some undefined (e.g. `suffix` present but undefined on an offset range), so normalize by value rather than key presence. Suffix ranges (`bytes=-N`) previously fell through to a 200 whose Content-Length reported the full object size against a partial body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 72537be The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
emily-shen
and removed request for
a team
Contributor
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
5 tasks
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
| if (!("body" in recheck)) { | ||
| return c.body(null, { status: 412, headers: objectHeaders(recheck) }); | ||
| return c.body(null, 412); |
Contributor
There was a problem hiding this comment.
🚩 412 response no longer includes object headers — intentional behavioral change
The old code at this line returned c.body(null, { status: 412, headers: objectHeaders(recheck) }), including ETag, Last-Modified, etc. in the 412 response. The new code returns c.body(null, 412) with no headers at all. Per RFC 7232 §4.2, a 412 response SHOULD include entity headers. The PR intends to match r2.dev behavior, and the tests only assert the status code, so this appears intentional. Worth confirming this matches actual r2.dev responses if fidelity is important.
Was this helpful? React with 👍 or 👎 to provide feedback.
tahmid-23
force-pushed
the
r2-local-public-bucket-fixes
branch
from
37a6395 to
de6e279
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
…ic endpoint The simulator clamps ranges starting at or beyond the object size; r2.dev rejects them with 416. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dpoint Hono already percent-decodes path params, so decoding them again corrupted keys containing `%`: a literal `%` that does not form a valid escape threw a URIError (surfacing as a bare 500), and keys like `a%2Bb` were silently served as `a+b`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The public endpoint must fetch with `bucket.get()` even for HEAD (only `get` evaluates conditional headers and ranges), and the 416 path returns before reading the body. Cancel the stream in both cases instead of leaving it open until garbage collection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tahmid-23
force-pushed
the
r2-local-public-bucket-fixes
branch
from
de6e279 to
72537be
Compare
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.
A series of fixes on my initial PR #14119. These fixes mostly aim to have parity with the real public bucket URL. Some of these are somewhat cosmetic.
I observed these behaviors against r2.dev. Some of these checks will be needed for correctness in #14280.
A picture of a cute animal (not mandatory, but encouraged)