Skip to content

fix(read): don't return the clamped value for an out of range int64 - #233

Merged
inhere merged 1 commit into
gookit:masterfrom
youdie006:int64-overflow
Oct 28, 2026
Merged

inhere merged 1 commit into
gookit:masterfrom
youdie006:int64-overflow

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

tryInt64 records the strconv.ParseInt range error but keeps the clamped value, so an out of range string comes back as MaxInt64/MinInt64:

c.Set("value", "9223372036854775808")
c.Int64("value") // before: 9223372036854775807   after: 0 (error recorded either way)
c.Int("value")   // before: 9223372036854775807 on 64-bit, 0 on 32-bit   after: 0

tryUint already resets the value to 0 on error since 87608e8 ("don't return the clamped max value"), and Int returns 0 when the value is outside the int range on 32-bit, so this does the same in tryInt64. In-range values and non-numeric strings (which already gave 0) are unchanged.

TestInt64OutOfRange covers both ends and fails without the change. go test -race ./... (with NAME unset), GOARCH=386 go test -run 'TestInt|TestUint' and go vet pass locally.

Written with AI assistance (Claude); I have reviewed the change.

tryInt64 recorded the ParseInt range error but returned the clamped
MaxInt64/MinInt64, so Int64 and (on 64-bit) Int returned it, while Uint
resets the value to 0 since 87608e8 and Int returns 0 on 32-bit.

@Ekko82 Ekko82 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go test . -run '^Test(Int|Uint)' -count=1 passes at c9b0427 on Windows with Go 1.27.1 for both amd64 and 386. This includes the full-range Int64 regressions from #228, so valid values beyond the 32-bit int range remain intact.

I also ran the new overflow test against the base read.go on amd64: both cases fail with the clamped extrema. They pass at this head, returning zero and recording the parse error. Local validation covered the numeric-reader tests; I did not run the full suite.

@inhere
inhere merged commit 4c215a7 into gookit:master Oct 28, 2026
12 checks passed
inhere added a commit that referenced this pull request Oct 28, 2026
Float returned the clamped +Inf/-Inf of strconv.ParseFloat with the range error
recorded, while tryInt64 (#233) and tryUint reset the value to 0 and Int returns 0
on 32-bit. Do the same here; a non-numeric value was already 0.
@inhere

inhere commented Oct 28, 2026

Copy link
Copy Markdown
Member

Landed as merge commit \4c215a7. The test insertion conflicted with \5e44dc9, which added \TestIntsIntMapParseEnv\ right after \TestInt64FullRange; the merge keeps both tests and the suite is green (amd64 full suite + GOARCH=386 numeric tests). Thanks for the fix, and to @Ekko82 for the Windows/386 verification.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants