Skip to content

json: report integer duration overflow against time.Duration - #167

Open
sueun-dev wants to merge 1 commit into
segmentio:masterfrom
sueun-dev:fix-duration-overflow-type
Open

json: report integer duration overflow against time.Duration#167
sueun-dev wants to merge 1 commit into
segmentio:masterfrom
sueun-dev:fix-duration-overflow-type

Conversation

@sueun-dev

Copy link
Copy Markdown

decodeDuration parses integer durations with parseInt(b, durationType), but on error it returned inputError(b, int32Type) and then checked v < math.MinInt64 || v > math.MaxInt64. v is an int64, so that branch is unreachable (staticcheck SA4003, the lint reported in #150), and both error paths reported int32 instead of time.Duration.

Decoding an integer that overflows int64 into a time.Duration field showed this:

before: json: cannot unmarshal "100000000000000000000000}" into Go struct field ...D of type int32
after:  json: cannot unmarshal number 100000000000000000000000} overflows into Go struct field ...D of type time.Duration

time.Duration is backed by int64, so decodeDuration can propagate the parseInt error the same way decodeInt64 does, with no separate bounds check. Added a regression test; go test ./... passes and staticcheck no longer flags the line.

Fixes #150

decodeDuration parses integer durations with parseInt(b, durationType),
but on error it returned inputError(b, int32Type) and then checked
v < math.MinInt64 || v > math.MaxInt64. v is an int64, so that branch is
unreachable (staticcheck SA4003, the lint in segmentio#150), and both error paths
reported int32 instead of time.Duration.

time.Duration is backed by int64, so propagate the parseInt error the
same way decodeInt64 does. Add a regression test.

Fixes segmentio#150

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified on the exact base/head pair 7d5a25d / 52c82d2. Using the PR regression test as a test-only oracle, the base is RED because the overflow reports UnmarshalTypeError.Type == int32 and mentions int32; the head is GREEN and reports time.Duration. go test ./... -count=1 passes across the repository, and the diff check is clean.

The implementation is consistent with the existing integer decoders: parseInt already enforces the int64 bounds and constructs its overflow error from the supplied durationType, so propagating that error removes the unreachable range check and preserves the correct target type.

Disclosure: this review was prepared with Codex assistance; I independently verified the diff, the base/head oracle, and the test results.

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.

Lint about impossible condition

2 participants