json-iterator / go

A high-performance 100% compatible drop-in replacement of "encoding/json"
http://jsoniter.com/migrate-from-go-std.html
MIT License
13.43k stars 1.03k forks source link

json.Unmarshaler fields trick generalStructDecoder into reporting wrong fields in errors #598

Open icio opened 2 years ago

icio commented 2 years ago

In the following snippet we provide a bad timestamp "bad" into the BadUnmarshalContent field:

package main

import (
    "fmt"
    "time"

    jsoniter "github.com/json-iterator/go"
)

func main() {
    var json = jsoniter.ConfigCompatibleWithStandardLibrary

    var v struct {
        ShouldntBeMentioned bool
        BadUnmarshalContent time.Time
    }
    err := json.Unmarshal([]byte(`{"BadUnmarshalContent": "bad", "ShouldntBeMentioned": true}`), &v)
    fmt.Println(err)
}

https://go.dev/play/p/bGfx9UwmEwY?v=gotip

Unexpectedly, this generates an error prefixed with "ShouldntBeMentioned", which was fine.

ShouldntBeMentioned: BadUnmarshalContent: unmarshalerDecoder: parsing time "\"bad\"" as "\"2006-01-02T15:04:05Z07:00\"": cannot parse "bad\"" as "2006", error found in #10 byte of ...|nt": "bad", "Shouldn|..., bigger context ...|{"BadUnmarshalContent": "bad", "ShouldntBeMentioned": true}|...

This happens because generalStructDecoder.Decode tries to decode all fields before checking whether there were any errors:

https://github.com/json-iterator/go/blob/024077e996b048517130b21ea6bf12aa23055d3d/reflect_struct_decoder.go#L507-L512

while structFieldDecoder.Decode believes any iter.Error is its own:

https://github.com/json-iterator/go/blob/024077e996b048517130b21ea6bf12aa23055d3d/reflect_struct_decoder.go#L1054-L1057

It seems that generalStructDecoder.Decode may be assuming nextToken won't return ',' after an error - but when decoding a json.Unmarshaller the buffer is ready at the next ',' even if there was an error. In the original example, if you change the type of BadUnmarshalContent to something that isn't a json.Unmarshaller but will fail to unmarshal from "bad" (e.g. an int) we will get the expected field attribution in the error: https://go.dev/play/p/qMDmXuCbmjZ.