[go] net/http: add support for socks5 proxy
Michel Lespinasse (Gerrit)
Michel Lespinasse has uploaded this change for review.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/net/http/transport.go 1 file changed, 188 insertions(+), 11 deletions(-)
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 571943d..215a1fa 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -23,6 +23,7 @@
"net/http/httptrace"
"net/url"
"os"
+ "strconv"
"strings"
"sync"
"sync/atomic"
@@ -275,13 +276,17 @@
return nil, nil
}
proxyURL, err := url.Parse(proxy)
- if err != nil || !strings.HasPrefix(proxyURL.Scheme, "http") {
+ if err != nil ||
+ (proxyURL.Scheme != "http" &&
+ proxyURL.Scheme != "https" &&
+ proxyURL.Scheme != "socks5") {
// proxy was bogus. Try prepending "http://" to it and
// see if that parses correctly. If not, we fall
// through and complain about the original one.
if proxyURL, err := url.Parse("http://" + proxy); err == nil {
return proxyURL, nil
}
+
}
if err != nil {
return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
@@ -964,6 +969,169 @@
}
}
+const socks5Version = 5
+
+const (
+ socks5AuthNone = 0
+ socks5AuthPassword = 2
+)
+
+const socks5CmdConnect = 1
+
+const (
+ socks5IP4 = 1
+ socks5Domain = 3
+ socks5IP6 = 4
+)
+
+var socks5Errors = []string{
+ "",
+ "general failure",
+ "connection forbidden",
+ "network unreachable",
+ "host unreachable",
+ "connection refused",
+ "TTL expired",
+ "command not supported",
+ "address type not supported",
+}
+
+// socks5Connect takes an existing connection to a socks5 proxy,
+// and commands the proxy to extend that connection to targetAddr,
+// which is always a canonicalAddr with a host and port.
+// function is largely lifted from x/net/proxy.socks5.Dial()
+func socks5Connect(conn net.Conn, proxyURL *url.URL, targetAddr string) error {
+ host, portStr, err := net.SplitHostPort(targetAddr)
+ if err != nil {
+ return err
+ }
+ // XXX error if len(host) >= 256
+
+ port, err := strconv.Atoi(portStr)
+ if err != nil {
+ return errors.New("proxy: failed to parse port number: " + portStr)
+ }
+ if port < 1 || port > 0xffff {
+ return errors.New("proxy: port number out of range: " + portStr)
+ }
+
+ // the size here is just an estimate
+ buf := make([]byte, 0, 6+len(host))
+
+ var username, password string
+ if u := proxyURL.User; u != nil {
+ username = u.Username()
+ password, _ = u.Password()
+ }
+ buf = append(buf, socks5Version)
+ if proxyURL.User != nil && len(username) < 256 && len(password) < 256 {
+ buf = append(buf, 2 /* num auth methods */, socks5AuthNone, socks5AuthPassword)
+ } else {
+ buf = append(buf, 1 /* num auth methods */, socks5AuthNone)
+ }
+
+ if _, err = conn.Write(buf); err != nil {
+ return errors.New("failed to write socks5 greeting to " + proxyURL.String() + ": " + err.Error())
+ }
+
+ if _, err = io.ReadFull(conn, buf[:2]); err != nil {
+ return errors.New("failed to read socks5 greeting from " + proxyURL.String() + ": " + err.Error())
+ }
+ if buf[0] != 5 {
+ return errors.New("unexpected socks version " + strconv.Itoa(int(buf[0])) + " from " + proxyURL.String())
+ }
+ if buf[1] == 0xff {
+ return errors.New("socks5 proxy at " + proxyURL.String() + " requires authentication")
+ }
+
+ if proxyURL.User != nil && buf[1] == socks5AuthPassword {
+ buf = buf[:0]
+ buf = append(buf, socks5Version)
+ buf = append(buf, uint8(len(username)))
+ buf = append(buf, username...)
+ buf = append(buf, uint8(len(password)))
+ buf = append(buf, password...)
+
+ if _, err = conn.Write(buf); err != nil {
+ return errors.New("failed to write socks5 authentication request to " + proxyURL.String() + ": " + err.Error())
+ }
+
+ if _, err = io.ReadFull(conn, buf[:2]); err != nil {
+ return errors.New("failed to read socks5 authentication reply from " + proxyURL.String() + ": " + err.Error())
+ }
+
+ if buf[1] != 0 {
+ return errors.New("socks5 proxy at " + proxyURL.String() + " rejected username/password")
+ }
+ }
+
+ buf = buf[:0]
+ buf = append(buf, socks5Version, socks5CmdConnect, 0 /* reserved */)
+
+ if ip := net.ParseIP(host); ip != nil {
+ if v4 := ip.To4(); v4 != nil {
+ buf = append(buf, socks5IP4)
+ buf = append(buf, []byte(v4)...)
+ } else {
+ buf = append(buf, socks5IP6)
+ buf = append(buf, []byte(ip)...)
+ }
+ } else {
+ buf = append(buf, socks5Domain)
+ buf = append(buf, byte(len(host)))
+ buf = append(buf, host...)
+ }
+ buf = append(buf, byte(port>>8), byte(port))
+
+ if _, err = conn.Write(buf); err != nil {
+ return errors.New("failed to write socks5 connect request to " + proxyURL.String() + ": " + err.Error())
+ }
+
+ if _, err = io.ReadFull(conn, buf[:4]); err != nil {
+ return errors.New("failed to read socks5 connect reply from " + proxyURL.String() + ": " + err.Error())
+ }
+
+ failure := "unknown error"
+ if int(buf[1]) < len(socks5Errors) {
+ failure = socks5Errors[buf[1]]
+ }
+
+ if len(failure) > 0 {
+ return errors.New("socks5 proxy at " + proxyURL.String() + " failed to connect: " + failure)
+ }
+
+ bytesToDiscard := 0
+ switch buf[3] {
+ case socks5IP4:
+ bytesToDiscard = 4
+ case socks5IP6:
+ bytesToDiscard = 16
+ case socks5Domain:
+ _, err := io.ReadFull(conn, buf[:1])
+ if err != nil {
+ return errors.New("failed to read socks5 domain length from " + proxyURL.String() + ": " + err.Error())
+ }
+ bytesToDiscard = int(buf[0])
+ default:
+ return errors.New("got unknown socks5 address type " + strconv.Itoa(int(buf[3])) + " from " + proxyURL.String())
+ }
+
+ if cap(buf) < bytesToDiscard {
+ buf = make([]byte, bytesToDiscard)
+ } else {
+ buf = buf[:bytesToDiscard]
+ }
+ if _, err = io.ReadFull(conn, buf); err != nil {
+ return errors.New("failed to read socks5 address from " + proxyURL.String() + ": " + err.Error())
+ }
+
+ // Also need to discard the port number
+ if _, err = io.ReadFull(conn, buf[:2]); err != nil {
+ return errors.New("failed to read socks5 port from " + proxyURL.String() + ": " + err.Error())
+ }
+ return nil
+}
+
func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistConn, error) {
pconn := &persistConn{
t: t,
@@ -1020,6 +1188,12 @@
switch {
case cm.proxyURL == nil:
// Do nothing. Not using a proxy.
+ case cm.proxyURL.Scheme == "socks5":
+ conn := pconn.conn
+ if err := socks5Connect(conn, cm.proxyURL, cm.targetAddr); err != nil {
+ conn.Close()
+ return nil, err
+ }
case cm.targetScheme == "http":
pconn.isProxy = true
if pa := cm.proxyAuth(); pa != "" {
@@ -1193,19 +1367,21 @@
//
// A connect method may be of the following types:
//
-// Cache key form Description
-// ----------------- -------------------------
-// |http|foo.com http directly to server, no proxy
-// |https|foo.com https directly to server, no proxy
-// http://proxy.com|https|foo.com http to proxy, then CONNECT to foo.com
-// http://proxy.com|http http to proxy, http to anywhere after that
+// Cache key form Description
+// ----------------- -------------------------
+// |http|foo.com http directly to server, no proxy
+// |https|foo.com https directly to server, no proxy
+// http://proxy.com|https|foo.com http to proxy, then CONNECT to foo.com
+// http://proxy.com|http http to proxy, http to anywhere after that
+// socks5://proxy.com|http|foo.com socks5 to proxy, then http to foo.com
+// socks5://proxy.com|https|foo.com socks5 to proxy, then https to foo.com
//
// Note: no support to https to the proxy yet.
//
type connectMethod struct {
proxyURL *url.URL // nil for no proxy, else full proxy URL
targetScheme string // "http" or "https"
- targetAddr string // Not used if proxy + http targetScheme (4th example in table)
+ targetAddr string // Not used if http proxy + http targetScheme (4th example in table)
}
func (cm *connectMethod) key() connectMethodKey {
@@ -1213,7 +1389,7 @@
targetAddr := cm.targetAddr
if cm.proxyURL != nil {
proxyStr = cm.proxyURL.String()
- if cm.targetScheme == "http" {
+ if strings.HasPrefix(cm.proxyURL.Scheme, "http") && cm.targetScheme == "http" {
targetAddr = ""
}
}
@@ -1982,8 +2158,9 @@
}
var portMap = map[string]string{
- "http": "80",
- "https": "443",
+ "http": "80",
+ "https": "443",
+ "socks5": "1080",
}
// canonicalAddr returns url.Host but always with a ":port" suffix
To view, visit this change. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #2 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/net/http/transport.go 1 file changed, 190 insertions(+), 11 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 2:
Why does this have a new socks5 implementation? Don't we already have one in the x/net repo we can vendor in?
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 2:
I don't know what are normal practices around vendoring. Do I just copy the code into src/vendor/golang_org/x/net/proxy or does this need any additional tracking metadata ?
The other part of it is that I didn't want the entire socks5 Dial function here - in the proxy case we already nave a net.Conn to the proxy server. But yes, socks5Connect was lifted from the rest of the proxy socks5.Dial function, and that's probably not the best way to do this.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 2:
Patch Set 2:
I don't know what are normal practices around vendoring. Do I just copy the code into src/vendor/golang_org/x/net/proxy or does this need any additional tracking metadata ?
Just copy it. The commit message which adds it should have the metadata saying which git version of the x/net source it came from.
The other part of it is that I didn't want the entire socks5 Dial function here - in the proxy case we already nave a net.Conn to the proxy server. But yes, socks5Connect was lifted from the rest of the proxy socks5.Dial function, and that's probably not the best way to do this.
We can add files in the vendor directory to export symbols otherwise inaccessible if needed.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #3 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go A src/vendor/golang_org/x/net/proxy/socks5connect.go 3 files changed, 46 insertions(+), 11 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 4:
I think this is ready to merge, PTAL?
To view, visit change 35488. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 4:Run-TryBot +1
Could you add a test or so?
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 4:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=ebd3329b
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 4:TryBot-Result +1
TryBots are happy.
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #5 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go A src/vendor/golang_org/x/net/proxy/socks5connect.go 4 files changed, 47 insertions(+), 11 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 5:
Added tiny test, only to check ProxyFromEnvironmnent() is happy with the socks5 urls.
A better test would be to actually connect to a socks5 proxy, like TestTransportProxy does for http proxies. However, we don't have socks5 server code in the standard library, so it's not as easy as for http proxies. If we really want this, I can probably write a net.Listener that emulates the socks5 connect message exchanges, but it won't be pretty...
To view, visit change 35488. To unsubscribe, visit settings.
Russ Cox (Gerrit)
Russ Cox posted comments on this change.
Patch set 5:
Thanks very much for fixing this.
(1 comment)
File src/vendor/golang_org/x/net/proxy/socks5connect.go:
Patch Set #5, Line 1:
package proxycopyright notice please
To view, visit change 35488. To unsubscribe, visit settings.
Russ Cox (Gerrit)
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 5:
(2 comments)
File src/net/http/transport_test.go:
Patch Set #5, Line 2163:
{env: "socks5://127.0.0.1", want: "socks5://127.0.0.1"},a little bit more test coverage would be nice.
write a minimal socks5 server and verify it gets connected to at least, if it doesn't speak all of socks5? Or maybe it could, since we already vendor the socks5 package.
File src/vendor/golang_org/x/net/proxy/socks5connect.go:
Patch Set #5, Line 8:
func SOCKS5Connect(conn net.Conn, addr string, auth *Auth, target string) error {I don't think you even need this. The upstream package looks like it lets you do this already:
https://godoc.org/golang.org/x/net/proxy#SOCKS5
Just call SOCKS5 with an implementation of "forward" that returns your net.Conn immediately, and only once, panicking or erroring if called more than once.
To view, visit change 35488. To unsubscribe, visit settings.
Russ Cox (Gerrit)
Russ Cox posted comments on this change.
Patch set 5:
(ignore again sorry)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #6 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go A src/vendor/golang_org/x/net/proxy/socks5connect.go 4 files changed, 116 insertions(+), 11 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 6:Run-TryBot +1
(3 comments)
a little bit more test coverage would be nice.
Added TestSocks5Proxy test. It's kinda ugly but should get the job done.
I don't see a socks5 package I could have used there - we have socks5 support as part of the proxy package but that only covers dialing into an existing proxy, not implementing it ???
File src/vendor/golang_org/x/net/proxy/socks5connect.go:
Patch Set #5, Line 1:
package proxycopyright notice please
Ignoring :)
Patch Set #5, Line 8:
func SOCKS5Connect(conn net.Conn, addr string, auth *Auth, target string) error {I don't think you even need this. The upstream package looks like it lets y
Tried that as https://go-review.googlesource.com/#/c/37507/
I think that makes it harder to read, but I can merge with this change if that's the way you want it.
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 6:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=b7aeb520
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Build is still in progress... This change failed on nacl-amd64p32: See https://storage.googleapis.com/go-build-log/b7aeb520/nacl-amd64p32_d35abfc8.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 6:TryBot-Result -1
2 of 16 TryBots failed: Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/b7aeb520/nacl-amd64p32_d35abfc8.log Failed on nacl-386: https://storage.googleapis.com/go-build-log/b7aeb520/nacl-386_9428a952.log
Consult https://build.golang.org/ to see whether they are new failures.
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 6:
(1 comment)
File src/net/http/transport_test.go:
Patch Set #6, Line 954:
l, err := net.Listen("tcp", "")Looks like you can't quite do that with NaCl ? don't know what to do about it.
To view, visit change 35488. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch Set #6, Line 954:
l, err := net.Listen("tcp", "")Looks like you can't quite do that with NaCl ? don't know what to do about
"tcp", ":0"
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #7 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go A src/vendor/golang_org/x/net/proxy/socks5connect.go 4 files changed, 116 insertions(+), 11 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 7:Run-TryBot +1
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 7:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=406747ee
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Build is still in progress... This change failed on nacl-386: See https://storage.googleapis.com/go-build-log/406747ee/nacl-386_c71295fc.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
To view, visit change 35488. To unsubscribe, visit settings.
Matt Layher (Gerrit)
Matt Layher posted comments on this change.
Patch set 7:Code-Review +1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 7:TryBot-Result -1
2 of 16 TryBots failed: Failed on nacl-386: https://storage.googleapis.com/go-build-log/406747ee/nacl-386_c71295fc.log Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/406747ee/nacl-amd64p32_fa39f903.log
Consult https://build.golang.org/ to see whether they are new failures.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #8 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 124 insertions(+), 11 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #9 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 124 insertions(+), 11 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #10 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 121 insertions(+), 11 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 10:Run-TryBot +1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 10:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=33d97b09
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 10:TryBot-Result +1
TryBots are happy.
To view, visit change 35488. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 10:Code-Review +1
(9 comments)
File src/net/http/transport.go:
no need for the pointer. You could use a value type.
in fact, you can drop the struct:
type oneConnDialer <-chan net.Conn
Patch Set #10, Line 977:
&oneConnDialer{ch: ch}return oneConnDialer(ch)
we don't use _ to mean unused often. I'd still write "network, addr string" here probably. But _ is also fine>
Patch Set #10, Line 985:
panic("oneConnDialer: no connection")or just "return nil, io.EOF"
Patch Set #10, Line 1058:
p.Dial("tcp", cm.targetAddr)I realize that this isn't context aware and can't be canceled if this hangs.
At least file a bug about that to address later so we don't forget.
File src/net/http/transport_test.go:
Patch Set #10, Line 959:
t.FatalfFatalf should only be called from the initial test goroutine.
See https://golang.org/pkg/testing/#T.FailNow
I'd just Errorf + return. But be sure to make sure that this goroutine defers some signal (channel close?) that the main goroutine can notice and fail.
Patch Set #10, Line 1000:
c.Head(ts.URL)check this return value? or say why not?
Patch Set #10, Line 1001:
got := <-chnotably, select here on the accept goroutine to fail, or at least put a few second timeout here so this doesn't hang forever on failure.
Patch Set #10, Line 1008:
want %q, got %qGo error style is got first, then want
"saw proxy connection for %q; want %q"
(or similar)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #11 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 157 insertions(+), 14 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 11:Run-TryBot +1
(8 comments)
no need for the pointer. You could use a value type.
Done
Patch Set #10, Line 977:
oneConnDialer(ch)return oneConnDialer(ch)
Done
we don't use _ to mean unused often. I'd still write "network, addr string"
Done
Patch Set #10, Line 985:
return nil, io.EOF
or just "return nil, io.EOF"
Fatalf should only be called from the initial test goroutine.
Done. closing ch will cause the main goroutine to read an empty string from it, which will make it fail.
Patch Set #10, Line 1000:
copy(buf[:3],
check this return value? or say why not?
Done. Had to implement a bit more of the socks5 fake proxy to make it return a successful connect & http header response.
Patch Set #10, Line 1001:
if _, err
notably, select here on the accept goroutine to fail, or at least put a few
Done
Go error style is got first, then want
Done. Also fixed the code I had cut&pasted from.
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 11:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=b3dc7ea4
Gobot Gobot (Gerrit)
Build is still in progress... This change failed on linux-amd64-race: See https://storage.googleapis.com/go-build-log/b3dc7ea4/linux-amd64-race_361adc4d.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 11:TryBot-Result -1
1 of 17 TryBots failed: Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/b3dc7ea4/linux-amd64-race_361adc4d.log
Consult https://build.golang.org/ to see whether they are new failures.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 11:
The test that failed is TestSocks5Proxy; however I don't understand how TLS got involved with it...
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 12:Run-TryBot +1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 12:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=c49e5042
Gobot Gobot (Gerrit)
Build is still in progress... This change failed on misc-vet-vetall: See https://storage.googleapis.com/go-build-log/c49e5042/misc-vet-vetall_7dcb24c0.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 12:TryBot-Result -1
2 of 17 TryBots failed: Failed on misc-vet-vetall: https://storage.googleapis.com/go-build-log/c49e5042/misc-vet-vetall_7dcb24c0.log Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/c49e5042/linux-amd64-race_4a0e26cd.log
Consult https://build.golang.org/ to see whether they are new failures.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #13 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 158 insertions(+), 14 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 13:Run-TryBot +1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 13:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=dd4c5453
Gobot Gobot (Gerrit)
Build is still in progress... This change failed on misc-vet-vetall: See https://storage.googleapis.com/go-build-log/dd4c5453/misc-vet-vetall_efc1e564.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
To view, visit change 35488. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 13:TryBot-Result -1
2 of 17 TryBots failed: Failed on misc-vet-vetall: https://storage.googleapis.com/go-build-log/dd4c5453/misc-vet-vetall_efc1e564.log Failed on linux-386: https://storage.googleapis.com/go-build-log/dd4c5453/linux-386_4cfc9afe.log
Consult https://build.golang.org/ to see whether they are new failures.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 14:Run-TryBot +1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 14:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=071f423d
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 14:TryBot-Result +1
TryBots are happy.
To view, visit change 35488. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 14:Code-Review +1
(1 comment)
File src/net/http/transport.go:
Patch Set #14, Line 974:
oneConnDialersince this file already imports the proxy package, you can use the return type "proxy.Dialer" here, which seems to convey the purpose better and minimizes the use of the oneConnDialer type.
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse uploaded patch set #15 to this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 158 insertions(+), 14 deletions(-)
To view, visit change 35488. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 15:Run-TryBot +1Code-Review +2
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 15:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=da3ca39e
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 15:
RELNOTE=transport socks5 support
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 15:TryBot-Result +1
TryBots are happy.
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse merged this change.
net/http: add support for socks5 proxy See #18508 This commit adds http Client support for socks5 proxies. Change-Id: Ib015f3819801da13781d5acdd780149ae1f5857b Reviewed-on: https://go-review.googlesource.com/35488 Reviewed-by: Brad Fitzpatrick <brad...@golang.org> Run-TryBot: Brad Fitzpatrick <brad...@golang.org> TryBot-Result: Gobot Gobot <go...@golang.org> --- M src/go/build/deps_test.go M src/net/http/transport.go M src/net/http/transport_test.go 3 files changed, 158 insertions(+), 14 deletions(-)
diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index c26ad06..f8ba532 100644
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -394,6 +394,7 @@
"golang_org/x/net/http2/hpack",
"golang_org/x/net/idna",
"golang_org/x/net/lex/httplex",
+ "golang_org/x/net/proxy",
"golang_org/x/text/unicode/norm",
"golang_org/x/text/width",
"internal/nettrace",
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 571943d..2aa00de 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -29,6 +29,7 @@
"time"
"golang_org/x/net/lex/httplex"
+ "golang_org/x/net/proxy"
)
// DefaultTransport is the default implementation of Transport and is
@@ -275,13 +276,17 @@
return nil, nil
}
proxyURL, err := url.Parse(proxy)
- if err != nil || !strings.HasPrefix(proxyURL.Scheme, "http") {
+ if err != nil ||
+ (proxyURL.Scheme != "http" &&
+ proxyURL.Scheme != "https" &&
+ proxyURL.Scheme != "socks5") {
// proxy was bogus. Try prepending "http://" to it and
// see if that parses correctly. If not, we fall
// through and complain about the original one.
if proxyURL, err := url.Parse("http://" + proxy); err == nil {
return proxyURL, nil
}
+
}
if err != nil {
return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
@@ -964,6 +969,23 @@
}
}
+type oneConnDialer <-chan net.Conn
+
+func newOneConnDialer(c net.Conn) proxy.Dialer {
+ ch := make(chan net.Conn, 1)
+ ch <- c
+ return oneConnDialer(ch)
+}
+
+func (d oneConnDialer) Dial(network, addr string) (net.Conn, error) {
+ select {
+ case c := <-d:
+ return c, nil
+ default:
+ return nil, io.EOF
+ }
+}
+
func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistConn, error) {
pconn := &persistConn{
t: t,
@@ -1020,6 +1042,23 @@
switch {
case cm.proxyURL == nil:
// Do nothing. Not using a proxy.
+ case cm.proxyURL.Scheme == "socks5":
+ conn := pconn.conn
+ var auth *proxy.Auth
+ if u := cm.proxyURL.User; u != nil {
+ auth = &proxy.Auth{}
+ auth.User = u.Username()
+ auth.Password, _ = u.Password()
+ }
+ p, err := proxy.SOCKS5("", cm.addr(), auth, newOneConnDialer(conn))
+ if err != nil {
+ conn.Close()
+ return nil, err
+ }
+ if _, err := p.Dial("tcp", cm.targetAddr); err != nil {
+ conn.Close()
+ return nil, err
+ }
case cm.targetScheme == "http":
pconn.isProxy = true
if pa := cm.proxyAuth(); pa != "" {
@@ -1193,19 +1232,21 @@
//
// A connect method may be of the following types:
//
-// Cache key form Description
-// ----------------- -------------------------
-// |http|foo.com http directly to server, no proxy
-// |https|foo.com https directly to server, no proxy
-// http://proxy.com|https|foo.com http to proxy, then CONNECT to foo.com
-// http://proxy.com|http http to proxy, http to anywhere after that
+// Cache key form Description
+// ----------------- -------------------------
+// |http|foo.com http directly to server, no proxy
+// |https|foo.com https directly to server, no proxy
+// http://proxy.com|https|foo.com http to proxy, then CONNECT to foo.com
+// http://proxy.com|http http to proxy, http to anywhere after that
+// socks5://proxy.com|http|foo.com socks5 to proxy, then http to foo.com
+// socks5://proxy.com|https|foo.com socks5 to proxy, then https to foo.com
//
// Note: no support to https to the proxy yet.
//
type connectMethod struct {
proxyURL *url.URL // nil for no proxy, else full proxy URL
targetScheme string // "http" or "https"
- targetAddr string // Not used if proxy + http targetScheme (4th example in table)
+ targetAddr string // Not used if http proxy + http targetScheme (4th example in table)
}
func (cm *connectMethod) key() connectMethodKey {
@@ -1213,7 +1254,7 @@
targetAddr := cm.targetAddr
if cm.proxyURL != nil {
proxyStr = cm.proxyURL.String()
- if cm.targetScheme == "http" {
+ if strings.HasPrefix(cm.proxyURL.Scheme, "http") && cm.targetScheme == "http" {
targetAddr = ""
}
}
@@ -1982,8 +2023,9 @@
}
var portMap = map[string]string{
- "http": "80",
- "https": "443",
+ "http": "80",
+ "https": "443",
+ "socks5": "1080",
}
// canonicalAddr returns url.Host but always with a ":port" suffix
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 085bb3c..ce98157 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -16,6 +16,7 @@
"context"
"crypto/rand"
"crypto/tls"
+ "encoding/binary"
"errors"
"fmt"
"internal/nettrace"
@@ -943,6 +944,98 @@
}
}
+func TestSocks5Proxy(t *testing.T) {
+ defer afterTest(t)
+ ch := make(chan string, 1)
+ ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+ ch <- "real server"
+ }))
+ defer ts.Close()
+ l := newLocalListener(t)
+ defer l.Close()
+ go func() {
+ defer close(ch)
+ s, err := l.Accept()
+ if err != nil {
+ t.Errorf("socks5 proxy Accept(): %v", err)
+ return
+ }
+ defer s.Close()
+ var buf [22]byte
+ if _, err := io.ReadFull(s, buf[:3]); err != nil {
+ t.Errorf("socks5 proxy initial read: %v", err)
+ return
+ }
+ if want := []byte{5, 1, 0}; !bytes.Equal(buf[:3], want) {
+ t.Errorf("socks5 proxy initial read: got %v, want %v", buf[:3], want)
+ return
+ }
+ if _, err := s.Write([]byte{5, 0}); err != nil {
+ t.Errorf("socks5 proxy initial write: %v", err)
+ return
+ }
+ if _, err := io.ReadFull(s, buf[:4]); err != nil {
+ t.Errorf("socks5 proxy second read: %v", err)
+ return
+ }
+ if want := []byte{5, 1, 0}; !bytes.Equal(buf[:3], want) {
+ t.Errorf("socks5 proxy second read: got %v, want %v", buf[:3], want)
+ return
+ }
+ var ipLen int
+ switch buf[3] {
+ case 1:
+ ipLen = 4
+ case 4:
+ ipLen = 16
+ default:
+ t.Fatalf("socks5 proxy second read: unexpected address type %v", buf[4])
+ }
+ if _, err := io.ReadFull(s, buf[4:ipLen+6]); err != nil {
+ t.Errorf("socks5 proxy address read: %v", err)
+ return
+ }
+ ip := net.IP(buf[4 : ipLen+4])
+ port := binary.BigEndian.Uint16(buf[ipLen+4 : ipLen+6])
+ copy(buf[:3], []byte{5, 0, 0})
+ if _, err := s.Write(buf[:ipLen+6]); err != nil {
+ t.Errorf("socks5 proxy connect write: %v", err)
+ return
+ }
+ done := make(chan struct{})
+ srv := &Server{Handler: HandlerFunc(func(w ResponseWriter, r *Request) {
+ done <- struct{}{}
+ })}
+ srv.Serve(&oneConnListener{conn: s})
+ <-done
+ srv.Shutdown(context.Background())
+ ch <- fmt.Sprintf("proxy for %s:%d", ip, port)
+ }()
+
+ pu, err := url.Parse("socks5://" + l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ c := &Client{Transport: &Transport{Proxy: ProxyURL(pu)}}
+ if _, err := c.Head(ts.URL); err != nil {
+ t.Error(err)
+ }
+ var got string
+ select {
+ case got = <-ch:
+ case <-time.After(5 * time.Second):
+ t.Fatal("timeout connecting to socks5 proxy")
+ }
+ tsu, err := url.Parse(ts.URL)
+ if err != nil {
+ t.Fatal(err)
+ }
+ want := "proxy for " + tsu.Host
+ if got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+}
+
func TestTransportProxy(t *testing.T) {
defer afterTest(t)
ch := make(chan string, 1)
@@ -960,11 +1053,18 @@
t.Fatal(err)
}
c := &Client{Transport: &Transport{Proxy: ProxyURL(pu)}}
- c.Head(ts.URL)
- got := <-ch
+ if _, err := c.Head(ts.URL); err != nil {
+ t.Error(err)
+ }
+ var got string
+ select {
+ case got = <-ch:
+ case <-time.After(5 * time.Second):
+ t.Fatal("timeout connecting to http proxy")
+ }
want := "proxy for " + ts.URL + "/"
if got != want {
- t.Errorf("want %q, got %q", want, got)
+ t.Errorf("got %q, want %q", got, want)
}
}
@@ -2160,6 +2260,7 @@
{env: "https://cache.corp.example.com", want: "https://cache.corp.example.com"},
{env: "http://127.0.0.1:8080", want: "http://127.0.0.1:8080"},
{env: "https://127.0.0.1:8080", want: "https://127.0.0.1:8080"},
+ {env: "socks5://127.0.0.1", want: "socks5://127.0.0.1"},
// Don't use secure for http
{req: "http://insecure.tld/", env: "http.proxy.tld", httpsenv: "secure.proxy.tld", want: "http://http.proxy.tld"},
To view, visit change 35488. To unsubscribe, visit settings.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 16:
(2 comments)
File src/net/http/transport.go:
Patch Set #10, Line 1058:
p.Dial("tcp", cm.targetAddr)I realize that this isn't context aware and can't be canceled if this hangs
File src/net/http/transport.go:
Patch Set #14, Line 974:
proxy.Dialer
since this file already imports the proxy package, you can use the return t
Done
To view, visit change 35488. To unsubscribe, visit settings.
Ian Lance Taylor (Gerrit)
Ian Lance Taylor posted comments on this change.
Patch set 16:
Everything is broken now.
From https://build.golang.org/log/e3427fce73eb3d7d689a920226bc89b6a5afe026:
##### Building packages and commands for linux/amd64. net/http/transport.go:32:2: cannot find package "golang_org/x/net/proxy" in any of: /tmp/workdir/go/src/vendor/golang_org/x/net/proxy (vendor tree) /tmp/workdir/go/src/golang_org/x/net/proxy (from $GOROOT) /root/go/src/golang_org/x/net/proxy (from $GOPATH)
Ian Lance Taylor (Gerrit)
I don't see how this passed the trybots.
Michel Lespinasse (Gerrit)
Michel Lespinasse posted comments on this change.
Patch set 16:
https://go-review.googlesource.com/#/c/35552/ needs to be submitted together with this change. Sorry about that - that was what I intended to do but is evidently not what happened.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 16:
The trybots run this exact commit. This commit had an unsubmitted git parent (its own outstanding Gerrit CL)
I approved the top of the stack. When it was submitted, Gerrit is set to "cherry-pick" for this project, and picked just the top, without the dependent parent.