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

Jump to bottom

encoding/json: Marshal silently ignores custom implementations on pass by value #30351

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

encoding/json: Marshal silently ignores custom implementations on pass by value #30351

mcandre opened this issue Feb 22, 2019 · 3 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Go1.13

Comments

@mcandre
Copy link

mcandre 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
Copy link
Contributor

ericlagergren commented Feb 22, 2019 •
edited

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
Copy link
Contributor

antong 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 the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 22, 2019
@dmitshur dmitshur added this to the Go1.13 milestone Feb 22, 2019
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
5 participants
@mcandre @antong @dmitshur @ericlagergren @gopherbot