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 if marshaling pointer and non-pointer of same base type #6458

Closed
dominikh opened this issue Sep 23, 2013 · 8 comments

Comments

Projects
None yet
4 participants
@dominikh
Copy link
Member

commented Sep 23, 2013

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. http://play.golang.org/p/ZUaqjIdkV_
2.
3.

What is the expected output?
No output

What do you see instead?
panic: reflect.Value.Addr of unaddressable value [recovered]
        panic: reflect.Value.Addr of unaddressable value

goroutine 1 [running]:
runtime.panic(0x4986c0, 0xc21000a1e0)
        /home/dominikh/go/src/pkg/runtime/panic.c:266 +0xb6
encoding/json.func·002()
        /home/dominikh/go/src/pkg/encoding/json/encode.go:271 +0xe5
runtime.panic(0x4986c0, 0xc21000a1d0)
        /home/dominikh/go/src/pkg/runtime/panic.c:248 +0x106
reflect.Value.Addr(0x498c40, 0xc2100511c0, 0x172, 0x6, 0x0, ...)
        /home/dominikh/go/src/pkg/reflect/value.go:279 +0x64
encoding/json.addrMarshalerEncoder(0xc2100520b0, 0x498c40, 0xc2100511c0, 0x172, 0x0)
        /home/dominikh/go/src/pkg/encoding/json/encode.go:439 +0x44
encoding/json.(*structEncoder).encode(0xc21003d3c0, 0xc2100520b0, 0x4b63a0,
0xc2100511c0, 0x192, ...)
        /home/dominikh/go/src/pkg/encoding/json/encode.go:597 +0x289
encoding/json.*structEncoder.(encoding/json.encode)·fm(0xc2100520b0, 0x4b63a0,
0xc2100511c0, 0x192, 0x0)
        /home/dominikh/go/src/pkg/encoding/json/encode.go:618 +0x58
encoding/json.(*encodeState).reflectValue(0xc2100520b0, 0x4b63a0, 0xc2100511c0, 0x192)
        /home/dominikh/go/src/pkg/encoding/json/encode.go:305 +0x71
encoding/json.(*encodeState).marshal(0xc2100520b0, 0x4b63a0, 0xc2100511c0, 0x0, 0x0)
        /home/dominikh/go/src/pkg/encoding/json/encode.go:276 +0xa6
encoding/json.Marshal(0x4b63a0, 0xc2100511c0, 0x10, 0x10, 0x4b63a0, ...)
        /home/dominikh/go/src/pkg/encoding/json/encode.go:132 +0x6c
main.main()
        /tmp/NAMz19uqqz.go:14 +0x110


Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Linux

Which version are you using?  (run 'go version')
go version devel +06c20cdf7bc0 Mon Sep 23 18:11:25 2013 +1000 linux/amd64

Please provide any additional information below.
Works in 1.1, doesn't in tip; only happens when marshaling &x followed by x; bisect
points to revision 5a51d54e34bb
@gopherbot

This comment has been minimized.

Copy link

commented Sep 23, 2013

Comment 1 by Lytvynov.A.V:

Clarification: only panics if marshaling non-pointer value after pointer values, not the
other way around.
Most likely cause:
https://code.google.com/p/go/source/browse/src/pkg/encoding/json/encode.go#370
The check for reflect.Value.CanAddr is only performed once per field (then encoderFunc
gets cached) and returns true if passed struct value was a pointer.
reflect.Value.CanAddr returns false for the same field if the containing struct, passed
in, was not a pointer value, thus the panic.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 23, 2013

Comment 2:

Also odd:  http://play.golang.org/p/oL41gqyFYw  
go1.1.2
{"Raw":{"key":"val"}}
{"Raw":"eyAia2V5IjogInZhbCIgfQ=="}
Why does the json.RawMessage encode differently for json.Marshal(&x) vs json.Marshal(x)?
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2013

Comment 3:

RawMessage's methods are on *RawMessage.
@gopherbot

This comment has been minimized.

Copy link

commented Sep 23, 2013

Comment 4 by Lytvynov.A.V:

Exactly. Methods are declared for *RawMessage and struct field is of type RawMessage.
So it probably makes sense to not treat it as json.Marshaler.
In any case, how this field is treated by json.Marshal should not depend on whether the
containing struct was passed as pointer or as a value.
This is caused by the exact line i posted before, by the way. From Brad's example: Raw
is addressable in first case and not addressable in the second (as reported by
reflect.Value.CanAddr).
@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 23, 2013

Comment 5:

Labels changed: added go1.2, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 23, 2013

Comment 6:

Sent out https://golang.org/cl/13839045/

Status changed to Started.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 24, 2013

Comment 7:

This issue was closed by revision 0f3ea75.

Status changed to Fixed.

@dominikh dominikh added fixed labels Sep 24, 2013

jonboulle added a commit to jonboulle/spec that referenced this issue Mar 11, 2015

schema: use pointer to json.RawMessage
RawMessage does not marshal correctly without a pointer receiver:
- golang/go#6528
- golang/go#6458 (comment)

This is only a partial solution as really we should be marshalling the
IsolatorValue rather than the RawMessage (since currently this will only
work when marshalling Isolators that have RawMessage populated).

jonboulle added a commit to jonboulle/spec that referenced this issue Mar 11, 2015

schema: use pointer to json.RawMessage
RawMessage does not marshal correctly without a pointer receiver:
- golang/go#6528
- golang/go#6458 (comment)

This is only a partial solution as really we should be marshalling the
IsolatorValue rather than the RawMessage (since currently this will only
work when marshalling Isolators that have RawMessage populated).

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015

@rsc rsc removed the go1.2 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 25, 2016

@gopherbot

This comment has been minimized.

Copy link

commented Oct 24, 2016

CL https://golang.org/cl/21811 mentions this issue.

This issue was closed.

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