workers-rs
workers-rs copied to clipboard
[BUG] Range requests in R2 are limited to 32 bit offset/length
Is there an existing issue for this?
- [X] I have searched the existing issues
What version of workers-rs
are you using?
0.2.0
What version of wrangler
are you using?
3.53.0
Describe the bug
The worker::r2::builder::Range
enum is defined like this:
pub enum Range {
OffsetWithLength { offset: u32, length: u32 },
OffsetWithOptionalLength { offset: u32, length: Option<u32> },
OptionalOffsetWithLength { offset: Option<u32>, length: u32 },
Suffix { suffix: u32 },
}
Using u32
for all the quantities makes the interface completely unable to handle files more than 4 GiB in size. I've done a bit of research, and it seems the code casts 32-bit numbers to f64
for JS (essentially supporting 52 bits), and those are converted into u64
in the workerd C++ code.
I can see multiple ways to fix this to varying degrees:
- Simply changing Rust high-level definition to
u64
, low-levelR2Range
definition tof64
, and erroring out during conversion if the number does not fit intof64
without precision loss. Will fix the practical problem with the least effort, but this is certainly a hack. Values read from the response could be silently wrong if they exceed 52 bits, though (should not be a practical issue). - There seems to be a (somehow undocumented) way to pass a
Headers
instance with pre-madeRange
header. The Rust code could format the header manually fromu64
values, bypassing the JSf64
dance entirely. - Support
BigInt
in the JS interface to pass throughu64
losslessly?
As a side note, the current Range
Rust definition seems very weird to me: there is way too much overlap between the first three variants, and some semantics are not clear to me. What does OptionalOffsetWithLength{None, L}
even mean? Is it the same as OffsetWithLength{0, L}
/OffsetWithOptionalLength{0, Some(L)}
? Or Suffix{L}
? I think that the Range semantics could be made simpler, and it could be done simultaneously with the required breaking change to fix the data types. Perhaps this could work?
pub enum Range {
OffsetWithLength { offset: u64, length: u64 },
FromOffset { offset: u64 },
Suffix { length: u64 },
}
Steps To Reproduce
No response
Thanks for the detailed description. I think option 1 is most viable, but I will investigate 2 I believe it may just support the standard Range header.
Regarding the API, I think this may have just been copied from the JavaScript API which appears to support the same options. I suspect that the Rust types are the way they are to allow for these 3 options, but not allow for omitting both. I will ask around about what omitting the offset does and see if your proposal is viable.