net/url: Parse returns URL whose Host contains colon

[crvv]

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

1.10.2 and tip(5776bd5)

Does this issue reproduce with the latest release?

Yes

What did you do?

https://play.golang.org/p/vYNt0qipkxf
Parse an invalid URL, http://socks5h://server

What did you see?

&url.URL{Scheme:"http", Host:"socks5h:", Path:"//server"}

What did you expect to see?

An error.
As per https://tools.ietf.org/html/rfc3986#section-3.2.2, host can't contain :

host        = IP-literal / IPv4address / reg-name
reg-name    = *( unreserved / pct-encoded / sub-delims )
unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
pct-encoded = "%" HEXDIG HEXDIG
reserved    = gen-delims / sub-delims
gen-delims  = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
            / "*" / "+" / "," / ";" / "="

[fraenkel]

While you are correct about : being invalid, the URL.Host allows host:port which is why the : is present.

[andybons]

@bradfitz

[bradfitz]

Every time we change the URL package we break tons of code. That might not happen in this case, but we've said that dozens of times before.

So I'd rather not change anything here. We don't strictly follow the RFCs for legacy reasons. There would need to be a good reason to change something here.

I also note that if I copy/paste these into Chrome, it works fine:

     https://golang.org:/
     https://golang.org:/ref/spec

(note the colons in the authority with an empty port)

I also note that this doesn't break the Port method on URL: https://play.golang.org/p/I3OO4NvwBTb

So what do we gain by changing something?

[crvv]

The issue comes up when I use proxy for http package.

package main
import "net/http"
func main() {
    _, err := http.Get("https://golang.org")
    println(err.Error())
}

And run it with HTTPS_PROXY=socks5h://localhost go run main.go
The error is Get https://golang.org: proxyconnect tcp: dial tcp: lookup socks5h: no such host
I think this is an unexpected error.

http package prepends http:// to the proxy url at this line.

go/src/net/http/transport.go

Line 299 in 65c365b

if proxyURL, err := url.Parse("http://" + proxy); err == nil {

Then http will try to connect to socks5h:.
Maybe it is better to handle this in http package. #24135

[crvv]

For url package, I just found a duplicate issue
#14836

According to https://tools.ietf.org/html/rfc3986#section-6.2.3
http://example.com:/ is a valid url, and its host is example.com, not example.com:
But I think this should be handled in url package, not in http.

[crvv]

I just found that url accepts http://fe80::/foo, and host is an IPv6 address fe80::
Then trimming the rightmost : correctly is not trivial
So I don't think it's worth changing anything for this

[adamdecaf]

So I don't think it's worth changing anything for this

Sounds like we should close this then?