fix: reject bare trailing decimal point (empty fraction) in decimal string parse#25
fix: reject bare trailing decimal point (empty fraction) in decimal string parse#25thedavidmeister wants to merge 3 commits into
Conversation
Adversarial mutation pass over decimalStringTofixedPoint. Existing tests
already kill most mutants; two genuine surviving mutants were found and
killed with new discriminating tests:
- Fractional-part overflow propagation (line 71, previously uncovered): a
>77-digit fraction overflows uint256 in the inner integer parse and the
resulting ParseDecimalOverflow selector is returned from the fractional
error branch. Pinned with a 78-nine fraction.
- Decimal-point gate (line 44): a non-point character directly after the
integer with no further input ("1x") must be rejected as an invalid
string. Without the gate the parse wrongly succeeds as 1e18. The
pre-existing corrupt-integer tests only reached this path via inputs whose
garbage is caught by a later gate, so they did not discriminate it.
Two remaining survivors are equivalent mutants (the strip-loop lower bound
>= -> > at fracStart, and the frac-add overflow guard < -> <=) that cannot
change any output; documented as such, no test added.
Tests only: src/ is unchanged. Adds an audit/mutation-test-scans.json scan
record for org-wide health tracking.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tring parse An empty fraction is invalid per the org design ruling in #24: the decimal point MUST be followed by at least one digit, so "1." returns ParseDecimalInvalidString instead of parsing as 1e18. Pins the revert on the exact selector for the whole category and pins adjacent valid forms unchanged. Closes #24 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thedavidmeister
added
the
ai:ready
|
🤖 ai:producer |
|
🤖 ai:vetter |
1 participant
What
LibFixedPointDecimalParse.decimalStringTofixedPointaccepted a bare trailing decimal point ("1.") as a valid empty fraction, parsing it as the plain integer value. Per the org design ruling recorded in #24 (2026-07-04, review of PR #22): an empty fraction is INVALID — a trailing point with no fractional digits is a malformed literal, not UX generosity.The decimal-point gate now requires at least one digit after the point: after consuming the point and skipping fraction digits, finding the cursor still at the fraction start returns
ParseDecimalInvalidString.selector(the repo's existing malformed-input error, exactly as the ruling names it).Per the rework note on #22 ("land the parser fix + this suite together"), this branch is based on #22's head and carries its mutation-hardening suite unchanged except for the single
"1."acceptance pin, which is replaced with a revert pin on the exact selector. The fractional-overflow tests and the"1x"/"10x"decimal-point-gate tests are kept as-is. When this merges, #22's commits land with it.Closes #24
Behavior
Rejected now (
ParseDecimalInvalidString, selector0x3e8e62d6):"1.","0.","123.","00.","115792089237316195423570985008687907853269984665640564039457."— the whole category: any integer digits followed by a bare trailing point"1.e5"— scientific notation is unsupported here, so nothing after the bare point can rescue the empty fraction"."appendedUnchanged:
"1","1.0","0","0.0","123","123.0", and theuint256-max string still parse to the same values as before (pinned)"."and".5"still reject withParseEmptyDecimalString(empty integer part, rejected before the fraction is considered; their status is pinned, not changed)"1."pin from test: mutation-harden LibFixedPointDecimalParse coverage #22, replaced here.LibFixedPointDecimalFormat.fixedPointToDecimalStringonly emits a.when the fraction is nonzero with at least one nonzero digit after it, so thetestStringRoundTripFuzzround-trip is unaffected.Deploy pins
This repo has no generated pointer/deploy-constant files and nothing deployed (pure
internallibrary), so no pin regeneration and no redeploy is required.QA
testDecimalStringToFixedPointEmptyFraction,testDecimalStringToFixedPointEmptyFractionFuzz(uint256),testDecimalStringToFixedPointInvalidAfterInteger(holds the replaced"1."revert pin) — each fails on base (verified by stashing thesrc/change and running the suite against the unchanged parser: exactly these 3 failed with0x00000000… != 0x3e8e62d6…, base returning success whereParseDecimalInvalidStringis required; base suite itself green, 80/80)src/lib/parse/LibFixedPointDecimalParse.solempty-fraction guardif (cursor == fracStart)→if (false)(re-allow the empty fraction) → killed by all three discriminating tests above; guard restored and full suite re-run green1e18,123e18,type(uint256).max) derived from decimal semantics, never recomputed through the parser; the expected selector is the pre-existingParseDecimalInvalidStringerror named by the ruling in Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24"0.","1.","123.","00.", 60-digit integer +".","1.e5", and a fuzz over all non-overflowing integers +"."), pinned the exact selector, and the sweep found only the test: mutation-harden LibFixedPointDecimalParse coverage #22 pin (replaced). The issue's category note about sibling parsers in OTHER repos (rain.math.float, rainlang literal parsing) is outside this repo; Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24 itself scopes this PR toLibFixedPointDecimalParse.Test summary: base 80 passed / 0 failed; with fix 83 passed / 0 failed (3 new test functions);
forge fmtproduced no further changes.🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com