Open fancl20 opened 3 years ago
At first glance this seems like a bug, thanks. There have been many bugs in the past involving struct embedding, but after a quick search I don't think this is a duplicate. Want to try working on a fix with a test?
cc @dsnet
It's because json.indirect()
only tries to get a field address with a named type. See https://cs.opensource.google/go/go/+/refs/tags/go1.16.4:src/encoding/json/decode.go;l=443.
The type of Outer2.Inner
is a struct without a name, so json.indirect()
doesn't try its pointer and of course it's not compatible with json.UnmarshalJSON
interface.
I think at the entrance of json.indirect()
, checking v.CanAddr()
is enough. It needs not to check if v is a named type.
@googollee, thank you for your analysis. I agree with your assessment of why this isn't triggering. It seems that this is a bug that's existed for 10 years :)
We could try fixing this for the Go1.18 release cycle, but I'm always wary of how fixing bugs breaks a number of users who already depend on the current behavior.
We could always merge this early in the 1.18 cycle, and see if any users complain. I would hope that noone is depending on the existing inconsistent behavior - I've been wrong on that assumption before, but not always :)
Change https://golang.org/cl/324469 mentions this issue: encoding/json: call UnmarshalJSON() when decoding with a unamed struct.
Removing the check of named structs looses the limitation. I think it should be fine with existing codes.
I made a change with a unit test.
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://play.golang.org/p/Xlxih58rSdC
Outer1 unmarshalled and Outer2 failed.
What did you expect to see?
Outer2 unmarshalled as Outer1
What did you see instead?