Skip to content

Commit 6acdf74

Browse files
jstroembuger
authored andcommitted
Implemented overflow detection in the current parseInt implementation.
Benchmarks shows this is ~1.5ns slower than the current impl: goos: darwin goarch: amd64 pkg: github.com/jstroem/jsonparser BenchmarkParseInt-8 200000000 7.35 ns/op 0 B/op 0 allocs/op BenchmarkParseIntUnsafeSlower-8 100000000 13.7 ns/op 0 B/op 0 allocs/op BenchmarkParseIntOverflows-8 200000000 5.97 ns/op 0 B/op 0 allocs/op BenchmarkEqualStr-8 500000000 3.27 ns/op 0 B/op 0 allocs/op BenchmarkBytesEqualStrSafe-8 500000000 3.29 ns/op 0 B/op 0 allocs/op BenchmarkBytesEqualStrUnsafeSlower-8 500000000 3.72 ns/op 0 B/op 0 allocs/op PASS ok github.com/jstroem/jsonparser 11.808s
1 parent f4dd9f5 commit 6acdf74

File tree

4 files changed

+109
-18
lines changed

4 files changed

+109
-18
lines changed

bytes.go

+26-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
package jsonparser
22

3-
// About 3x faster then strconv.ParseInt because does not check for range error and support only base 10, which is enough for JSON
4-
func parseInt(bytes []byte) (v int64, ok bool) {
3+
import (
4+
bio "bytes"
5+
)
6+
7+
// minInt64 '-9223372036854775808' is the smallest representable number in int64
8+
const minInt64 = `9223372036854775808`
9+
10+
// About 2x faster then strconv.ParseInt because it only supports base 10, which is enough for JSON
11+
func parseInt(bytes []byte) (v int64, ok bool, overflow bool) {
512
if len(bytes) == 0 {
6-
return 0, false
13+
return 0, false, false
714
}
815

916
var neg bool = false
@@ -12,17 +19,29 @@ func parseInt(bytes []byte) (v int64, ok bool) {
1219
bytes = bytes[1:]
1320
}
1421

22+
var b int64 = 0
1523
for _, c := range bytes {
1624
if c >= '0' && c <= '9' {
17-
v = (10 * v) + int64(c-'0')
25+
b = (10 * v) + int64(c-'0')
1826
} else {
19-
return 0, false
27+
return 0, false, false
28+
}
29+
if overflow = (b < v); overflow {
30+
break
31+
}
32+
v = b
33+
}
34+
35+
if overflow {
36+
if neg && bio.Equal(bytes, []byte(minInt64)) {
37+
return b, true, false
2038
}
39+
return 0, false, true
2140
}
2241

2342
if neg {
24-
return -v, true
43+
return -v, true, false
2544
} else {
26-
return v, true
45+
return v, true, false
2746
}
2847
}

bytes_test.go

+66-9
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import (
77
)
88

99
type ParseIntTest struct {
10-
in string
11-
out int64
12-
isErr bool
10+
in string
11+
out int64
12+
isErr bool
13+
isOverflow bool
1314
}
1415

1516
var parseIntTests = []ParseIntTest{
@@ -34,18 +35,37 @@ var parseIntTests = []ParseIntTest{
3435
out: -12345,
3536
},
3637
{
37-
in: "9223372036854775807",
38+
in: "9223372036854775807", // = math.MaxInt64
3839
out: 9223372036854775807,
3940
},
4041
{
41-
in: "-9223372036854775808",
42+
in: "-9223372036854775808", // = math.MinInt64
4243
out: -9223372036854775808,
4344
},
4445
{
45-
in: "18446744073709551616", // = 2^64; integer overflow is not detected
46-
out: 0,
46+
in: "-92233720368547758081",
47+
out: 0,
48+
isErr: true,
49+
isOverflow: true,
50+
},
51+
{
52+
in: "18446744073709551616", // = 2^64
53+
out: 0,
54+
isErr: true,
55+
isOverflow: true,
56+
},
57+
{
58+
in: "9223372036854775808", // = math.MaxInt64 - 1
59+
out: 0,
60+
isErr: true,
61+
isOverflow: true,
62+
},
63+
{
64+
in: "-9223372036854775809", // = math.MaxInt64 - 1
65+
out: 0,
66+
isErr: true,
67+
isOverflow: true,
4768
},
48-
4969
{
5070
in: "",
5171
isErr: true,
@@ -70,7 +90,10 @@ var parseIntTests = []ParseIntTest{
7090

7191
func TestBytesParseInt(t *testing.T) {
7292
for _, test := range parseIntTests {
73-
out, ok := parseInt([]byte(test.in))
93+
out, ok, overflow := parseInt([]byte(test.in))
94+
if overflow != test.isOverflow {
95+
t.Errorf("Test '%s' error return did not overflow expectation (obtained %t, expected %t)", test.in, overflow, test.isOverflow)
96+
}
7497
if ok != !test.isErr {
7598
t.Errorf("Test '%s' error return did not match expectation (obtained %t, expected %t)", test.in, !ok, test.isErr)
7699
} else if ok && out != test.out {
@@ -93,3 +116,37 @@ func BenchmarkParseIntUnsafeSlower(b *testing.B) {
93116
strconv.ParseInt(*(*string)(unsafe.Pointer(&bytes)), 10, 64)
94117
}
95118
}
119+
120+
// Old implementation that did not check for overflows.
121+
func BenchmarkParseIntOverflows(b *testing.B) {
122+
bytes := []byte("123")
123+
for i := 0; i < b.N; i++ {
124+
parseIntOverflows(bytes)
125+
}
126+
}
127+
128+
func parseIntOverflows(bytes []byte) (v int64, ok bool) {
129+
if len(bytes) == 0 {
130+
return 0, false
131+
}
132+
133+
var neg bool = false
134+
if bytes[0] == '-' {
135+
neg = true
136+
bytes = bytes[1:]
137+
}
138+
139+
for _, c := range bytes {
140+
if c >= '0' && c <= '9' {
141+
v = (10 * v) + int64(c-'0')
142+
} else {
143+
return 0, false
144+
}
145+
}
146+
147+
if neg {
148+
return -v, true
149+
} else {
150+
return v, true
151+
}
152+
}

parser.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ var (
1717
MalformedArrayError = errors.New("Value is array, but can't find closing ']' symbol")
1818
MalformedObjectError = errors.New("Value looks like object, but can't find closing '}' symbol")
1919
MalformedValueError = errors.New("Value looks like Number/Boolean/None, but can't find its end: ',' or '}' symbol")
20+
OverflowIntegerError = errors.New("Value is number, but overflowed while parsing")
2021
MalformedStringEscapeError = errors.New("Encountered an invalid escape sequence in a string")
2122
)
2223

@@ -1183,7 +1184,10 @@ func ParseFloat(b []byte) (float64, error) {
11831184

11841185
// ParseInt parses a Number ValueType into a Go int64
11851186
func ParseInt(b []byte) (int64, error) {
1186-
if v, ok := parseInt(b); !ok {
1187+
if v, ok, overflow := parseInt(b); !ok {
1188+
if overflow {
1189+
return 0, OverflowIntegerError
1190+
}
11871191
return 0, MalformedValueError
11881192
} else {
11891193
return v, nil

parser_test.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,6 @@ var getTests = []GetTest{
734734
isFound: true,
735735
data: "c",
736736
},
737-
738737
// Array index paths
739738
{
740739
desc: "last key in path is index",
@@ -814,6 +813,18 @@ var getIntTests = []GetTest{
814813
isFound: true,
815814
data: int64(1),
816815
},
816+
{ // Issue #138: overflow detection
817+
desc: `Fails because of overflow`,
818+
json: `{"p":9223372036854775808}`,
819+
path: []string{"p"},
820+
isErr: true,
821+
},
822+
{ // Issue #138: overflow detection
823+
desc: `Fails because of underflow`,
824+
json: `{"p":-9223372036854775809}`,
825+
path: []string{"p"},
826+
isErr: true,
827+
},
817828
}
818829

819830
var getFloatTests = []GetTest{

0 commit comments

Comments
 (0)