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.

Sign up for GitHub

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

Jump to bottom

encoding/json: unmarshaler doesn't find UnmarshalJSON on embedded anonymous struct #46516

Open
fancl20 opened this issue Jun 2, 2021 · 6 comments
Open

encoding/json: unmarshaler doesn't find UnmarshalJSON on embedded anonymous struct #46516

fancl20 opened this issue Jun 2, 2021 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@fancl20
Copy link

fancl20 commented Jun 2, 2021 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.15.9 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.15"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.15/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build393930481=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/Xlxih58rSdC

Outer1 unmarshalled and Outer2 failed.

type Outer1 struct {
	Inner inner
}
type inner struct{ Mixin }

type Outer2 struct {
	Inner struct{ Mixin }
}

type Mixin string

func (r * Mixin) UnmarshalJSON(data []byte) error {
	return json.Unmarshal(data, (*string)(r))
}

What did you expect to see?

Outer2 unmarshalled as Outer1

What did you see instead?

panic: json: cannot unmarshal string into Go struct field Outer2.Inner of type struct { main.Mixin }
@mvdan
Copy link
Member

mvdan commented Jun 2, 2021

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

@mvdan mvdan changed the title json unmarshaler doesn't find UnmarshalJSON on embedded anonymous struct encoding/json: unmarshaler doesn't find UnmarshalJSON on embedded anonymous struct Jun 2, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 2, 2021
@googollee
Copy link

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.UnmarshalJSONinterface.

I think at the entrance of json.indirect(), checking v.CanAddr() is enough. It needs not to check if v is a named type.

@dsnet
Copy link
Member

dsnet commented Jun 2, 2021

@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.

@mvdan
Copy link
Member

mvdan commented Jun 2, 2021

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 :)

@gopherbot
Copy link

Change https://golang.org/cl/324469 mentions this issue: encoding/json: call UnmarshalJSON() when decoding with a unamed struct.

@googollee
Copy link

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.

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
@seankhliao seankhliao mentioned this issue Sep 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@googollee @fancl20 @mvdan @dsnet @gopherbot @seankhliao