Skip to content

Commit

Permalink
encoding/json: remove unnecessary isValidNumber call
Browse files Browse the repository at this point in the history
The decoder called this function to check numbers being decoded into a
json.Number. However, these can't be quoted as strings, so the tokenizer
has already verified they are valid JSON numbers.

Verified this by adding a test with such an input. As expected, it
produces a syntax error, not the fmt.Errorf - that line could never
execute.

Since the only remaining non-test caller of isvalidnumber is in
encode.go, move the function there.

This change should slightly reduce the amount of work when decoding into
json.Number, though that isn't very common nor part of any current
benchmarks.

Change-Id: I67a1723deb3d18d5b542d6dd35f3ae56a43f23eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/184817
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
mvdan committed Aug 27, 2019
1 parent 49f0431 commit e414c37
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 63 deletions.
65 changes: 2 additions & 63 deletions decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,66 +199,6 @@ func (n Number) Int64() (int64, error) {
return strconv.ParseInt(string(n), 10, 64)
}

// isValidNumber reports whether s is a valid JSON number literal.
func isValidNumber(s string) bool {
// This function implements the JSON numbers grammar.
// See https://tools.ietf.org/html/rfc7159#section-6
// and https://json.org/number.gif

if s == "" {
return false
}

// Optional -
if s[0] == '-' {
s = s[1:]
if s == "" {
return false
}
}

// Digits
switch {
default:
return false

case s[0] == '0':
s = s[1:]

case '1' <= s[0] && s[0] <= '9':
s = s[1:]
for len(s) > 0 && '0' <= s[0] && s[0] <= '9' {
s = s[1:]
}
}

// . followed by 1 or more digits.
if len(s) >= 2 && s[0] == '.' && '0' <= s[1] && s[1] <= '9' {
s = s[2:]
for len(s) > 0 && '0' <= s[0] && s[0] <= '9' {
s = s[1:]
}
}

// e or E followed by an optional - or + and
// 1 or more digits.
if len(s) >= 2 && (s[0] == 'e' || s[0] == 'E') {
s = s[1:]
if s[0] == '+' || s[0] == '-' {
s = s[1:]
if s == "" {
return false
}
}
for len(s) > 0 && '0' <= s[0] && s[0] <= '9' {
s = s[1:]
}
}

// Make sure we are at the end.
return s == ""
}

// decodeState represents the state while decoding a JSON value.
type decodeState struct {
data []byte
Expand Down Expand Up @@ -1027,10 +967,9 @@ func (d *decodeState) literalStore(item []byte, v reflect.Value, fromQuoted bool
switch v.Kind() {
default:
if v.Kind() == reflect.String && v.Type() == numberType {
// s must be a valid number, because it's
// already been tokenized.
v.SetString(s)
if !isValidNumber(s) {
return fmt.Errorf("json: invalid number literal, trying to unmarshal %q into Number", item)
}
break
}
if fromQuoted {
Expand Down
1 change: 1 addition & 0 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ var unmarshalTests = []unmarshalTest{
{in: `[1, 2, 3+]`, err: &SyntaxError{"invalid character '+' after array element", 9}},
{in: `{"X":12x}`, err: &SyntaxError{"invalid character 'x' after object key:value pair", 8}, useNumber: true},
{in: `[2, 3`, err: &SyntaxError{msg: "unexpected end of JSON input", Offset: 5}},
{in: `{"F3": -}`, ptr: new(V), out: V{F3: Number("-")}, err: &SyntaxError{msg: "invalid character '}' in numeric literal", Offset: 9}},

// raw value errors
{in: "\x01 42", err: &SyntaxError{"invalid character '\\x01' looking for beginning of value", 1}},
Expand Down
60 changes: 60 additions & 0 deletions encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,66 @@ func stringEncoder(e *encodeState, v reflect.Value, opts encOpts) {
}
}

// isValidNumber reports whether s is a valid JSON number literal.
func isValidNumber(s string) bool {
// This function implements the JSON numbers grammar.
// See https://tools.ietf.org/html/rfc7159#section-6
// and https://json.org/number.gif

if s == "" {
return false
}

// Optional -
if s[0] == '-' {
s = s[1:]
if s == "" {
return false
}
}

// Digits
switch {
default:
return false

case s[0] == '0':
s = s[1:]

case '1' <= s[0] && s[0] <= '9':
s = s[1:]
for len(s) > 0 && '0' <= s[0] && s[0] <= '9' {
s = s[1:]
}
}

// . followed by 1 or more digits.
if len(s) >= 2 && s[0] == '.' && '0' <= s[1] && s[1] <= '9' {
s = s[2:]
for len(s) > 0 && '0' <= s[0] && s[0] <= '9' {
s = s[1:]
}
}

// e or E followed by an optional - or + and
// 1 or more digits.
if len(s) >= 2 && (s[0] == 'e' || s[0] == 'E') {
s = s[1:]
if s[0] == '+' || s[0] == '-' {
s = s[1:]
if s == "" {
return false
}
}
for len(s) > 0 && '0' <= s[0] && s[0] <= '9' {
s = s[1:]
}
}

// Make sure we are at the end.
return s == ""
}

func interfaceEncoder(e *encodeState, v reflect.Value, opts encOpts) {
if v.IsNil() {
e.WriteString("null")
Expand Down

0 comments on commit e414c37

Please sign in to comment.