skymethod / denoflare

Develop, test, and deploy Cloudflare Workers with Deno.
https://denoflare.dev
MIT License
710 stars 36 forks source link

Allow range header on HEAD requests for r2-public-read-worker #24

Closed droher closed 2 years ago

droher commented 2 years ago

I'm not sure how unusual this is, but I have a use case for range headers on HEAD requests - the duckdb-wasm library can do partial reads on files, and it uses a HEAD request with a range of bytes=0- to check whether partial reads are supported. The current worker only computes the range for GET requests. Simply removing the conditional makes it work as expected:

        let range = tryParseRange(headers);
johnspurlock-skymethod commented 2 years ago

Hmm this one is tricky.

The http spec is pretty explicit afaict that Range should be ignored for HEAD requests

A server MUST ignore a Range header field received with a request method other than GET.

But S3 head-object returns 206 partial content responses (like a GET) with no body - which would work for a lib like yours.

Are you sure your lib uses HEAD for detection? Usually libs use GET for the first two bytes or something and look at the response headers.

In the meantime I have a question out to the R2 team to see what they are planning to do in their upcoming native public bucket website implementation.

droher commented 2 years ago

Yeah, I'm sure that they use HEAD here. It's an immature library, so perhaps the better fix is on their end (unless they have an S3-related reason for doing it this way, which is possible). In the meantime I have it patched for my little project, so it's not a problem for me. Thanks for checking with R2!

johnspurlock-skymethod commented 2 years ago

Sounds like R2 is going to handle it like S3, so I'll make the change here too.

Just published commit 24f66b72d56ad7bc5a3c3e7b4d7f5174532f55d0 (24f66b72d56ad7bc5a3c3e7b4d7f5174532f55d0) that should implement this, would you mind testing it with your use case?

To deploy from a commit, just change the url in the deploy instructions, e.g. v0.5.6 to 24f66b72d56ad7bc5a3c3e7b4d7f5174532f55d0

droher commented 2 years ago

Yep, works perfectly.

johnspurlock-skymethod commented 2 years ago

Shipped with tagged release v0.5.7

https://github.com/skymethod/denoflare/releases