x/net/proxy: doesn't support socks5h:// protocol

[nathanleclaire]

This package supports detecting proxies with protocol socks5://, e.g. socks5://localhost:5000, but not socks5h://. Since this is a common format (e.g. http://blog.mafr.de/2013/11/24/setting-up-a-socks-proxy-using-openssh/) and other tools such as curl support it, this package should likely support it as well. I can potentially submit code to support this if desired.

The offending line: https://github.com/golang/net/blob/master/proxy/proxy.go#L81 . Both seem to be valid to curl, so perhaps both should be supported.

[bradfitz]

Sure. Send a change. I'd hope the change has docs referencing any specs, and tests?

[nathanleclaire]

I'll definitely keep it in mind for any patch. I have to admit my understanding of what supporting socks5h:// would entail was more cursory when first filing this issue (now realize it will be more complicated), but I'm willing to take a look.

[dajohi]

Here is what curl does if socks5h is set

https://github.com/bagder/curl/blob/master/lib/socks.c#L569-L573

[dajohi]

Just glancing at net/proxy/socks5.go in Dial()

        if ip := net.ParseIP(host); ip != nil {
                if ip4 := ip.To4(); ip4 != nil {
                        buf = append(buf, socks5IP4)
                        ip = ip4
                } else {
                        buf = append(buf, socks5IP6)
                }
                buf = append(buf, ip...)
        } else {
                if len(host) > 255 {
                        return nil, errors.New("proxy: destination hostname too long: " + host)
                }
                buf = append(buf, socks5Domain)
                buf = append(buf, byte(len(host)))
                buf = append(buf, host...)
        }

Looks like socks5 is already acting like socks5h?

[nathanleclaire]

@dajohi If that's the case, it might just need to be updated to support socks5h:// notation, and maybe to skip the hostname checking in the case of simply socks5://. I can't find any official RFC describing socks5h:// notation specifically though.

[mahmoud]

Bumping this, since it's we've seen it cause some failures in docker/docker-machine (17.05).

[gopherbot]

Change https://golang.org/cl/156517 mentions this issue: proxy: support socks5h scheme in proxy URL