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.

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

net/url: Userinfo should support encoding/gob #10964

Closed
chowey opened this issue May 27, 2015 · 6 comments
Closed

net/url: Userinfo should support encoding/gob #10964

chowey opened this issue May 27, 2015 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8Maybe

Comments

@chowey
Copy link

chowey commented May 27, 2015

Currently the encoding/gob package does not support embedded structs with no exported fields. See #5819.

This is problematic for the net/url package. I cannot serialize a url.URL into a gob because url.Userinfo has no exported fields. I can't be the only one who has needed to serialize a struct with a URL in it.

This is solvable without modification to the encoding/gob if we fix url.Userinfo. This is possible by implementing encoding.BinaryMarshaler and encoding.BinaryUnmarshaler:

func (u *Userinfo) UnmarshalBinary(data []byte) error {
    return nil
}

func (u *Userinfo) MarshalBinary() (data []byte, err error) {
    return nil, nil
}
@bradfitz
Copy link
Contributor

Or perhaps encoding/gob should let you register marshal/unmarshal funcs for a given concrete type yourself, then net/url can remain happily oblivious of gob.

/cc @robpike

@bradfitz bradfitz changed the title net/url Userinfo should support encoding/gob net/url: Userinfo should support encoding/gob May 27, 2015
@nightlyone
Copy link
Contributor

Implementing encoding.BinaryMarshaler and encoding.BinaryUnmarshaler seems very sane to me, since it solves that problem for other binary encodings, too. Please remember that the generic marshaler/unmarshaler support in package encoding has been introduced exactly to solve those problems in a generic way and per type that needs it.

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jun 3, 2015
@rsc
Copy link
Contributor

rsc commented Nov 5, 2015

If we're going to do something for url.URL, why not give URL MarshalText/UnmarshalText methods instead? Why work on this tiny piece of the URL instead of the whole thing?

That said, we have to proceed very carefully here because of backwards compatibility concerns. It may not happen for 1.6.

@chowey
Copy link
Author

chowey commented Nov 5, 2015

This works too. MarshalText could call URL.String, and UnmarshalText could call url.Parse. It is very straightforward.

How would this break backward compatibility? If someone is relying on the fact that url.URL cannot be marshaled or unmarshaled?

@rsc rsc modified the milestones: Unplanned, Go1.6 Dec 3, 2015
@rsc
Copy link
Contributor

rsc commented Dec 3, 2015

@chowey It's a change in behavior. It can surprise people. And if it can, it will. We've struggled with this when adding custom encodings for other types as well.

@bradfitz bradfitz modified the milestones: Go1.8Maybe, Unplanned May 6, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Oct 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
7 participants
@bradfitz @rsc @quentinmit @chowey @nightlyone @ianlancetaylor @gopherbot