Skip to content

fix(decode): handle json.Number pointers - #215

Open
terminalchai wants to merge 1 commit into
go-viper:mainfrom
terminalchai:fix/json-number-pointer
Open

terminalchai wants to merge 1 commit into
go-viper:mainfrom
terminalchai:fix/json-number-pointer

Conversation

@terminalchai

Copy link
Copy Markdown

Decoding a *json.Number into an integer, unsigned integer, or float currently panics with interface conversion: interface {} is *json.Number, not json.Number. For example, n := json.Number("42"); var result int64; mapstructure.Decode(&n, &result) panics instead of decoding 42.

Use the already dereferenced input value in all three numeric decoders. This preserves the existing parsers and error wrapping. A focused table covers the three successful conversions and malformed-number errors.

The full race-enabled, shuffled test suite, vet, build, and CI-pinned golangci-lint v2.6.2 pass locally. Lint ran on an LF validation copy to match CI checkout formatting.

AI assistance was used for implementation, regression testing, and independent automated specification and code reviews.

@Ekko82 Ekko82 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.

Checked 310c79586b3044981e72703b4a8c8f3bccc223b2 with Go 1.27.1 on Windows/amd64 and Windows/386. All six added subtests pass on each architecture.

Of 17 additional public-API probes per architecture, 14 pass. Six single-pointer cases that panic at base 52aa5c6dc1d27226460807054ca2107b2d54fb2d now produce the expected values or ParseError, including int64 minimum, uint64 maximum, malformed numbers and negative-to-unsigned input. Both direct-value controls keep their results. Six typed-nil controls also preserve behavior with ZeroFields on and off. Three **json.Number cases remain rejected as unconvertible on both revisions.

I did not run the full suite, race detector, vet, build or lint.

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.

2 participants