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: Marshal skips checking Marshaler implementation for nil pointer #48430

Closed
mdaliyan opened this issue Sep 17, 2021 · 6 comments
Closed

Comments

@mdaliyan
Copy link

mdaliyan commented Sep 17, 2021

Go version:

$ go version
go version go1.17 darwin/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="/Users/{my-user}/Library/Caches/go-build"
GOENV="/Users/{my-user}/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/{my-user}/go/pkg/mod"
GONOPROXY="go-packages.intra.rakuten-it.com"
GONOSUMDB="go-packages.intra.rakuten-it.com"
GOOS="darwin"
GOPATH="/Users/{my-user}/go"
GOPRIVATE="go-packages.intra.rakuten-it.com"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.17/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.17/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/y2/hj8l5gvx1hd1qjk5tdwrfb19r_252c/T/go-build1795948986=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I tried to call json.Marshal func on the encoding/json package and give it a nil pointer of my custom-defined type that satisfies the json.Marshaler interface.

I tried to write the simplest example. Please look at the output for the b1 and b2 variables in the playground .

What did you expect to see?

I expected json.Marshal called the MarshalJSON receiver that is defined on my custom-defined type since it already implements json.Marshaler.

What did you see instead?

If the passed interface argument is a nil pointer, thejson.Marshal method doesn't check if it satisfies the json.Marshaler or not

@mvdan
Copy link
Member

mvdan commented Sep 17, 2021

This is working as documented:

Marshal traverses the value v recursively. If an encountered value implements the Marshaler interface and is not a nil pointer, Marshal calls its MarshalJSON method to produce JSON.

For better or worse, we can't change that right now, as it would break existing users of the API.

@dsnet
Copy link
Member

dsnet commented Sep 17, 2021

Closing as the current behavior seems well-documented and can't be changed with breaking existing usages.

@dsnet dsnet closed this as completed Sep 17, 2021
@dsnet dsnet changed the title json.Marshal skips checking json.Marshaler implementation if the given argument is a nil pointer encoding/json: Marshal skips checking Marshaler implementation for nil pointer Sep 17, 2021
@mdaliyan
Copy link
Author

mdaliyan commented Sep 18, 2021

Feels like the documentation is just pointing to a bug that exists. The thing that scares me is Go being ended up like Javascript that can’t fix so many bugs because existing codes rely on them.

The example I wrote in the playground is what I expect to happen. I want control over the process of marshaling, even if it’s a nil pointer. Sadly it’s not possible and I have to write a messy code to gain that control.

I just hope it can be fixed in Go2

@dsnet
Copy link
Member

dsnet commented Sep 18, 2021

I'm not sure I agree that calling 'MarshalJSON' on a nil pointer is behavior that people expect. I actually believe the current behavior is correct.

@mdaliyan
Copy link
Author

mdaliyan commented Sep 18, 2021

It’s arguable. The interface is not exactly a nil without any identity. It has a type if you use reflect on it. And olso if you check it, it satidfies the json.Marshaller interface. It’s very different from a simple nil.

We implement the json.Marshaller for a reason. To have control over the flow. It’s a little odd that we expect our reciever not to be called in some situations. This tells us we don’t have the full control over the code.

@dsnet
Copy link
Member

dsnet commented Sep 18, 2021

My earlier comment was serving as a counter-argument to the claim that the current behavior is a bug that must be fixed.

Both behaviors are reasonable. In the absent of an obviously right approach, we need to pick one and document it. That's what the current json package does.

Also, if you want a want to ensure the MarshalJSON method is called even on nil pointers, you can embed a pointer to the type in an empty struct:

json.Marshal(struct{ *aStruct }{theStruct})

See https://play.golang.org/p/QNnip0082RG

@golang golang locked and limited conversation to collaborators Sep 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants