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

encoding/json: don't silently ignore errors from (*Decoder).More #67525

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions src/encoding/json/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import (

// A Decoder reads and decodes JSON values from an input stream.
type Decoder struct {
r io.Reader
buf []byte
d decodeState
scanp int // start of unread data in buf
scanned int64 // amount of data already scanned
scan scanner
err error
r io.Reader
buf []byte
d decodeState
scanp int // start of unread data in buf
scanned int64 // amount of data already scanned
scan scanner
err error
errFromMore error

tokenState int
tokenStack []int
Expand Down Expand Up @@ -47,6 +48,11 @@ func (dec *Decoder) DisallowUnknownFields() { dec.d.disallowUnknownFields = true
// See the documentation for [Unmarshal] for details about
// the conversion of JSON into a Go value.
func (dec *Decoder) Decode(v any) error {
if dec.errFromMore != nil {
err := dec.errFromMore
dec.errFromMore = nil
return err
}
if dec.err != nil {
return dec.err
}
Expand Down Expand Up @@ -365,6 +371,12 @@ func (d Delim) String() string {
// to mark the start and end of arrays and objects.
// Commas and colons are elided.
func (dec *Decoder) Token() (Token, error) {
if dec.errFromMore != nil {
err := dec.errFromMore
dec.errFromMore = nil
return nil, err
}

for {
c, err := dec.peek()
if err != nil {
Expand Down Expand Up @@ -481,7 +493,19 @@ func (dec *Decoder) tokenError(c byte) (Token, error) {
// current array or object being parsed.
func (dec *Decoder) More() bool {
c, err := dec.peek()
return err == nil && c != ']' && c != '}'
if err != nil {
if err == io.EOF {
// Preserve the io.EOF error, next read calls from the io.Reader
// do not have to return the io.EOF error, but we've made
// a decision based on that error, so it is better to return it from
// next Token or Decode call.
dec.errFromMore = io.EOF
return false
}
dec.errFromMore = err
return true
}
return c != ']' && c != '}'
}

func (dec *Decoder) peek() (byte, error) {
Expand Down
173 changes: 173 additions & 0 deletions src/encoding/json/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package json

import (
"bytes"
"errors"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -520,3 +521,175 @@ func TestHTTPDecoding(t *testing.T) {
t.Errorf("Decode error:\n\tgot: %v\n\twant: io.EOF", err)
}
}

type decoderMoreWithDecodeFailingReader struct {
t *testing.T
i int
err error
}

func (c *decoderMoreWithDecodeFailingReader) Read(b []byte) (n int, err error) {
i := c.i
c.i++

defer func() {
// Decoder always passes a buffer with size that has at least 512 Bytes. This test
// depends on that behaviour, so that the reader does not get unnecessarily complicated.
if len(b) == n {
c.t.Fatal("small buffer passed to Read")
}
}()

switch i {
case 0:
return copy(b, `[{ "test": 1 }`), nil
case 1:
return 0, c.err
default:
return 0, io.EOF
}
}

func TestDecoderMoreWithDecode(t *testing.T) {
type Message struct{ Test int }

notIgnoredError := errors.New("not ignored error")
dec := NewDecoder(&decoderMoreWithDecodeFailingReader{t: t, err: notIgnoredError})

if val, err := dec.Token(); err != nil || val != Delim('[') {
t.Fatalf("(*Decoder).Token() = (%v, %v); want = ([, <nil>)", val, err)
}

if err := dec.Decode(new(Message)); err != nil {
t.Fatalf("(*Decoder).Decode() = %v; want = <nil>", err)
}

if !dec.More() {
t.Fatalf("(*Decoder).More() = false; want = true")
}

if err := dec.Decode(new(Message)); err != notIgnoredError {
t.Fatalf("(*Decoder).Decode() = %v; want = %v", err, notIgnoredError)
}

if dec.More() {
t.Fatalf("(*Decoder).More() = true; want = false")
}

if err := dec.Decode(new(Message)); err != io.EOF {
t.Fatalf("(*Decoder).Decode() = %v; want = %v", err, io.EOF)
}
}

type decoderMoreWithTokenFailingReader struct {
t *testing.T
i int
err error
}

func (c *decoderMoreWithTokenFailingReader) Read(b []byte) (n int, err error) {
i := c.i
c.i++

defer func() {
// Decoder always passes a buffer with size that has at least 512 Bytes. This test
// depends on that behaviour, so that the reader does not get unnecessarily complicated.
if len(b) == n {
c.t.Fatal("small buffer passed to Read")
}
}()

switch i {
case 0:
return copy(b, `{`), nil
case 1:
return 0, c.err
default:
return 0, io.EOF
}
}

func TestDecoderMoreWithToken(t *testing.T) {
notIgnoredError := errors.New("not ignored error")
dec := NewDecoder(&decoderMoreWithTokenFailingReader{t: t, err: notIgnoredError})

if val, err := dec.Token(); err != nil || val != Delim('{') {
t.Fatalf("(*Decoder).Token() = (%v, %v); want = ({, <nil>)", val, err)
}

if !dec.More() {
t.Fatalf("(*Decoder).More() = false; want = true")
}

if val, err := dec.Token(); err != notIgnoredError {
t.Fatalf("(*Decoder).Token() = (%v, %v); want = (<nil>, %v)", val, err, notIgnoredError)
}

if dec.More() {
t.Fatalf("(*Decoder).More() = true; want = false")
}

if val, err := dec.Token(); err != io.EOF {
t.Fatalf("(*Decoder).Token() = (%v, %v); want = (<nil>, %v)", val, err, io.EOF)
}
}

type moreWithErrorsTestReader struct {
t *testing.T
i int
err error
}

func (c *moreWithErrorsTestReader) Read(b []byte) (n int, err error) {
i := c.i
c.i++

defer func() {
// Decoder always passes a buffer with size that has at least 512 Bytes. This test
// depends on that behaviour, so that the reader does not get unnecessarily complicated.
if len(b) == n {
c.t.Fatal("small buffer passed to Read")
}
}()

switch i {
case 0:
return copy(b, `[{ "test": 1 }`), nil
case 1:
return 0, c.err
case 2:
return copy(b, `, {"test": 2}]`), nil
default:
return 0, io.EOF
}
}

func TestMoreWithErrors(t *testing.T) {
notIgnoredError := errors.New("not ignored error")
dec := NewDecoder(&moreWithErrorsTestReader{t: t, err: notIgnoredError})

if val, err := dec.Token(); err != nil || val != Delim('[') {
t.Fatalf("(*Decoder).Token() = (%v, %v); want = ([, <nil>)", val, err)
}

second := false
for dec.More() {
var msg struct{ Test int }
err := dec.Decode(&msg)
var expectErr error = nil
if second {
expectErr = notIgnoredError
}
if err != expectErr {
t.Fatalf("(*Decoder).Decode() = %v; want = %v", err, expectErr)
}
if err != nil {
return
}
second = true
}

if val, err := dec.Token(); err != nil || val != Delim(']') {
t.Fatalf("(*Decoder).Token() = (%v, %v); want = (], <nil>)", val, err)
}
}