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

net: clarify safe way to do URL deep-copying #38351

Open
rasky opened this issue Apr 10, 2020 · 4 comments
Open

net: clarify safe way to do URL deep-copying #38351

rasky opened this issue Apr 10, 2020 · 4 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rasky
Copy link
Member

rasky commented Apr 10, 2020

It sometimes happens to have a need to duplicate (aka "deep copy") a net.URL object.

My understanding is that net.URL can be safely duplicated by simply copying the structure:

   newUrl := *url

Since net.URL is usually handled by pointer, it's normal to check the documentation to see whether the object contains internal pointers, in which case duplication might get hairy. net.URL has the following contents:

type URL struct {
    Scheme     string
    Opaque     string    // encoded opaque data
    User       *Userinfo // username and password information
    Host       string    // host or host:port
    Path       string    // path (relative paths may omit leading slash)
    RawPath    string    // encoded path hint (see EscapedPath method); added in Go 1.5
    ForceQuery bool      // append a query ('?') even if RawQuery is empty; added in Go 1.7
    RawQuery   string    // encoded query values, without '?'
    Fragment   string    // fragment for references, without '#'
}

So there is indeed a pointer in there. This brought people to come up with alternative ways of duplicating a URL such as a String/Parse roundtrip:
https://medium.com/@nrxr/quick-dirty-way-to-deep-copy-a-url-url-on-go-464312241de4

The trick here is that User *Userinfo is actually "immutable". This is documented in Userinfo: The Userinfo type is an immutable encapsulation of username and password [...]. So it looks like that, given this guarantee of immutability, it is indeed safe to simply copy a URL instance to duplicate it. Copying a URL instance is also performed by the standard library, for instance:

*r2 = *r

Hence the subject of this issue:

  • Can I get a confirmation that copying a URL is the correct way and not just an accident of its current implementation (so, it's guaranteed by Go 1.0 compatibility)?
  • Should we document it a little bit better, since it tends to be confusing to newcomers to this library? Either explicitly stating the correct way of copying it, or moving the "immutable" word in the inline documentation of the User *Userinfo field (or both?).
@ALTree ALTree added Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 10, 2020
@abemotion
Copy link

abemotion commented Apr 22, 2020

User       *Userinfo // username and password immutable information

This documentation is better?

@nrxr
Copy link

nrxr commented Apr 26, 2020

Hi @rasky,

Indeed it's safe and actually I, the author of that Medium post, used the newU := *u method in a PR that got merged some weeks ago: https://github.com/golang/go/pull/35578/files#diff-6c2d018290e298803c0c9419d8739885R824-R837

@icholy
Copy link

icholy commented Oct 29, 2020

Relevant comment #41733 (comment) by @rsc

I still don't understand what exactly is worth calling out in URL's documentation.
It is a general property of data that if you want to make a copy before mutating you can do u2 := *u; mutate u2.

And I still don't understand how often this operation is needed on URLs.
@ainar-g has seen it, but that establishes existence not frequency.

@realitycheck
Copy link

realitycheck commented Aug 4, 2022

There is something confusing about #41733 due to there actually is cloneURL function in net/http internals not just a u2 := *u thing, however it's not a part of url.URL public protocol itself for some reason.

@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants