diff --git a/jreader/token_reader_default.go b/jreader/token_reader_default.go index 446bc09..e60eba3 100644 --- a/jreader/token_reader_default.go +++ b/jreader/token_reader_default.go @@ -8,6 +8,7 @@ import ( "io" "strconv" "unicode" + "unicode/utf16" "unicode/utf8" ) @@ -367,7 +368,9 @@ func (r *tokenReader) skipWhitespaceAndReadByte() (byte, bool) { if !ok { return 0, false } - if !unicode.IsSpace(rune(ch)) { + // JSON permits only these four whitespace characters between tokens. Any other + // character (including other Unicode spaces) is the start of a token. + if ch != ' ' && ch != '\t' && ch != '\n' && ch != '\r' { r.lastPos = r.pos - 1 return ch, true } @@ -390,67 +393,96 @@ func (r *tokenReader) consumeASCIILowercaseAlphabeticChars() int { return n } -func (r *tokenReader) readNumber(_ byte) (float64, bool) { - startPos := r.lastPos +func isDigit(b byte) bool { + return b >= '0' && b <= '9' +} + +// consumeDigits advances past any decimal digits in data starting at index i, returning the +// index of the first non-digit (or the end of the input). +func consumeDigits(data []byte, i, n int) int { + for i < n && isDigit(data[i]) { + i++ + } + return i +} + +func (r *tokenReader) readNumber(first byte) (float64, bool) { + // The grammar is: [ - ] int [ frac ] [ exp ], where int is a single 0 or a 1-9 digit + // followed by more digits, frac is '.' followed by at least one digit, and exp is + // [eE][-+]? followed by at least one digit. We scan the input directly by index rather + // than through readByte/unreadByte, then leave r.pos pointing just past the number. + start := r.lastPos + data, n := r.data, r.len + i := start + 1 // the first byte has already been read isFloat := false - var ch byte - var ok bool - for { - ch, ok = r.readByte() - if !ok { - break - } - if (ch < '0' || ch > '9') && (ch != '.' || isFloat) { - break - } - if ch == '.' { - isFloat = true + + // Optional minus sign, then the first digit of the integer part. + if first == '-' { + if i >= n || !isDigit(data[i]) { + return 0, false } + first = data[i] + i++ } - hasExponent := false - if ch == 'e' || ch == 'E' { - // exponent must match this regex: [eE][-+]?[0-9]+ - ch, ok = r.readByte() - if !ok { - return 0, false + + // Integer part. + if first == '0' { + if i < n && isDigit(data[i]) { + return 0, false // a leading zero cannot be followed by another digit } - if ch == '+' || ch == '-' { //nolint:gocritic,revive - } else if ch >= '0' && ch <= '9' { - r.unreadByte() - } else { + } else { + i = consumeDigits(data, i, n) + } + + // Fractional part: a decimal point must be followed by at least one digit. + if i < n && data[i] == '.' { + isFloat = true + i++ + if i >= n || !isDigit(data[i]) { return 0, false } - for { - ch, ok = r.readByte() - if !ok { - break - } - if ch < '0' || ch > '9' { - r.unreadByte() - break - } - hasExponent = true + i = consumeDigits(data, i, n) + } + + // Exponent part: [eE][-+]? followed by at least one digit. + if i < n && (data[i] == 'e' || data[i] == 'E') { + isFloat = true + i++ + if i < n && (data[i] == '+' || data[i] == '-') { + i++ } - if !hasExponent { + if i >= n || !isDigit(data[i]) { return 0, false } - isFloat = true - } else { //nolint:gocritic - if ok { - r.unreadByte() + i = consumeDigits(data, i, n) + } + + r.pos = i + chars := data[start:i] + if !isFloat { + if num, ok := parseIntFromBytes(chars); ok { + return float64(num), true } + // The integer literal overflows int64. Fall through to float parsing, which yields the + // same value encoding/json produces for magnitudes within float64 range (and rejects + // those beyond it, consistent with out-of-range float literals). } - chars := r.data[startPos:r.pos] - if isFloat { - // Unfortunately, strconv.ParseFloat requires a string - there is no []byte equivalent. This means we can't - // avoid a heap allocation here. Easyjson works around this by creating an unsafe string that points directly - // at the existing bytes, but in our default implementation we can't use unsafe. - n, err := strconv.ParseFloat(string(chars), 64) - return n, err == nil - } else { //nolint:revive - n, ok := parseIntFromBytes(chars) - return float64(n), ok + // Unfortunately, strconv.ParseFloat requires a string - there is no []byte equivalent. This means we can't + // avoid a heap allocation here. Easyjson works around this by creating an unsafe string that points directly + // at the existing bytes, but in our default implementation we can't use unsafe. + num, err := strconv.ParseFloat(string(chars), 64) + return num, err == nil +} + +// beginEscapedCopy switches readString off its zero-copy fast path by copying the literal +// prefix [startPos:endPos) that has been validated so far into a fresh buffer with headroom +// for the decoded remainder. +func beginEscapedCopy(data []byte, startPos, endPos int) []byte { + buf := make([]byte, endPos-startPos, endPos-startPos+20) + if endPos > startPos { + copy(buf, data[startPos:endPos]) } + return buf } func (r *tokenReader) readString() ([]byte, error) { @@ -462,25 +494,33 @@ func (r *tokenReader) readString() ([]byte, error) { _, _ = reader.Seek(int64(r.pos), io.SeekStart) for { - ch, _, err := reader.ReadRune() + ch, size, err := reader.ReadRune() if err != nil { return nil, r.syntaxErrorOnLastToken(errMsgInvalidString) } if ch == '"' { break } + if ch < 0x20 { + // Control characters must be escaped inside a JSON string. + return nil, r.syntaxErrorOnLastToken(errMsgInvalidString) + } if ch != '\\' { - if haveEscaped { + if ch == utf8.RuneError && size == 1 { + // An invalid UTF-8 byte. encoding/json substitutes the Unicode replacement + // character for these; do the same, which forces us off the zero-copy path. + if !haveEscaped { + chars = beginEscapedCopy(r.data, startPos, (r.len-reader.Len())-1) + haveEscaped = true + } + chars = appendRune(chars, utf8.RuneError) + } else if haveEscaped { chars = appendRune(chars, ch) } continue } if !haveEscaped { - pos := (r.len - reader.Len()) - 1 // don't include the backslash we just read - chars = make([]byte, pos-startPos, pos-startPos+20) - if pos > startPos { - copy(chars, r.data[startPos:pos]) - } + chars = beginEscapedCopy(r.data, startPos, (r.len-reader.Len())-1) // exclude the backslash just read haveEscaped = true } ch, _, err = reader.ReadRune() @@ -501,10 +541,9 @@ func (r *tokenReader) readString() ([]byte, error) { case 't': chars = appendRune(chars, '\t') case 'u': - if ch, ok := readHexChar(&reader); ok { - chars = appendRune(chars, ch) - } else { - return nil, r.syntaxErrorOnLastToken(errMsgInvalidString) + chars, err = r.readUnicodeEscape(&reader, chars) + if err != nil { + return nil, err } default: return nil, r.syntaxErrorOnLastToken(errMsgInvalidString) @@ -525,6 +564,35 @@ func (r *tokenReader) readString() ([]byte, error) { } } +// readUnicodeEscape decodes a \u escape (the leading "\u" has already been consumed), combining +// a UTF-16 surrogate pair into a single code point when the escape is a surrogate followed by a +// valid pairing escape. A lone or invalid surrogate becomes the Unicode replacement character +// with the following bytes left to be parsed normally, matching encoding/json. +func (r *tokenReader) readUnicodeEscape(reader *bytes.Reader, chars []byte) ([]byte, error) { + decoded, ok := readHexChar(reader) + if !ok { + return nil, r.syntaxErrorOnLastToken(errMsgInvalidString) + } + if !utf16.IsSurrogate(decoded) { + return appendRune(chars, decoded), nil + } + mark := r.len - reader.Len() + combined := unicode.ReplacementChar + if b1, e1 := reader.ReadByte(); e1 == nil && b1 == '\\' { + if b2, e2 := reader.ReadByte(); e2 == nil && b2 == 'u' { + if low, lowOK := readHexChar(reader); lowOK { + if pair := utf16.DecodeRune(decoded, low); pair != unicode.ReplacementChar { + combined = pair + } + } + } + } + if combined == unicode.ReplacementChar { + _, _ = reader.Seek(int64(mark), io.SeekStart) + } + return appendRune(chars, combined), nil +} + func readHexChar(reader *bytes.Reader) (rune, bool) { var digits [4]byte for i := 0; i < 4; i++ { @@ -551,6 +619,10 @@ func (r *tokenReader) syntaxErrorOnNextToken(msg string) error { } // This is faster than creating a string to pass to strconv.Atoi. +// +// It returns ok == false when the literal's magnitude exceeds MaxInt64 (including int64's own +// minimum, -2^63, whose magnitude is 2^63) so the caller can fall back to float parsing rather +// than receive a silently wrapped value. func parseIntFromBytes(chars []byte) (int64, bool) { negate := false p := 0 @@ -565,8 +637,14 @@ func parseIntFromBytes(chars []byte) (int64, bool) { return 0, false } } + const maxInt64 = 1<<63 - 1 for p < len(chars) { - ret = ret*10 + int64(chars[p]-'0') + d := int64(chars[p] - '0') + // Signal overflow rather than silently wrapping; the caller falls back to float parsing. + if ret > (maxInt64-d)/10 { + return 0, false + } + ret = ret*10 + d p++ } if negate { diff --git a/jreader/token_reader_rfc8259_default_test.go b/jreader/token_reader_rfc8259_default_test.go new file mode 100644 index 0000000..a0d1360 --- /dev/null +++ b/jreader/token_reader_rfc8259_default_test.go @@ -0,0 +1,286 @@ +package jreader + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// These tests exercise RFC 8259 compliance of the tokenizer. +// +// The authority is encoding/json: json.Valid for grammar (accept/reject) and json.Unmarshal +// for decoded values. One deliberate exception: numeric literals whose magnitude exceeds +// float64's range (e.g. 1e400) are valid per the JSON grammar but cannot be represented, so -- +// like json.Unmarshal into a float64 or interface{} -- the reader rejects them. + +// rejectedByEncodingJSON is a sanity check so the "must reject" table cannot drift into +// asserting that valid JSON is rejected. +func rejectedByEncodingJSON(t *testing.T, input string) { + t.Helper() + require.False(t, json.Valid([]byte(input)), + "test bug: %q is actually valid JSON, so the reader should not reject it", input) +} + +func parseWholeValue(input string) error { + r := NewReader([]byte(input)) + r.Any() + if err := r.Error(); err != nil { + return err + } + return r.RequireEOF() +} + +func TestReaderRejectsMalformedWhitespace(t *testing.T) { + // JSON permits only space, tab, LF, and CR as whitespace between tokens. + for _, input := range []string{ + "\x0b1", // vertical tab + "\x0c1", // form feed + "1\x0b", // vertical tab as trailing content + "[\x0c1]", // form feed inside a structure + "\xc2\x851", // NEL (U+0085), encoded as UTF-8 + "\xc2\xa01", // NBSP (U+00A0), encoded as UTF-8 + } { + t.Run(input, func(t *testing.T) { + rejectedByEncodingJSON(t, input) + require.Error(t, parseWholeValue(input)) + }) + } +} + +func TestReaderAcceptsAllJSONWhitespace(t *testing.T) { + // Carriage return in particular was not covered by the shared suite. + for _, input := range []string{ + " 1 ", + "\t1\t", + "\n1\n", + "\r1\r", + "\r\n 1 \r\n", + } { + t.Run(input, func(t *testing.T) { + require.True(t, json.Valid([]byte(input))) + require.NoError(t, parseWholeValue(input)) + }) + } +} + +func TestReaderRejectsMalformedNumbers(t *testing.T) { + for _, input := range []string{ + "01", + "-01", + "00", + "007", + "1.", + "-1.", + "1.e3", + "-", + "1e", + "1e+", + "1E-", + "1.2.3", + } { + t.Run(input, func(t *testing.T) { + rejectedByEncodingJSON(t, input) + require.Error(t, parseWholeValue(input)) + }) + } +} + +func TestReaderAcceptsValidNumbers(t *testing.T) { + for _, tc := range []struct { + input string + want float64 + }{ + {"0", 0}, + {"-0", 0}, + {"3", 3}, + {"-3", 3 * -1}, + {"1.5", 1.5}, + {"-1.5", -1.5}, + {"0.5", 0.5}, + {"1e3", 1000}, + {"1E3", 1000}, + {"1e+3", 1000}, + {"1e-3", 0.001}, + {"1.5e2", 150}, + {"10", 10}, + {"100", 100}, + {"0e1", 0}, + {"0.0e0", 0}, + {"1e10", 1e10}, + {"1E+2", 100}, + } { + t.Run(tc.input, func(t *testing.T) { + require.True(t, json.Valid([]byte(tc.input))) + r := NewReader([]byte(tc.input)) + got := r.Float64() + require.NoError(t, r.Error()) + require.NoError(t, r.RequireEOF()) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestReaderRejectsUnescapedControlCharsInString(t *testing.T) { + for _, input := range []string{ + "\"a\tb\"", // literal tab + "\"a\nb\"", // literal newline + "\"a\rb\"", // literal carriage return + "\"\x00\"", // NUL + "\"\x01\"", // SOH + "\"\x1f\"", // unit separator (highest control char) + } { + t.Run(input, func(t *testing.T) { + rejectedByEncodingJSON(t, input) + require.Error(t, parseWholeValue(input)) + }) + } +} + +func TestReaderCombinesSurrogatePairs(t *testing.T) { + // A \u-escaped UTF-16 surrogate pair must decode to a single code point rather than two + // replacement characters. These use raw string literals (backticks) so the literal + // backslash-u escape bytes reach the reader -- and readUnicodeEscape's combining branch -- + // rather than being resolved to decoded text by the Go compiler. + for _, tc := range []struct { + input string + want string + }{ + {`"\uD834\uDD1E"`, "\U0001D11E"}, // musical G clef + {`"a\uD834\uDD1Eb"`, "a\U0001D11Eb"}, // with surrounding characters + {`"\uD83D\uDE02"`, "\U0001F602"}, // face with tears of joy + {`"\uDBFF\uDFFF"`, "\U0010FFFF"}, // maximum code point + } { + t.Run(tc.input, func(t *testing.T) { + var stdlib string + require.NoError(t, json.Unmarshal([]byte(tc.input), &stdlib)) + assert.Equal(t, tc.want, stdlib, "test bug: expectation disagrees with encoding/json") + + r := NewReader([]byte(tc.input)) + got := r.String() + require.NoError(t, r.Error()) + require.NoError(t, r.RequireEOF()) + assert.Equal(t, tc.want, got) + }) + } + + // The same code point encoded as literal UTF-8 bytes must also decode intact -- this goes + // through the plain ReadRune path rather than the escape path. + r := NewReader([]byte(`"a𝄞b"`)) + got := r.String() + require.NoError(t, r.Error()) + require.NoError(t, r.RequireEOF()) + assert.Equal(t, "a\U0001D11Eb", got) +} + +func TestReaderReplacesInvalidSurrogates(t *testing.T) { + // Lone or malformed surrogates decode to the replacement character, matching + // encoding/json (which does not treat them as an error). + for _, tc := range []struct { + input string + want string + }{ + {`"\uD834"`, "�"}, // lone high surrogate + {`"\uDD1E"`, "�"}, // lone low surrogate + {`"\uD834x"`, "�x"}, // high surrogate followed by a normal char + {`"\uD834\uD834"`, "��"}, // high surrogate followed by another high surrogate + } { + t.Run(tc.input, func(t *testing.T) { + var stdlib string + require.NoError(t, json.Unmarshal([]byte(tc.input), &stdlib)) + assert.Equal(t, tc.want, stdlib, "test bug: expectation disagrees with encoding/json") + + r := NewReader([]byte(tc.input)) + got := r.String() + require.NoError(t, r.Error()) + require.NoError(t, r.RequireEOF()) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestReaderRejectsControlCharInPropertyName(t *testing.T) { + // The control-char rule applies to property names, not just string values. + input := "{\"a\tb\":1}" + require.False(t, json.Valid([]byte(input))) + + r := NewReader([]byte(input)) + obj := r.Object() + obj.Next() + require.Error(t, r.Error()) +} + +func TestReaderSubstitutesInvalidUTF8(t *testing.T) { + // Raw invalid UTF-8 bytes inside a string decode to the Unicode replacement character, + // matching encoding/json, regardless of where an escape appears in the same string. + for _, tc := range []struct { + name string + input []byte + }{ + {"lone invalid byte", []byte("\"\xff\"")}, + {"invalid byte then text", []byte("\"\xffab\"")}, + {"text then invalid byte", []byte("\"ab\xff\"")}, + {"invalid byte after escape", []byte("\"\\n\xff\"")}, + {"invalid byte before escape", []byte("\"\xff\\n\"")}, + {"CESU-8 encoded surrogate", []byte("\"\xed\xa0\x80\"")}, + } { + t.Run(tc.name, func(t *testing.T) { + require.True(t, json.Valid(tc.input), "precondition: valid JSON grammar") + var want string + require.NoError(t, json.Unmarshal(tc.input, &want)) + + r := NewReader(tc.input) + got := r.String() + require.NoError(t, r.Error()) + require.NoError(t, r.RequireEOF()) + assert.Equal(t, want, got) + }) + } +} + +func TestReaderLargeIntegersMatchEncodingJSON(t *testing.T) { + // Integer literals that overflow int64 but are within float64 range must decode to the same + // value encoding/json produces, not a wrapped-int64 garbage value. + for _, input := range []string{ + "99999999999999999999", // > 2^63 + "12345678901234567890", // > 2^63 + "9223372036854775808", // 2^63 exactly + "18446744073709551616", // 2^64 + "123456789012345678901234567890", // ~1.2e29, still within float64 range + } { + t.Run(input, func(t *testing.T) { + var want float64 + require.NoError(t, json.Unmarshal([]byte(input), &want)) + + r := NewReader([]byte(input)) + got := r.Float64() + require.NoError(t, r.Error()) + require.NoError(t, r.RequireEOF()) + assert.Equal(t, want, got) + }) + } +} + +func TestReaderRejectsOutOfRangeNumbers(t *testing.T) { + // Numbers whose magnitude exceeds float64's range are valid JSON grammar but cannot be + // represented. encoding/json rejects them when decoding into a float64/interface{}, and so + // does the reader (a deliberate exception to strict json.Valid parity). + for _, input := range []string{ + "1e400", + "9e999", + "-1e400", + "1E4000", // out-of-range float + "1" + strings.Repeat("0", 400), // out-of-range integer literal + } { + t.Run(input, func(t *testing.T) { + require.True(t, json.Valid([]byte(input)), "precondition: valid JSON grammar") + var sink interface{} + require.Error(t, json.Unmarshal([]byte(input), &sink), + "precondition: encoding/json also rejects the value") + + require.Error(t, parseWholeValue(input)) + }) + } +} diff --git a/jwriter/token_writer_default.go b/jwriter/token_writer_default.go index 806557f..19a5c0c 100644 --- a/jwriter/token_writer_default.go +++ b/jwriter/token_writer_default.go @@ -2,11 +2,17 @@ package jwriter import ( "encoding/json" + "errors" "io" + "math" "strconv" "unicode/utf8" ) +// errNonFiniteNumber is returned when attempting to write a NaN or infinite floating-point +// value, which cannot be represented in JSON. +var errNonFiniteNumber = errors.New("cannot encode NaN or infinite number as JSON") + // This file defines the low-level JSON token writer. We don't define an interface for these methods, // because calling them through an interface would limit performance. @@ -84,6 +90,9 @@ func (tw *tokenWriter) Int(value int) error { // Float64 writes a JSON number. func (tw *tokenWriter) Float64(value float64) error { + if math.IsNaN(value) || math.IsInf(value, 0) { + return errNonFiniteNumber + } if value == 0 { tw.buf.WriteByte('0') } else { diff --git a/jwriter/token_writer_rfc8259_default_test.go b/jwriter/token_writer_rfc8259_default_test.go new file mode 100644 index 0000000..b655f75 --- /dev/null +++ b/jwriter/token_writer_rfc8259_default_test.go @@ -0,0 +1,32 @@ +package jwriter + +import ( + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// NaN and infinities cannot be represented in JSON, so the writer must report an error rather +// than emit invalid output. +func TestWriterRejectsNonFiniteFloat64(t *testing.T) { + for _, value := range []float64{ + math.NaN(), + math.Inf(1), + math.Inf(-1), + } { + t.Run("", func(t *testing.T) { + w := NewWriter() + w.Float64(value) + require.Error(t, w.Error()) + }) + } +} + +func TestWriterAcceptsFiniteFloat64(t *testing.T) { + w := NewWriter() + w.Float64(1.5) + require.NoError(t, w.Error()) + assert.Equal(t, "1.5", string(w.Bytes())) +}