Repository navigation
fix: cover all arbitrary HTTP methods and schemes - #596
Open
RaphaelFakhri wants to merge 1 commit into
Open
RaphaelFakhri wants to merge 1 commit into
RaphaelFakhri wants to merge 1 commit into
Conversation
The `Arbitrary` impls for `http_method::Type` and `scheme::Type` pick a variant with a modulo whose range does not match the match arms: * `scheme::Type` uses `% 3` but matches `3` for `Unregistered`, so it never produces an unregistered scheme and produces the invalid `Registered(2)` for a third of the values. * `http_method::Type` uses `% 9` and matches `8` for `Unregistered`, so it never produces the registered `TRACE` method (8). This change uses `% 10` with `9` for unregistered methods and matches `2` for unregistered schemes, so both generators cover every registered value and the unregistered variant, and never produce an invalid registered value. Tests generate values from both impls and check that every registered variant and the unregistered variant appear and that every registered value is valid. Signed-off-by: Raphael Fakhri <153192858+RaphaelFakhri@users.noreply.github.com>
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.
Problem
The
arbitraryfeature'sArbitraryimpls forhttp_method::Typeandscheme::Typechoose a variant with a modulo whose range does not match the match arms:scheme::Typeusesu16::arbitrary(g) % 3but matches3forUnregistered. That arm is unreachable, so the generator never produces an unregistered scheme, and one third of its values areRegistered(2), which is not a validscheme::Registeredvalue.http_method::Typeuses% 9and matches8forUnregistered, so it never produces the registeredTRACEmethod (8).Property tests that use these generators, such as the tap match tests in linkerd2-proxy, never exercise unregistered schemes or
TRACE, and they often get an invalid registered scheme.Solution
http_method::Type: use% 10and match9forUnregistered, so0..=8covers every registered method.scheme::Type: match2forUnregistered, so0..=1coversHTTPandHTTPS.Validation
Two new tests (
arbitrary_http_method_covers_all_typesandarbitrary_scheme_covers_all_types) generate 10,000 values each. They check that every registered value is valid and that every registered variant and the unregistered variant appear.cargo test --all-features --lockedwithout the fix: 2 failed.cargo fmt -- --checkpasses.