Repository navigation
Description
Describe the bug
toInt in pkg/github/params.go is documented to reject "values outside the int range", but its upper bound check compares against math.MaxInt as a float:
if f > math.MaxInt || f < math.MinInt {
return 0, fmt.Errorf("numeric value out of int range: %v", f)
}
return int(f), nilmath.MaxInt (2^63 - 1) cannot be represented as a float64 and rounds up to exactly 2^63. A value of 2^63 is therefore not greater than the bound, passes the check, and int(f) on an out-of-range float is implementation-defined in Go. On amd64 it produces math.MinInt (-9223372036854775808).
Any value that parses to 2^63, including the integer 9223372036854775807 itself, is accepted without an error and becomes a large negative number. This affects every tool that reads integers through RequiredInt, OptionalIntParam, OptionalIntParamWithDefault and OptionalPaginationParams (for example issue_number, pull_number, page, perPage). toInt64 is not affected because it uses a round-trip check.
Affected version
main at 07e2466, built from source (Go toolchain from go.mod, linux/amd64).
Steps to reproduce the behavior
-
Add this test to
pkg/github:package github import "testing" func TestToIntUpperBound(t *testing.T) { for _, v := range []any{"9223372036854775807", float64(9223372036854775807), "9223372036854775808", "1e19"} { n, err := OptionalIntParam(map[string]any{"pull_number": v}, "pull_number") t.Logf("%v -> %d, err=%v", v, n, err) } }
-
Run
go test ./pkg/github -run TestToIntUpperBound -v.
Expected vs actual behavior
Expected: every value at or above 2^63 is rejected with numeric value out of int range, as 1e19 already is.
Actual:
9223372036854775807 -> -9223372036854775808, err=<nil>
9.223372036854776e+18 -> -9223372036854775808, err=<nil>
9223372036854775808 -> -9223372036854775808, err=<nil>
1e19 -> 0, err=parameter pull_number is not a valid number: numeric value out of int range: 1e+19
The negative value is then sent on to the GitHub API (for example as a pull request number or page) instead of being rejected locally with a clear parameter error.
Logs
Not applicable; the output above is from the test.
Possible Solution
- Make the upper bound exclusive at 2^63 (
f >= -float64(math.MinInt)), or reuse the round-trip check thattoInt64already performs. - Add the boundary values above to the existing
toInttests.
if f >= -float64(math.MinInt) || f < float64(math.MinInt) {
return 0, fmt.Errorf("numeric value out of int range: %v", f)
}
return int(f), nilI'm happy to send a PR.
Activity
fix: reject integer parameters at or above 2^63 (github#3650)