fix: parse signed hex and octal integers#2749
Open
StressTestor wants to merge 1 commit into
Open
Conversation
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.
Fixes #2748.
the problem
a
+or-sign before a hex or octal int makes yq error instead of producing the number:the value is tagged
!!intbutparseInt64(pkg/yqlib/lib.go) checks the0x/0oprefix against the whole string. a leading sign means the prefix doesn't match, so it falls through to a base-10ParseIntof"+0x12", which fails. it hits+0x12,-0x12,+0o22,-0o22.the fix
peel an optional leading
+/-off, detect the prefix on the remainder, and parsesign + digitswith the right base.ParseIntaccepts the sign for any base, so+0x12becomesParseInt("+12", 16, 64)= 18,-0x12= -18. the base-10 fallback is unchanged (it already handled signs).scope
format strings are left as-is (
0x%X/0o%o). switching to%#xwould lowercase hex digits and regress the uppercase-hex output thatoperator_value_test.goandtoml_test.gorely on (0x1A,0xDEADBEEF). one consequence: a negative hex/octal value used in arithmetic still renders with the sign inside the prefix (-0x12->0x-12), but that is pre-existing behaviour (yq '0x10 - 0x30'already yields0x-20on master) and the JSON path in this issue discards the format string, so the reported bug is fully fixed. tightening that render is a separate change.tests
parseInt64Scenariosgains the signed-positive cases (+0x12-> 18,+0o22-> 18), which also round-trip cleanly through the format string.encodescenarios injson_test.gocover the issue's exact path (yaml in, json out) for both signs and both bases:+0x12/-0x12->18/-18,+0o22/-0o22->18/-18.both fail on master and pass with the fix.
go test ./...,go vet,gofmt -s, andscripts/spelling.share clean; the changed files are lint-clean under the CI-pinned golangci-lint.