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/gob: Go 1.2 cannot decode Go 1.1 gobs containing net.IP #6760

Closed
rsc opened this issue Nov 13, 2013 · 5 comments
Closed

encoding/gob: Go 1.2 cannot decode Go 1.1 gobs containing net.IP #6760

rsc opened this issue Nov 13, 2013 · 5 comments
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Nov 13, 2013

In Go 1.2, encoding/gob respects the new encoding.TextMarshaler and
encoding.TextUnmarshaler methods, and net.IP adds those methods.

Unfortunately, this means that the gob encoding of a net.IP changes from Go 1.1 to Go
1.2. That by itself might be okay: I don't believe people expect that Go 1.1 will be
able to read Go 1.2 gobs.

However, gob is also a bit picky about the decoding: if the type has a TextUnmarshaler,
gob expects to get the TextMarshaler version of the encoding and does not accept an
encoding of the underlying data. 

A Go 1.1 gob encoding of a net.IP will send the gob encoding of a standard []byte. Go
1.2 gob will refuse to decode that form.

The only other type we have added a MarshalText or MarshalBinary method to is time.Time.
But time.Time already had a GobEncode and GobDecode methods, so its behavior with gob
does not change. It appears that net.IP is the only affected type in the standard
library.

Possibilities:

1. Do nothing; allow Go 1.1 and Go 1.2 to be mutually unintelligible in gob when there
is a net.IP involved.

2. Change gob to accept the concrete data representation during decode, so that Go 1.1
can talk to Go 1.2 but not vice versa.

3. Change gob to ignore MarshalText/UnmarshalText, allowing only
MarshalBinary/UnmarshalBinary.

4. Remove net.IP's MarshalText/UnmarshalText methods, breaking encoding/json's support
for net.IP (new in Go 1.2).

5. Replace net.IP's MarshalText/UnmarshalText methods with MarshalJSON/UnmarshalJSON, so
that we keep encoding/json working well for Go 1.2. (Of course, the new generic methods
were designed explicitly to avoid making package net mention JSON, so it is a bit of a
shame not to use them.)

I am leaning toward 5, which seems like the most limited change.

Rob, what do you think?
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 13, 2013

Comment 1:

Other possibility: changing encoding/gob to special-case the net.IP type, if that's the
only type affected.  But that potentially means adding a dependency on net from gob.
@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 13, 2013

Comment 2:

#5 is worse than I thought, because we'd also have to put XML methods in, and you can't
do that without importing encoding/xml, which we absolutely won't do. That was the even
bigger reason for the new generic methods.
Now leaning toward #3.
@minux
Copy link
Member

@minux minux commented Nov 13, 2013

Comment 3:

i think #3 or #4 is better.
so you won't end up with something we must  regretfully support forever.
making net relying on encoding/json is not ok IMO.
@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 14, 2013

Comment 4:

This issue was closed by revision 7dd086e.

Status changed to Fixed.

@adg
Copy link
Contributor

@adg adg commented Nov 18, 2013

Comment 5:

This issue was closed by revision a30cab4951a3.

@rsc rsc added fixed labels Nov 18, 2013
@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
adg added a commit that referenced this issue May 11, 2015
…lText

««« CL 22770044 / 23fc3139589c
encoding/gob: do not use MarshalText, UnmarshalText

This seems to be the best of a long list of bad ways to fix this issue.

Fixes #6760.

R=r
CC=golang-dev
https://golang.org/cl/22770044
»»»

R=golang-dev
CC=golang-dev
https://golang.org/cl/28110043
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.