Mapping of HTTP ranges to std::ops::Bound based ranges is faulty

[jcgruenhage]

tl;dr: bytes=-x is being mapped to ..x by this crate, where http headers mean the last x bytes, and rust BoundRanges mean the first x bytes.

Introduction to the ranges and their definitions

HTTP Ranges

For HTTP ranges, there are four options:

1. Not defining a range. This means everything is included
2. Range: <unit>=<range-start>-, where only the range beginning is given. This means everything from start is included.
3. Range: <unit>=<range-start>-<range-end>, where both the start and end is given. This means everything between start and end are included
4. Range: <unit>=-<suffix-length>, where just a suffix is given. This means only the tail, the last suffix bytes are returned.

Rust RangeBounds

Rust's RangeBounds also have (kinda) four options:

1. .., which includes everything.
2. start.., which includes everything except the first start bytes.
3. start..end (or start..=(end - 1)), which includes everything from start to end, excluding end.
4. ..end (or ..=(end - 1)), which includes everything from 0 to end, excluding end.

Current Mapping between them

Currently, this mapping is just done one-to-one based on their syntax, not based on their definition. This means that Range: bytes=x-y is mapped to x..y, which is fine if both x and y are defined. Only including x is also fine, as they'll still mean the same. But, if only have y, the mapping becomes wrong.

There is no correct mapping for these cases, as "give me the last n bytes" is not something you can represent in Rust's RangeBounds. When the length we're dealing with is known, conversion is possible, but RangeBounds are probably not the right type in that case for storing the header information, as converting back isn't possible.

Options

Require length before returning ranges

Given the length, converting from HTTP Ranges is possible. This would mean that the signature of headers::Range::iter would need to be changed, to include the length of the content being returned.

Don't use RangeBounds

Alternatively to giving the length, we could use a separate type here, and have a conversion method (which takes the length again) to map it to RangeBounds.

[seanmonstar]

4. Range: <unit>=-<suffix-length>, where just a suffix is given. This means only the tail, the last suffix bytes are returned.

Well then, that's a confusing and inconsistent variant.

Regarding your two options, that does seem about right. I'm not immediately sure which I like more. Do you have recommendations on which? It's unfortunate that both are "breaking changes", but it seems necessary. I suppose we could introduce an iter_betterer(len) method, and deprecate the original, but probably just go straight for the breaking change and bump the minor version.

[jcgruenhage]

Well then, that's a confusing and inconsistent variant.

That's true, and I wouldn't have guessed it from the syntax either, if a coworker hadn't pointed it out to me.. It is more powerful than Rust's RangeBounds though, since they can't be used to represent this variant.

I'm not immediately sure which I like more.

Me neither. I think I slightly prefer the later variant, of creating a type here, but I don't really have a strong opinion on how to do this.

It's unfortunate that both are "breaking changes", but it seems necessary. I suppose we could introduce an iter_betterer(len) method, and deprecate the original, but probably just go straight for the breaking change and bump the minor version.

I think going straight for a breaking change is best, yes. headers is still on a 0.x.y version after all, so users of the library should know that breaking changes are still coming in when necessary.

[jcgruenhage]

Oh, and what I forgot to ask: Do you prefer fixing it yourself, or would you prefer this to be fixed in a PR?

[jcgruenhage]

@seanmonstar can you take a look at the PR provided for fixing this issue?