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: panic for nil map key types implementing TextMarshaler #33675

wI2L opened this issue Aug 15, 2019 · 6 comments

encoding/json: panic for nil map key types implementing TextMarshaler #33675

wI2L opened this issue Aug 15, 2019 · 6 comments


Copy link

@wI2L wI2L commented Aug 15, 2019

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

$ go version
go version go1.12.8 darwin/amd64

Does this issue reproduce with the latest release?


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

go env Output
$ go env
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/rw/cyt9bjxx34qglfg2jsfm27jr0000gn/T/go-build363570579=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Given the following program that use a map key type implementing the encoding.TextMarshaler interface, when the key is nil, the code panic.

What did you expect to see?

Expected a json.UnsupportedValueError, or the key to be encoded as an empty string.

What did you see instead?

Following panic stack trace.
Taken from playground output.

panic: value method net.IP.MarshalText called using nil *IP pointer [recovered]
	panic: value method net.IP.MarshalText called using nil *IP pointer

goroutine 1 [running]:
encoding/json.(*encodeState).marshal.func1(0x44af48, 0x5d4c)
	/usr/local/go/src/encoding/json/encode.go:302 +0xc0
panic(0x1275a0, 0x40c2c0)
	/usr/local/go/src/runtime/panic.go:522 +0x240
net.(*IP).MarshalText(0x0, 0x134040, 0x0, 0x1, 0xeed34048, 0x0, 0x1, 0x128d80)
	<autogenerated>:1 +0xa0
encoding/json.(*reflectWithString).resolve(0x4301e0, 0x2, 0x2, 0x0)
	/usr/local/go/src/encoding/json/encode.go:871 +0x420
encoding/json.mapEncoder.encode(0x15a05c, 0x4540c0, 0x1235c0, 0x43e360, 0x15, 0x120100)
	/usr/local/go/src/encoding/json/encode.go:690 +0x4a0
encoding/json.(*encodeState).reflectValue(0x4540c0, 0x1235c0, 0x43e360, 0x15, 0x120100, 0x207a60)
	/usr/local/go/src/encoding/json/encode.go:334 +0xa0
encoding/json.(*encodeState).marshal(0x4540c0, 0x1235c0, 0x43e360, 0x100, 0x0, 0x0)
	/usr/local/go/src/encoding/json/encode.go:306 +0x100
encoding/json.Marshal(0x1235c0, 0x43e360, 0x40a0f0, 0x5d4c, 0x444260, 0x43e280, 0x0, 0x5d4c)
	/usr/local/go/src/encoding/json/encode.go:160 +0x60
	/tmp/sandbox157275249/prog.go:15 +0x1a0

The (encodeWithString).resolve method does not check if a reflect.Value of kind Ptr is nil before doing the type assertion to encoding.TextMarshaler with the result of the (reflect.Value).Interface call.

After adding the following check after line 869 of encoding/json/encode.go:

if w.v.Kind() == reflect.Ptr && w.v.IsNil() {
	return nil

I now get this output:


Am I missing something regarding nil keys in maps ?

@wI2L wI2L changed the title encoding/json: panic for nil map keys encoding/json: panic for nil map key types implementing TextMarshaler Aug 15, 2019
Copy link

@agnivade agnivade commented Aug 16, 2019

@agnivade agnivade added this to the Unplanned milestone Aug 16, 2019
Copy link

@mvdan mvdan commented Aug 16, 2019

Could you check older Go versions, to see if this was a regression at some point? Could you also investigate if this happens with either encoding.TextMarshaler or json.Marshaler in other scenarios?

Copy link
Contributor Author

@wI2L wI2L commented Aug 16, 2019

The change was introduced after this proposal #12146 in this CL It was released with go1.7 (see encoding/json changelog part

I've tried to reproduce the panic with the sample program with the last version of all majors: 1.11.13, 1.10.8, 1.9.7, 1.8.7 and 1.7.6, and can confirm that it panics for all.

Marshaling a type that implements the encoding.TextMarshaler interface directly, or via a struct field are fine:
I don't think json.Marshaler is impacted.
What kind of scenarios do you have in mind ?

I discovered this bug while writing the testsuite for a custom JSON encoder, as part of a personal project (most of the test cases outputs are compared against the standard library), and I found no other related issues (using last version 1.12.9).

/cc @augustoroman for original changes

Copy link

@mvdan mvdan commented Aug 17, 2019

Fair enough, thanks. Can you send your change above as a CL, along with a test? We can continue the discussion there. Without investigating further, this seems like something to fix.

Copy link
Contributor Author

@wI2L wI2L commented Aug 17, 2019

@mvdan Sure thing. I'll send a CL.
edit: CL sent with new test testing both nil/non-nil cases.

Copy link

@gopherbot gopherbot commented Aug 17, 2019

Change mentions this issue: encoding/json: fix panic for nil instances of TextMarshaler in map keys

@gopherbot gopherbot closed this in 85f3ca7 Sep 1, 2019
t4n6a1ka added a commit to t4n6a1ka/go that referenced this issue Sep 5, 2019
This change adds a a check in the encodeWithString.resolve method
to ensure that a reflect.Value with kind Ptr is not nil before
the type assertion to TextMarshaler.

If the value is nil, the method returns a nil error, and the map key
encodes to an empty string.

Fixes golang#33675

Change-Id: I0a04cf690ae67006f6a9c5f8cbb4cc99d236bca8
GitHub-Last-Rev: 6c987c9
GitHub-Pull-Request: golang#33700
Run-TryBot: Daniel Martí <>
TryBot-Result: Gobot Gobot <>
Reviewed-by: Daniel Martí <>
@golang golang locked and limited conversation to collaborators Aug 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.