Skip to content

Respect native int range when parsing JSON numbers - #100

Open
00200200 wants to merge 1 commit into
bitly:masterfrom
00200200:codex/int-native-range
Open

00200200 wants to merge 1 commit into
bitly:masterfrom
00200200:codex/int-native-range

Conversation

@00200200

Copy link
Copy Markdown

On 32-bit platforms, Int() parses a JSON number as int64 and then narrows it to int without checking its range. NewJson([]byte("2147483648")) followed by Int() returns -2147483648, nil; consequently MustInt(defaultValue) also treats the wrapped value as valid.

Parse json.Number using the native int width. Out-of-range values now return a range error and the corresponding int boundary, so MustInt can use its default. Tests cover both boundaries, positive/negative overflow, fractions, exponents, and MustInt defaults, with bounds derived from the platform int width.

Validation (Go 1.27.1):

  • Before the fix, both overflow regression subtests fail on Linux ARM 32-bit: 2147483648 becomes -2147483648 with no error, and -2147483649 becomes 2147483647 with no error.
  • After the fix, targeted tests and the complete suite pass on Linux ARM 32-bit in a local Docker container under QEMU with networking disabled; go vet ./... passes there.
  • On macOS arm64, targeted tests, go test -race -count=1 ./..., and go vet ./... pass. gofmt and git diff --check pass.

AI disclosure: prepared and checked with OpenAI Codex. No personal human review was performed. The 32-bit tests ran under emulation, not on physical ARM hardware; the race detector was run only on macOS arm64. No live services or downstream applications were tested.

@qodo-for-bitly

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review
Tip of the day
💡 Did you know, you can tweak Display settings with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗


Powered by Qodo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant