Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Range header issues #197

Open
liamwhite opened this issue Dec 12, 2024 · 1 comment
Open

Range header issues #197

liamwhite opened this issue Dec 12, 2024 · 1 comment

Comments

@liamwhite
Copy link

liamwhite commented Dec 12, 2024

The Range header is one of the least useful header types in this library. Most of these problems stem from the fact that it contains only a string HeaderValue, instead of a parsed representation of the Range.

  1. The internal representation of range endpoints passed to bytes and returned from satisfiable_ranges is Bound<u64>. But this is a very poor fit, because it suggests in the return type that Bound::Excluded would be a valid value to use in a Range header, when it is actually never possible for it to appear.

    headers should re-introduce the ByteRangeSpec enum to encode the three valid cases for the Range header.

  2. Range::bytes allows construction from a single impl RangeBounds<u64>. Range should be extended to allow construction from impl Iterator<ByteRangeSpec>.

  3. The internal representation of a Range is a string cloned from the HeaderValue passed to decode. It should instead be a collection of ByteRanges, perhaps a SmallVec<[ByteRangeSpec; 1]> since the expected case is that there will only be one range (multipart ranges are generally uncommon).

    If you are constructing or parsing this header, it is expected that you are going to access the fields, so the extra memory overhead of doing so is explicitly necessary. And the type already clones the string header value! Doing the SmallVec optimization would reduce the number of allocations by 1 for the typical case.

  4. satisfiable_ranges assumes the caller will know the size of the resource being fetched. It may not, in which case passing u64::MAX will return nonsense results when a suffix range is parsed.

@liamwhite
Copy link
Author

related: #93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant