Repository navigation
encoding/json: bad encoding of field with MarshalJSON method #22967
Description
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
version 1.9
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (go env)?
windows/amd64
What did you do?
package main
import (
"encoding/json"
"os"
)
type T bool
func (v *T) MarshalJSON() ([]byte, error) {
return []byte{'1'}, nil
}
type S struct {
X T
}
func main() {
v := S{true}
e := json.NewEncoder(os.Stderr)
e.Encode(v) // should print {"X":true}
e.Encode(&v) // should print the same value
}What did you expect to see?
{"X":true}
{"X":true}What did you see instead?
{"X":true}
{"X":1}Issue
The json.Marshal documentation states that:
Pointer values encode as the value pointed to. A nil pointer encodes as the null JSON value.
Thus, &v should be marshaled to the same JSON value as v:
{"X":true}Moreover, it states that:
If an encountered value implements the Marshaler interface and is not a nil pointer, Marshal calls its MarshalJSON method to produce JSON.
Therefore, Marshal should not call the MarshalJSON method to produce JSON for the X field, because its T type does not implement the json.Marshaler interface. In fact, MarshalJSON has *T receiver and the Go documentation states:
The method set of an interface type is its interface. The method set of any other type
Tconsists of all methods declared with receiver typeT. ... The method set of a type determines the interfaces that the type implements ...
As a final remark:
-
from the source code, the most relevant difference between cases
vand&vis thatXfield becomes addressable in case&v, changing the encoding generated bycondAddrEncoder; -
implementing
MarshalJSONwithTvalue receiver makesTajson.Marshalerinterface, withXvalues properly encoded by MarshalJSON:{"X":1} {"X":1} -
if the intended behavior is that Marshal should anyway call the
MarshalJSONmethod on encodingTvalues, whenever*Timplements thejson.Marshalerinterface, that should be clearly documented.
Activity
dsnet commentedon Dec 1, 2017
This does seem like surprising behavior. It should either be documented or the encoder will need to jump through some hoops to copy the value so that it can be addressable.
larhun commentedon Dec 3, 2017
This issue is more general and affects all custom encoders accepted by
jsonandxmlpackages (maybe other?):More examples at:
It occurs whenever a type
Thas a custom encoder with*Tpointer receiver. In that case something weird and not documented happens on encoding a valuexof typeT: it does try to get thexaddress and call the custom encoder. However, ifxis not addressable, it cannot succeed.Is the address resolution really necessary? I do not think so. The custom encoder should only be called if properly implemented, as clearly stated by the Go documentation:
The idiomatic and recommended implementation for a custom encoder should be with
Tvalue receiver. However, if*Tpointer receiver is used,&x(addressable) should be encoded asx(not addressable): a varying behavior is confusing and not acceptable.larhun commentedon Dec 3, 2017
A minimal solution could be: clarify the behavior of each custom encoders by adding that
Tvalue receiver is the expected usage and*Tpointer receiver may fail to be used.A definitive solution could be: change the documentation and implementation of the encoding process by first checking if the value under encoding is a pointer, i.e. the first step should be:
and, therefore, call a custom encoder only if implemented with
Tvalue receiver. That way, a custom encoder with*Tpointer receiver will never be called.rsc commentedon Nov 28, 2018
It seems clear that Encode(&v) should return
{"X":1}.What's less clear is Encode(v). It can't return
{"X":1}and today it returns{"X":true}but perhaps it should instead return an error (I can see there's a pointer method but I can't take the address of the value to get a pointer).Leaving that decision - is this OK or is should Encode return an error? - for Go 1.13.
12 remaining items
dsnet commentedon Jan 10, 2024
We can't the behavior in "encoding/json" as this would break existing usages. The "encoding/json/v2" draft design proposes to fix this issue by always treating values as addressable (shallow copying if necessary).
dmitris commentedon Feb 15, 2024
could changing the receiver of
MarshalJSONfrom a pointer to the value have performance implications, with the whole structure being copied instead of just a pointer?op-chain-ops: address semgrep
Inomurko/v1.7.6 (#126)
Inomurko/v1.7.7 (#160)
gopherbot commentedon Aug 20, 2024
Change https://go.dev/cl/606495 mentions this issue:
encoding: add full support for marshalers with pointer receiversFix VUL-224: Change MarshalJSON to use value receiver
cannon: remove final dep on bindings (#10408)
jsonschema: use value receiver for Schema.MarshalJSON (#52)
cannon: remove final dep on bindings (#10408)
dsnet commentedon Aug 21, 2026
This is an unfortunate bug in v1 where values are not made addressable in order to call methods on a pointer receiver. The buggy behavior is documented at https://pkg.go.dev/encoding/json#CallMethodsWithLegacySemantics. For backward compatibility reasons, we cannot change this.
With the release of Go 1.27, we now have "encoding/json/v2", which consistently always calls methods on pointer receivers even if the value is not addressable. See https://go.dev/play/p/TsettsM6w3D