proposal: net/url: URL.Clone, Values.Clone

[seankhliao]

Proposal Details

Proposed addition:

// Clone returns a copy of u
func (u *URL) Clone() *URL

// Clone returns a copy of v
func (v Values) Clone() Values

Motivation

Right now, there's no clear way to safely copy a net/url.URL: #38351
As the linked issue states, it is safe to do url2 := *url1, but it's not clear to users.
For example a rough (String/Parse are common names, so guestimate with u) search shows:

• 21.2k hits for url2 = *url1: https://github.com/search?q=language%3Ago+%2F%28.*%29u%28rl%29%3F+%3A%3F%3D+%5C*%28.*%29u%28rl%29%3F%2F&type=code
• 3.3k hits for url2, _ = url.Parse(url1.String()): https://github.com/search?q=language%3Ago+%2Furl%5C.Parse%5C%28.*u%28rl%29%3F.%5CString%5C%28%5C%29%5C%29%2F&type=code

That is a significant portion (1/8) of users paying the additional cost of serialization/deserialization for what could be a simple copy.
The parse variant also means having to deal with an error return variable that should always be nil.

The safe and efficient way of doing it should be:

go/src/net/http/clone.go

Line 41 in 49860cf

func cloneURL(u *url.URL) *url.URL {

For Values, because of its nested map[string][]string structure, a deep clone is necessary (and not possible with . While it is less common than a url's Parse(u.String()), I think it should be available:

• 148: https://github.com/search?q=language%3Ago+%2FParseQuery%5C%28.*%5C.Encode%5C%28%5C%29%5C%29%2F&type=code

cc @neild @rsc

[gabyhelp]

Related Issues

• proposal: net/url: add URL.Clone method #41733 (closed)
• net/url: clarify safe way to do URL deep-copying #38351
• proposal: net/url: url manipulation after creation/parsing #40239 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[andig]

@seankhliao I'm probably missing something, but shouldn't url.Clone also duplicate the UserInfo and hence differ from *url?

[seankhliao]

please read #41733

I suppose someone can do *u2.User = <something new>, though nobody does.
We can add an additional check for it, as in
https://github.com/golang/go/blob/go1.19/src/net/http/clone.go#L28

[earthboundkid]

This is really two proposals in one.

• http.Header has .Clone(), so I don't see a good reason for not having it on url.Values, which is the same thing except for the canonicalization/stringification process.
• As in proposal: net/url: add URL.Clone method #41733, I think url.URL.Clone() would be helpful from a documentation point of view.

[sudomateo]

For Values, because of its nested map[string][]string structure, a deep clone is necessary (and not possible with . While it is less common than a url's Parse(u.String()), I think it should be available:

Is this thought incomplete?

please read #41733

Instead of referencing other issues and pulling people's focus off this one could we summarize what context we expect readers to come away with after reading a linked issue? I'm assuming in this case the context is that *UserInfo is immutable. How will that affect this proposal?

---

This is really two proposals in one.

Agreed. I can clearly see the use case for a url.Values.Clone() method. I've been guilty of using a combination of Encode and ParseQuery to build multiple URLs with different query parameters (e.g., database URLs).

However, I don't personally see the value in a url.URL.Clone() method outside being beneficial for documentation, which is still pretty beneficial overall. Generally when I'm building multiple URLs I'll start with a *url.URL as a base URL and then update its path and query parameters and save the desired multiple URLs as strings along the way. Can we elaborate on use cases of a potential url.URL.Clone() in this proposal outside of saved serialization and deserialization cost?

[seankhliao]

reading past proposals is critical to not rehash past discussions. please do that, especially since this is the previous proposal, but with evidence backing up why the current situation is deficient.

given the widespread use of cloning in existing codebases (see the search results), I don't think it's necessary to list the use cases. it's clearly something people need and do, but inefficiently.
if you don't need it, then you don't.

[sudomateo]

reading past proposals is critical to not rehash past discussions. please do that, especially since this is the previous proposal, but with evidence backing up why the current situation is deficient.

I did read the linked issue. That's why I asked what context you're referring to so we can all be on the same page. Specifically around *UserInfo cloning and how it affects this proposal.

given the widespread use of cloning in existing codebases (see the search results), I don't think it's necessary to list the use cases. it's clearly something people need and do, but inefficiently. if you don't need it, then you don't.

With all due respect, and I do truly mean that as someone that's grateful that you submitted this proposal, this perspective isn't helpful to anyone or any proposal and comes off a bit paternalistic. The search results provide data but that data is not a substitute for defining and writing down the use cases so future readers can understand the motivation for this proposal. Especially when there are open questions around the still unclear behavior of *UserData. If cloning URLs is something "people clearly need and do" then defining the use cases and writing them down should be simple for this proposal.

[seankhliao]

The confusion with UserInfo is what lead to the current situation, how exactly it works isn't really relevant beyond that it should be unambiguously safe and efficient with the proposed api.

I don't see how knowing what people use it for will change the evaluation of the proposal. In fact, I think it's a distraction from the proposal, as it's not to make a new capability possible.

The motivation is as the original post, driven by real world usage data: the current api (or lack thereof) leads to suboptimal performance and ergonomics in a large number of existing programs when manipulating URLs. Since safely modifying URLs is not a pattern to discourage, it should be made better.

[sudomateo]

The confusion with UserInfo is what lead to the current situation, how exactly it works isn't really relevant beyond that it should be unambiguously safe and efficient with the proposed api.

If the confusion is what led us to the current situation then the confusion should be documented in the proposal so that, again, we're all on the same page. The proposal lists an API but does not discuss the implementation or note the *UserInfo confusion.

I don't see how knowing what people use it for will change the evaluation of the proposal. In fact, I think it's a distraction from the proposal, as it's not to make a new capability possible.

Programming languages are built for people. Knowing how people use the features in a programming language and, more importantly, writing it down is how we can ensure the work done via a proposal is the right work and positively impacts the community.

The motivation is as the original post, driven by real world usage data: the current api (or lack thereof) leads to suboptimal performance and ergonomics in a large number of existing programs when manipulating URLs. Since safely modifying URLs is not a pattern to discourage, it should be made better.

The data is limited to code on GitHub that matches a specific pattern which is not representative of all URL cloning usage or all Go code. Granted, no data set can truly be representative of every user but I digress. The data tells us that some users are cloning URLs but it does not tell us whether there's a critical performance issue in these code paths or whether users even care about having a new API for cloning URLs. This is the importance of gathering user feedback rather than relying strictly on "the data" to make decisions. I've seen past Go proposals rely strictly on data to the detriment of the community.

That is a significant portion (1/8) of users paying the additional cost of serialization/deserialization for what could be a simple copy.

I do have a question born out of curiosity. How was it calculated that the data collected represents an eighth of users? Perhaps I'm simply missing something.

Lastly, I fear that I know what's next based on these discussions. This is mainly for the lurkers here but I have to say something because I'm tired of seeing the same behavior play out again and again. Someone will come in and say that these comments are "off topic" or "distracting" and pull up some code of conduct or rules to wield as a threat of banning if the discussions do not cease. Outside of my question above I have nothing further to add on this proposal since it's clear that the original poster is not interested in answering the questions I've asked. Thank you again for this proposal and good luck with it!