Require at least one digit in a number's exponent - #81
Merged
Merged
Conversation
The exponent loop could match nothing, so 1e scanned as a number. With UseJSONNumber the token is returned before the ParseFloat check, so the decoder accepted a value its own encoder had already decided was a quoteless string.
Member
|
Looks good, thanks for the PR! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
hjson-go cannot read back what it just wrote for a string like
1e.Same for
1E,1e+,1.2e,-1e.The two halves disagree
The exponent scan in
tryParseNumber(parseNumber.go:77) allows the digit loop to match nothing:That is harmless when the caller reaches
strconv.ParseFloatatparseNumber.go:108, whichrejects
1e. But when the destination makes the decoder usejson.Number— anymap[string]interface{}orinterface{}target —parseNumber.go:105returns first:so
json.Number("1e")escapes unvalidated and the laterjson.Marshalfails withInternal error.The encoder already gets this right.
startsWithNumber(encode.go:137) calls the samefunction as
tryParseNumber(text, true, false)— withuseJSONNumberfalse it hitsParseFloat,correctly concludes
1eis not a number, and writes it quoteless. So the two call sites of onefunction disagree about whether
1eis a number, and the decoder is the one that is wrong.The sibling implementations agree
hjson-js behaves the same way. This change moves hjson-go onto that behaviour rather than away
from it.
Change
Four lines at the scan site — an exponent needs at least one digit:
Nothing about the numeric range is touched, so
UseJSONNumber's large-exponent preservation isunaffected;
1e5still decodes to100000.Verification
A test added next to the others covering the five inputs above plus the
1e5control. Red withonly
parseNumber.goreverted ("1e": Internal error), green with the change.go test ./...passes before and after;
gofmt -lclean.Found by round-tripping 200,000 generated strings through
Marshal→Unmarshal; the 106Internal erroroccurrences all reduce to this shape, and go to 0 with the change.Behaviour change:
Unmarshalof{k: 1e}into a map goes from returning an error to yieldingthe string
"1e". That only affects callers relying on the error, and it is what the otherimplementations already do.
Disclosure: prepared with AI assistance; I verified the round trip, the hjson-py comparison and the
red/green runs myself.