Repository navigation
fix(mcpx): refuse non-finite ParseBytes spellings explicitly (#3521) - #4788
Conversation
ParseBytes refused NaN/Inf only incidentally (float-to-int conversion for NaN, magnitude check for infinities). Add an explicit math.IsNaN/IsInf guard naming the offending value, and name the non-finite spellings (bare and suffixed) in the refusal test table and the Validate loop so a future float-side refactor regresses loudly.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 17 minutes. View limit details
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. CodeRabbit Verify
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Residual of #3521 after triage task 5496 (queue task 6147).
Problem:
ParseBytesrefused non-finite spellings only incidentally — NaN via implementation-defined float→int conversion landing in then<0check, infinities via the 2^63 magnitude check. No regression test named a non-finite spelling, so a future refactor to float-side comparisons (which NaN defeats) would regress silently.Change (pinned scope, no finite-behavior change):
pkgs/mcpx/internal/settings/value.go: explicitmath.IsNaN/math.IsInfguard on the scaled value, errorsize must be finite: %qnaming the offending value (same style as the neighboring refusals); doc comment notes mcpx(settings): ParseBytes accepts non-finite spellings — NaN/Inf (bare and suffixed) validate as MinInt64, a residual #1697's sign/range checks do not catch #3521.pkgs/mcpx/internal/settings/registry_test.go:TestParseBytesRefusesNegativeAndOverflowingSizesrefusal table + theValidateloop extended withNaN, nan, Inf, +Inf, -Inf, Infinity, NaNMB, InfGB.Tests:
TestParseBytesRefusesNegativeAndOverflowingSizes,TestByteSizesAcceptBothDecimalAndBinarySuffixes— PASS../internal/settings/: only 5 failures, all proven pre-existing on pristineorigin/main(TestEveryDocumentHasAChapter,TestNoUndeclaredEnvRead,TestEverySettingHasAnEffectProbeOrAnExemption,TestTheScriptEnvironmentIsWhatTheCodeSets,TestTheEnvironmentDocIsCurrent) — unrelated to this change.Closes #3521.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.