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: new JoinPath function and method use Path instead of EscapedPath() #53763

Closed
gazerro opened this issue Jul 9, 2022 · 9 comments
Closed

net/url: new JoinPath function and method use Path instead of EscapedPath() #53763

gazerro opened this issue Jul 9, 2022 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.19

Comments

@gazerro
Copy link
Contributor

gazerro commented Jul 9, 2022

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

go version go1.19rc1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

package main

import "net/url"

func main() {
	u, err := url.JoinPath("https://www.example.com/a%2fb", "a%2fb")
	if err != nil {
		panic(err)
	}
	print(u)
}

https://go.dev/play/p/YS_-s65X9n2?v=gotip

What did you expect to see?

https://www.example.com/a%2fb/a%2fb

What did you see instead?

https://www.example.com/a/b/a%2fb

How to fix

In the JoinPath method,

elem = append([]string{u.Path}, elem...)

should be written as

elem = append([]string{u.EscapedPath()}, elem...)
@Stavrospanakakis
Copy link

May I try to take this issue? I'd be very happy to open a pull request to resolve it :)

@seankhliao
Copy link
Member

net/url.URL.ResolveReference uses EscapedPath(), JoinPath should use it too

@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 9, 2022
@seankhliao seankhliao added this to the Go1.19 milestone Jul 9, 2022
@gopherbot
Copy link

Change https://go.dev/cl/416774 mentions this issue: net/url: use EscapedPath for url.JoinPath

@ianlancetaylor
Copy link
Contributor

CC @carlmjohnson @neild

1 similar comment
@ianlancetaylor
Copy link
Contributor

CC @carlmjohnson @neild

@earthboundkid
Copy link
Contributor

This makes sense to me.

@earthboundkid
Copy link
Contributor

The function calls url.Parse, which turns %2f into /. So I think the output should be /a/b/a/b, no?

@seankhliao
Copy link
Member

url.Parse roundtrips /a%2fb when output via .String() and .EscapedPath(), only .Path turns it into /a/b.

@gazerro
Copy link
Contributor Author

gazerro commented Jul 9, 2022

@carlmjohnson See this example

package main

import "net/url"

func main() {
	u, err := url.Parse("https://www.example.com/a%2fb")
	if err != nil {
		panic(err)
	}
	println(u.String())              // print: https://www.example.com/a%2fb
	println(u.JoinPath("").String()) // print: https://www.example.com/a/b
}

JoinPath shouldn't change the URL.

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@seankhliao @jproberts
net/url: use EscapedPath for url.JoinPath
daaf947 
Fixes golang#53763

Change-Id: I08b53f159ebdce7907e8cc17316fd0c982363239
Reviewed-on: https://go-review.googlesource.com/c/go/+/416774
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jul 11, 2023
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.19
Development

No branches or pull requests
6 participants
@earthboundkid @gazerro @ianlancetaylor @gopherbot @seankhliao @Stavrospanakakis