net/url: add JoinPath, URL.JoinPath

[wanglong001]

Url Join is often used

Unsatisfactory using path.Join： http://www -> http:/www

I want to add Join method to url package

// Join  concatenates baseUrl and the elements
// - check baseUrl format
// - concatenates baseUrl and the elements
func Join(baseUrl string, elem ...string) (result string, err error) {
	url, err := Parse(baseUrl)
	if err != nil {
		return
	}
	if len(elem) > 0 {
		elem = append([]string{url.Path}, elem...)
		url.Path = path.Join(elem...)
	}
	result = url.String()
	return
}

unit test

func TestJoin(t *testing.T) {
	type args struct {
		baseUrl string
		elem    []string
	}
	tests := []struct {
		name       string
		args       args
		wantResult string
		wantErr    bool
	}{
		{
			name: "test normal url",
			args: args{
				baseUrl: "https://go.googlesource.com",
				elem:    []string{"go"},
			},
			wantResult: "https://go.googlesource.com/go",
			wantErr:    false,
		},
		{
			name: "test .. parent url",
			args: args{
				baseUrl: "https://go.googlesource.com/a/b/c",
				elem:    []string{"../../../go"},
			},
			wantResult: "https://go.googlesource.com/go",
			wantErr:    false,
		},
		{
			name: "test . cul path",
			args: args{
				baseUrl: "https://go.googlesource.com/",
				elem:    []string{"./go"},
			},
			wantResult: "https://go.googlesource.com/go",
			wantErr:    false,
		},
		{
			name: "test multiple Separator",
			args: args{
				baseUrl: "https://go.googlesource.com//",
				elem:    []string{"/go"},
			},
			wantResult: "https://go.googlesource.com/go",
			wantErr:    false,
		},
		{
			name: "test more elems",
			args: args{
				baseUrl: "https://go.googlesource.com//",
				elem:    []string{"/go", "a", "b", "c"},
			},
			wantResult: "https://go.googlesource.com/go/a/b/c",
			wantErr:    false,
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			gotResult, err := Join(tt.args.baseUrl, tt.args.elem...)
			if (err != nil) != tt.wantErr {
				t.Errorf("Join() error = %v, wantErr %v", err, tt.wantErr)
				return
			}
			if gotResult != tt.wantResult {
				t.Errorf("Join() = %v, want %v", gotResult, tt.wantResult)
			}
		})
	}
}

[gopherbot]

Change https://golang.org/cl/332209 mentions this issue: net/url: add Join method

[earthboundkid]

I actually just implemented something like this for my request client library: earthboundkid/requests@cbdbf91

I think a slightly better name is JoinPaths because it shouldn't do any host changing, just the path.

[earthboundkid]

I also propose that it should be a method on URL, rather than package level function.

// JoinPaths joins the provided path to any existing Path 
// and cleans the result of any ./ or ../ elements.
func (u *URL) JoinPaths(paths ...string) {
	if len(paths) == 0 { 
		return
	}
	paths = append([]string{url.Path}, paths...)
	url.Path = path.Join(paths...)
	return
}

[earthboundkid]

One complication: I think it's quite reasonable to expect that paths which begin with / will be interpreted as absolute (which is what URL.ResolveRef does), but that isn't what you implemented and might be surprising to users either way.

[wanglong001]

Like this topic

It continues to be paid attention to. Path.Join cannot be used directly. Generally, the string type is converted to the url type, then the error is checked, and then ResolveReference or path.join, and finally the complete url path is output. For ordinary users, this process is very repetitive

[earthboundkid]

I agree that this is probably worth doing on some level, but it's not totally clear what semantics users expect. For example, if we used ResolveReference as the standard, we get very different results:

func Join(u *url.URL, path string) {
	*u = *u.ResolveReference(&url.URL{Path: path})
}

func main() {
	u, _ := url.Parse("http://example.com")
	fmt.Println(u)
	for _, p := range []string{"a/", "b/", "c", "d", "../e", "/f"} {
		Join(u, p)
		fmt.Println(u)
	}
}

Results:

http://example.com
http://example.com/a/
http://example.com/a/b/
http://example.com/a/b/c
http://example.com/a/b/d
http://example.com/a/e
http://example.com/f

[wanglong001]

Yes, using ResolveReference will indeed confuse,
In my opinion, the function of url.Join should be similar to path.Join.
The same is true for the literal understanding

[earthboundkid]

Here is a note on surprising behavior with Python’s url.parse: https://utcc.utoronto.ca/~cks/space/blog/python/UrllibParsePartialURLs

Now suppose someone accidentally creates a URL for a web page of yours that looks like 'https://example.org//your/page/url' (with two slashes after the host instead of one) and visits it, and you attempt to decode the result of what Apache will hand you:

>>> urllib.parse.urlparse("//your/page/url")
ParseResult(scheme='', netloc='your', path='/page/url', params='', query='', fragment='')

The problem here is that '//ahost.org/some/path' is a perfectly legal protocol-relative URL, so that's what urllib.parse will produce when you give it something that looks like one, which is to say something that starts with '//'.

This could be a problem for url.Join as well. Either assuming that // is wrong and should be / or assuming it’s right could lead to surprising behavior depending on the context.

[rsc]

If we do add this, it seems like Join should be restricted to manipulating the path, as in the top StackOverflow answer:

u, err := url.Parse("http://foo")
u.Path = path.Join(u.Path, "bar.html")
s := u.String() // prints http://foo/bar.html

That is, we should not be doing anything with or like ResolveReference, which has to deal with full URL syntax as the second argument. Maybe we should make that clear by calling it url.JoinPath?

So the API would be:

func JoinPath(baseURL string, elem ...string) (string, error)
func (u *URL) JoinPath(elem ...string) (*URL, error) 

Do I have that right?

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[wanglong001]

@rsc Thank you for your answer

Does this API need to return a new object URL? Or modify the Path directly on the original object, which of these two methods is more suitable?

func (u *URL) JoinPath(elem ...string) (*URL, error) 

1

func (u *URL) JoinPath(elem ...string)  *URL{
       url := URL{
             Scheme : u.Scheme
	     Opaque : u.Opaque
	     User   : u.User
	     Host   : u.Host
	     Path       : u.Path
	     RawPath     : u.RawPath
	     ForceQuery  : u.ForceQuery
	     RawQuery     : u.RawQuery
	     Fragment    : u.Fragment
	     RawFragment  : u.RawFragment
        }
	if len(elem) > 0 {
		elem = append([]string{u.Path}, elem...)
		url.Path = path.Join(elem...)
	}
      return  &url
}

func JoinPath(baseUrl string, elem ...string) (result string, err error) {
	url, err := Parse(baseUrl)
	if err != nil {
		return
	}
       urlAfterJoin := url.JoinPath(elem...)
	result = urlAfterJoin.String()
	return
}

2

func (u *URL) JoinPath(elem ...string) {
	if len(elem) > 0 {
		elem = append([]string{u.Path}, elem...)
		u.Path = path.Join(elem...)
	}
}

func JoinPath(baseUrl string, elem ...string) (result string, err error) {
	url, err := Parse(baseUrl)
	if err != nil {
		return
	}
	url.JoinPath(elem...)
	result = url.String()
	return
}

[rsc]

I don't believe we have any methods on URL today that modify the receiver (except Unmarshal).
It seems like a mistake to start.
Especially since path.Join returns a new thing rather than modifying anything.

Any other objections to the API in #47005 (comment)?

[wanglong001]

@rsc hi, I’m not sure if I really understand what you mean, I resubmitted the code, please help me review it again, thank you

[earthboundkid]

This is a cleaner implementation, but I'm not sure if package url can import path without causing a dependency cycle somewhere:

func (u *URL) JoinPath(elem ...string)  *URL{
	u2 := *u
	if len(elem) > 0 {
		elem = append([]string{u.Path}, elem...)
		u2.Path = path.Join(elem...)
	}
      return  &u2
}

func JoinPath(baseUrl string, elem ...string) (result string, err error) {
	u, err := Parse(baseUrl)
	if err != nil {
		return
	}
    
	return u.JoinPath(elem...).String()
}

[rsc]

It should be OK for net/url to import path. path definitely doesn't import url.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

[gopherbot]

Change https://golang.org/cl/374654 mentions this issue: net/url: add JoinPath, URL.JoinPath

[drigz]

Thank you for adding this, looks useful! Is it intended that the new method strips trailing / characters? url.JoinPath("http://host/foo", "bar/") == "http://host/foo/bar", nil

This is not an important distinction for filesystem paths but sometimes makes an important difference for URLs, depending on the server, so I can imagine it surprising the user. The behavior isn't documented for JoinPath in the linked change.

[earthboundkid]

Hmm, it does seem like it should preserve the trailing slash, IMO. File a new issue so we can decide this before 1.19 ships.

[drigz]

Done, thanks! #52074