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 silently ignores custom implementations on pass by value #30351

Closed
mcandre opened this Issue Feb 22, 2019 · 3 comments

Comments

Projects
None yet
5 participants
@mcandre
Copy link

commented Feb 22, 2019

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

$ go version
go version go1.11 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
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/tkmamhf/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/tkmamhf/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/tkmamhf/.gvm/gos/go1.11"
GOTMPDIR=""
GOTOOLDIR="/Users/tkmamhf/.gvm/gos/go1.11/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/y6/dbk1vvs179nbpfv38j__10b5rsnf87/T/go-build193800130=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Carefully implemented json.Marshaler and json.Unmarshaler interfaces, and passed my struct directly to json.Marshal().

What did you expect to see?

The resulting JSON byte array uses my custom json.Marshaler interface method.

What did you see instead?

The resulting JSON byte array ignores my custom json.Marshaler interface method, instead presenting the keys and values of the plain Go struct declaration. This causes downstream breakages, as my custom JSON marshaling is needed to offset some issues with the data I happen to be handling.

Workaround

As a workaround, I am making sure that I pass a reference (&) of my object to the json.Marshal() method. This is quite dangerous, as it is a really easy mistake to make, across a large codebase. And currently no way to validate this with the Go type system on the existing API. With other Go API calls, I at least get an annoying compile about needing a receiver pointer.

In the future, can we improve the json API to prevent this from happening?

@ericlagergren

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

FWIW: you only need to pass a pointer to json.Unmarshal if you implement (*T).MarshalJSON. And if you always pass a pointer, you can implement either (*T).MarshalJSON or (T).MarshalJSON.

See: https://play.golang.org/p/4i6MZABdBa7

@antong

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

If at all possible, please include in the "what did you do" section the actual code, and in the "what did you see instead" the actual result and errors if any. This would make it so much easier to analyze and help.

My guess is that the value you pass to json.Marshal(yourval) does not actually implement the json.Marshal interface at all, probably because the method had a pointer receiver e.g., func (x *MyType) MarshalJSON() ([]byte, error and you passed a MyType value, not a pointer.

Example: https://play.golang.org/p/K2M4Ap6uO-C

@bradfitz bradfitz changed the title json.Marshal() silently ignores custom implementations on pass by value encoding/json: Marshal silently ignores custom implementations on pass by value Feb 22, 2019

@dmitshur dmitshur added this to the Go1.13 milestone Feb 22, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Mar 22, 2019

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this Mar 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.