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

Jump to bottom

net/url: Parse documentation does not adequately explain escaping rules or RFC compliance #30611

Open
neilalexander opened this issue Mar 5, 2019 · 6 comments
Open

net/url: Parse documentation does not adequately explain escaping rules or RFC compliance #30611

neilalexander opened this issue Mar 5, 2019 · 6 comments
Labels
Documentation help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@neilalexander
Copy link
Contributor

neilalexander commented Mar 5, 2019 •
edited

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

$ go version
go version go1.11.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes, see playground.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/neilalexander/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/neilalexander/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cv/wv7k9w2s4qdfjfd60nd7_5t40000gn/T/go-build007721452=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

See playground:

if _, err := url.Parse("http://[fe80::1%en0]:12345"); err != nil {
	panic(err)
}

What did you expect to see?

IPv6 link-local literals should be accepted and fe80::1%en0 should be returned within the Host.

What did you see instead?

Parsing error due to invalid URL escape with the percent sign - in above example produces:

panic: parse http://[fe80::1%en0]:12345: invalid URL escape "%en"
@mikioh mikioh changed the title net/url: url.Parse fails on link-local IPv6 address literals net/url: Parse fails on link-local IPv6 address literals Mar 6, 2019
@mikioh
Copy link
Contributor

mikioh commented Mar 6, 2019

See https://tools.ietf.org/html/rfc6874, the delimiter is "%25". Also, #20669 might describe the limitation of IPv6 LLA /w zone in URI.

@mikioh mikioh closed this as completed Mar 6, 2019
@neilalexander
Copy link
Contributor Author

Thanks for the fast response. If not an issue with Go per-se then I consider this an issue with documentation, as the godoc for url.Parse does not state compliance to RFC 6874, nor does it suggest that there are particular escape codes that may cover situations like this.

@mikioh
Copy link
Contributor

mikioh commented Mar 6, 2019

Then, please re-title this issue.

@mikioh mikioh reopened this Mar 6, 2019
@neilalexander neilalexander changed the title net/url: Parse fails on link-local IPv6 address literals net/url: Parse documentation does not adequately explain escaping rules or RFC compliance Mar 6, 2019
@agnivade agnivade added this to the Unplanned milestone Mar 6, 2019
@ldb
Copy link

ldb commented Apr 5, 2019

Would adding "IPv6 addresses must be encoded according to RFC 3986 and 6874." to the documentation of Parse and ParseRequestURI adequately explain the rules? If yes, I would like to make a CL. However I see that compliance is already documented in line 625, so I am not sure if it is really needed.

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2019
@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

@neilalexander, would @Cosmonawt's proposed change address your issue?

(@rsc and @bradfitz are listed as the owners of this package, so I'll leave it to them to decide whether such a change would be a net improvement from a maintainer's perspective.)

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 12, 2019
@neilalexander
Copy link
Contributor Author

I think it would help, but it would be better if it makes an explicit mention of the fact that it affects link-local addresses containing the interface specifier as it will save people time in the future.

@gknw gknw mentioned this issue Sep 7, 2022
Open
@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@mikioh @neilalexander @agnivade @bcmills @gopherbot @ldb @seankhliao