net/url: JoinPath() drops trailing slashes

[drigz]

What version of Go are you using (go version)?

dev branch (playground)

Does this issue reproduce with the latest release?

No, url.JoinPath() is not in Go 1.18.

What operating system and processor architecture are you using (go env)?

dev branch (playground)

What did you do?

https://go.dev/play/p/xCJcVPN5pPh?v=gotip

func main() {
	fmt.Println(url.JoinPath("http://host/foo", "bar/"))
}

What did you expect to see?

The trailing slash should be preserved: http://host/foo/bar/

What did you see instead?

The trailing slash is lost: http://host/foo/bar

[earthboundkid]

Yes, I think that we shouldn't be following path.Clean quite so closely here.

The quick fix is to change

	if len(elem) > 0 {
		elem = append([]string{u.Path}, elem...)
		url.setPath(path.Join(elem...))
	}

to

	if len(elem) > 0 {
		elem = append([]string{u.Path}, elem...)
                p := path.Join(elem...)
                if strings.HasSuffix(elem[len(elem)-1], "/") {
                   p += "/"
                }
		url.setPath(p)
	}

Another approach is to copy and modify path.Join. Doing a copy could save some allocations by working directly in a strings.Builder instead of bouncing between []byte and string.

[ianlancetaylor]

@gopherbot Please open backport to 1.18

This function is new in 1.18 so we should keep it consistent with this change.

[gopherbot]

Backport issue(s) opened: #52087 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[gopherbot]

Change https://go.dev/cl/397256 mentions this issue: net/url: preserve a trailing slash in JoinPath

[earthboundkid]

The function is slated for 1.19. It missed 18.

[ianlancetaylor]

Whoops. Thanks.

[earthboundkid]

What should url.JoinPath do in these cases:

url.JoinPath("https://example.com/a//")
url.JoinPath("https://example.com/a//", "")

I think the output should be "https://example.com/a//" and "https://example.com/a/" but I can be argued into thinking otherwise.

[ianlancetaylor]

Personally I think it's OK if JoinPath drops extra trailing slashes. I guess it should be documented, though.