Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parseInt overflow check false negative #231

Merged
merged 1 commit into from
Jun 20, 2021

Conversation

carsonip
Copy link
Contributor

@carsonip carsonip commented May 11, 2021

Description: The current parseInt overflow check may return false negative for input like 27670116110564327410. This patch fixes the overflow checking.

Benchmark before change:

goos: darwin
goarch: amd64
pkg: benchmarks
BenchmarkJsonParserLarge
BenchmarkJsonParserLarge-12                               124323             46955 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserMedium
BenchmarkJsonParserMedium-12                              771728              7705 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserDeleteMedium
BenchmarkJsonParserDeleteMedium-12                        728377              8323 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualMedium
BenchmarkJsonParserEachKeyManualMedium-12                1000000              5590 ns/op             112 B/op          2 allocs/op
BenchmarkJsonParserEachKeyStructMedium
BenchmarkJsonParserEachKeyStructMedium-12                 965860              6122 ns/op             560 B/op         12 allocs/op
BenchmarkJsonParserObjectEachStructMedium
BenchmarkJsonParserObjectEachStructMedium-12              770988              7783 ns/op             512 B/op         11 allocs/op
BenchmarkJsonParserSmall
BenchmarkJsonParserSmall-12                              8176395               733 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualSmall
BenchmarkJsonParserEachKeyManualSmall-12                 9562789               629 ns/op              80 B/op          2 allocs/op
BenchmarkJsonParserEachKeyStructSmall
BenchmarkJsonParserEachKeyStructSmall-12                 7560410               795 ns/op             192 B/op          8 allocs/op
BenchmarkJsonParserObjectEachStructSmall
BenchmarkJsonParserObjectEachStructSmall-12              9493182               630 ns/op             176 B/op          7 allocs/op
BenchmarkJsonParserSetSmall
BenchmarkJsonParserSetSmall-12                           6147346               976 ns/op             768 B/op          4 allocs/op
BenchmarkJsonParserDelSmall
BenchmarkJsonParserDelSmall-12                           4584642              1307 ns/op               0 B/op          0 allocs/op
PASS
ok      benchmarks      77.529s

Benchmark after change:

goos: darwin
goarch: amd64
pkg: benchmarks
BenchmarkJsonParserLarge
BenchmarkJsonParserLarge-12                               126126             47046 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserMedium
BenchmarkJsonParserMedium-12                              773224              7708 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserDeleteMedium
BenchmarkJsonParserDeleteMedium-12                        689145              8246 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualMedium
BenchmarkJsonParserEachKeyManualMedium-12                1000000              5597 ns/op             112 B/op          2 allocs/op
BenchmarkJsonParserEachKeyStructMedium
BenchmarkJsonParserEachKeyStructMedium-12                 964692              6189 ns/op             560 B/op         12 allocs/op
BenchmarkJsonParserObjectEachStructMedium
BenchmarkJsonParserObjectEachStructMedium-12              761317              7813 ns/op             512 B/op         11 allocs/op
BenchmarkJsonParserSmall
BenchmarkJsonParserSmall-12                              8193255               731 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualSmall
BenchmarkJsonParserEachKeyManualSmall-12                 9560017               627 ns/op              80 B/op          2 allocs/op
BenchmarkJsonParserEachKeyStructSmall
BenchmarkJsonParserEachKeyStructSmall-12                 7580191               795 ns/op             192 B/op          8 allocs/op
BenchmarkJsonParserObjectEachStructSmall
BenchmarkJsonParserObjectEachStructSmall-12              9382509               634 ns/op             176 B/op          7 allocs/op
BenchmarkJsonParserSetSmall
BenchmarkJsonParserSetSmall-12                           6122930               979 ns/op             768 B/op          4 allocs/op
BenchmarkJsonParserDelSmall
BenchmarkJsonParserDelSmall-12                           4542772              1310 ns/op               0 B/op          0 allocs/op
PASS
ok      benchmarks      77.334s

parseInt benchmark before:

goos: darwin
goarch: amd64
pkg: github.com/buger/jsonparser
BenchmarkParseInt
BenchmarkParseInt-12                	173004537	         6.85 ns/op
BenchmarkParseIntUnsafeSlower
BenchmarkParseIntUnsafeSlower-12    	67225647	        17.7 ns/op
BenchmarkParseIntOverflows
BenchmarkParseIntOverflows-12       	221040992	         5.37 ns/op
PASS

parseInt benchmark after:

goos: darwin
goarch: amd64
pkg: github.com/buger/jsonparser
BenchmarkParseInt
BenchmarkParseInt-12                	178624494	         6.70 ns/op
BenchmarkParseIntUnsafeSlower
BenchmarkParseIntUnsafeSlower-12    	65089550	        18.2 ns/op
BenchmarkParseIntOverflows
BenchmarkParseIntOverflows-12       	201364219	         5.93 ns/op
PASS

@carsonip carsonip force-pushed the fix-parseint-overflow-check branch from 12fee25 to fc8b497 Compare May 11, 2021 17:18
@carsonip carsonip force-pushed the fix-parseint-overflow-check branch from fc8b497 to 608d8c5 Compare May 11, 2021 17:33
@buger buger merged commit 2d9d634 into buger:master Jun 20, 2021
@buger
Copy link
Owner

buger commented Jun 20, 2021

👌🏻

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