Repository navigation
🐛 bug: reject out-of-range int and uint in generic parsers on 32-bit - #4832
Conversation
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4832 +/- ##
==========================================
+ Coverage 95.96% 95.99% +0.02%
==========================================
Files 153 153
Lines 19707 19707
==========================================
+ Hits 18912 18917 +5
+ Misses 484 479 -5
Partials 311 311
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
On 386 and arm, genericParseType parsed int and uint at 64 bits and converted the result, so Query[int], Params[int] and GetReqHeader[int] returned a wrapped value for a number that does not fit instead of the default value. Test_GenericParseTypeInts and Test_GenericParseTypeUints already expect the error and fail with GOARCH=386. Co-Authored-By: Claude Opus 5.5 <[email protected]>
64c5398 to
c5a9a34
Compare
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Integer conversion checks helpers.go |
int and uint parsing now returns errParseValue if conversion to the platform-sized type and back changes the value. Parse errors continue to return the same error. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Merge Risk: 
c5a9a
-
The range checks address the reported 32-bit parsing issue without changing valid 64-bit conversions.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the main change: rejecting out-of-range int and uint values in generic parsers on 32-bit platforms. Description check The description clearly explains the overflow bug, affected APIs, implementation, test results, and selected change type. Some template items remain incomplete, including the issue reference, change-d… Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description clearly explains the overflow bug, affected APIs, implementation, test results, and selected change type. Some template items remain incomplete, including the issue reference, change-detail checkboxes, and checklist confirmations, but the core description is complete and relevant.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
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.
I’m a rabbit, ears upright,
I check each number fits just right.
If round trips change what values say,
I send errParseValue on its way.
Then hop along beneath the moon,
With integers checked in tune.
|
Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
Description
On 32-bit platforms (386, arm)
genericParseTypeparsesintanduintwith the 64-bitutils.ParseInt/utils.ParseUintand then converts, so a value that doesn't fit wraps instead of failing. For examplefiber.Query[int](c, "n", 10)with?n=2147483648returns-2147483648instead of10;Params[int]andGetReqHeader[int]do the same. This adds a range check after parsing, which does nothing on 64-bit. I keptutils.ParseIntbecause of the speed note inconstraint.go.Test_GenericParseTypeIntsandTest_GenericParseTypeUintsalready expect the error whenintis 32 bits, but CI has no 32-bit target. WithGOARCH=386they fail before this change (Expected error with "failed to parse value" in chain but got nil.) and pass after.go test .passes on 386 and amd64 (Go 1.27.3), andgolangci-lint run --new-from-rev=HEAD~1 .(v2.14.0) reports 0 issues.Changes introduced
Type of change
Checklist
/docs/directory for Fiber's documentation.I used Claude Code to write this change.
🤖 Maintainers:
/generatere-runs the code generators on this branch