Return an error for degenerate Range/ContentRange bounds instead of overflowing (#231) - #235
Open
youdie006 wants to merge 1 commit into
Open
Conversation
…verflowing Range::bytes and ContentRange::bytes did unchecked u64 arithmetic on the bounds. An exclusive end of 0 (e.g. Range::bytes(0..0)) computed `end - 1`, which panics in debug and, in release, produces a bogus 2^64-byte or negative-width range header; ContentRange had the same class of overflow on Excluded-start (`s + 1`), Excluded-end (`e - 1`), and Unbounded-end with a complete_length of 0 (`max - 1`). Both constructors already return a Result and error on other invalid inputs, so return that same error for degenerate/empty/overflowing bounds via checked arithmetic. Add regression tests. Fixes hyperium#231
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #231.
Problem
Range::bytesandContentRange::bytesdo uncheckedu64arithmetic on the bounds:Range::bytes(0u64..0u64)computesend - 1on the exclusive end → panics in debug (attempt to subtract with overflow), and in release emitsbytes=0-18446744073709551615(a 2^64-byte span).Range::bytes(3u64..3u64)yieldsbytes=3-2(start > last).ContentRange::byteshas the same class of bug: Excluded-starts + 1overflows atu64::MAX, Excluded-ende - 1underflows at0, and an Unbounded end withcomplete_length: Some(0)underflows onmax - 1.Fix
Both constructors already return a
Resultand alreadyErron other invalid inputs (e.g. unbounded-start arms), so the contract-consistent fix is to return that same error for degenerate/empty/overflowing bounds using checked arithmetic, rather than panicking or emitting an invalid header.Test
Added
test_bytes_rejects_degenerate_bounds(range.rs) andbytes_rejects_degenerate_bounds(content_range.rs). Red-green verified withcargo test: before the change both panic withattempt to subtract with overflow; after, the degenerate cases returnErrand valid ranges still succeed.cargo fmt --checkis clean and the change adds no new clippy warnings.Disclosure: I used AI assistance (Claude) while preparing this change. I reproduced the overflow, ran the tests (red-green), and take responsibility for the contribution.