json: report integer duration overflow against time.Duration - #167
json: report integer duration overflow against time.Duration#167sueun-dev wants to merge 1 commit into
Conversation
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
left a comment
There was a problem hiding this comment.
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.
decodeDurationparses integer durations withparseInt(b, durationType), but on error it returnedinputError(b, int32Type)and then checkedv < math.MinInt64 || v > math.MaxInt64.vis anint64, so that branch is unreachable (staticcheck SA4003, the lint reported in #150), and both error paths reportedint32instead oftime.Duration.Decoding an integer that overflows int64 into a
time.Durationfield showed this:time.Durationis backed byint64, sodecodeDurationcan propagate theparseInterror the same waydecodeInt64does, with no separate bounds check. Added a regression test;go test ./...passes and staticcheck no longer flags the line.Fixes #150