fix: Range suffixes are not Rust RangeTo #155
Conversation
seanmonstar
force-pushed
the
range-satisfiable
branch
from
fb9a4e0
to
836f397
Compare
|
This waited so long because it was a breaking change, but can now be included with the It's a very conservative change, while allowing something much more comprehensive from eventually being added. |
|
Idk if you're big on tests, but if we're testing satisfiable range, then the opposite should also be tested, shouldn't it? |
An HTTP Range of `bytes=-100` means a suffix, the last 100 bytes. This was wrongly parsed as the Rust range `..100`, which means the first 100 bytes. This has been fixed, but doing so required change `Range::iter` to accept a length argument, to determine if the ranges are satisfiable. BREAKING CHANGE: Change `.iter()` calls to `.satisfiable_ranges(len)`. Also, the `Range::bytes()` constructor will now return an error if pass a `RangeTo` (e.g. `Range::bytes(..100)`).
seanmonstar
force-pushed
the
range-satisfiable
branch
from
836f397
to
6e0517f
Compare
| pub fn satisfiable_ranges<'a>( | ||
| &'a self, | ||
| len: u64, | ||
| ) -> impl Iterator<Item = (Bound<u64>, Bound<u64>)> + 'a { |
There was a problem hiding this comment.
Hi @seanmonstar
Thanks for these changes, but it looks like it still works not as expected
We can pass the len of the content, but for unbounded or included > len there is no usage of min(), like for example it was implemented in #93
And in general would be nice to have more validation, like from < to.
Otherwise hard to say it's "satisfiable" ranges 🤔
An HTTP Range of
bytes=-100means a suffix, the last 100 bytes. This was wrongly parsed as the Rust range..100, which means the first 100 bytes.This has been fixed, but doing so required change
Range::iterto accept a length argument, to determine if the ranges are satisfiable.Closes #87
BREAKING CHANGE: Change
.iter()calls to.satisfiable_ranges(len).Also, the
Range::bytes()constructor will now return an error ifpass a
RangeTo(e.g.Range::bytes(..100)).