go icon indicating copy to clipboard operation
go copied to clipboard

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

Open icio opened this issue 3 years ago • 0 comments

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.

icio avatar Jan 27 '22 12:01 icio