Skip to content

encoding/json: RawMessage should marshal without pointer receiver #6528

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.

Sign up for GitHub

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
Closed
augustoroman opened this issue Oct 2, 2013 · 6 comments
Closed

encoding/json: RawMessage should marshal without pointer receiver #6528

augustoroman opened this issue Oct 2, 2013 · 6 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone
Unplanned

Comments

@augustoroman
Copy link
Contributor 

The current json.RawMessage implementation defines MarshalJSON on a pointer receiver. 
This causes unexpected behavior when marshalling with non-pointer-receiver objects.

For example:
http://play.golang.org/p/Cf_6zpIKC0

Instead, MarshalJSON should be defined on a non-pointer receiver for json.RawMessage. 
This will continue to work for existing code that uses pointer-receivers, but will
change behavior to work "correctly" on code that accidentally uses
non-pointer-receivers.
@robpike
Copy link
Contributor

robpike commented Oct 3, 2013

Comment 1:

Can't be changed but worth documenting.

Labels changed: added priority-later, documentation, removed priority-triage.

Status changed to Accepted.

@gopherbot
Copy link
Contributor

Comment 2 by mattyjwilliams:

What's the solution here? Adding a comment to
http://golang.org/src/pkg/encoding/json/stream.go?s=4434:4484#L175 to warn that calling
json.Marshal on a non-pointer json.RawMessage can return "meaningless" data with err ==
nil

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 3:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added repo-main.

@augustoroman augustoroman added accepted Documentation Issues describing a change to documentation. labels Dec 4, 2013
jonboulle added a commit to jonboulle/spec that referenced this issue Mar 11, 2015
@jonboulle
schema: use pointer to json.RawMessage
4b15352 
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
@jonboulle
schema: use pointer to json.RawMessage
433a328 
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 Unplanned milestone Apr 10, 2015
kylec1 added a commit to dropbox/changes-client that referenced this issue Oct 26, 2015
@kylec1
Print debugConfig if supplied; fix RawMessage use
c8780d6 
Summary:
When we use debugConfig, it's nice to say what tweaks happened in the log.
To avoid golang/go#6528, needed to switch to using *json.RawMessage.

Test Plan: Existing unit doesn't cover it, sadly, but should be quite safe.

Reviewers: nate

Reviewed By: nate

Subscribers: changesbot, anupc

Differential Revision: https://tails.corp.dropbox.com/D147620
@cespare cespare mentioned this issue Apr 13, 2016
Closed
@rsc
Copy link
Contributor

rsc commented Oct 24, 2016

Dup of #14493.

@rsc rsc closed this as completed Oct 24, 2016
@golang golang locked and limited conversation to collaborators Oct 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@rsc @augustoroman @robpike @gopherbot