[net] proxy: add Dial, DialTimeout, and DialContext
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: f4a08796885431483e9ee8ea7a8532b344d1dff3
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
2 files changed, 363 insertions(+), 0 deletions(-)
diff --git a/proxy/dial.go b/proxy/dial.go
new file mode 100644
index 0000000..cf55687
--- /dev/null
+++ b/proxy/dial.go
@@ -0,0 +1,75 @@
+package proxy
+
+import (
+ "context"
+ "net"
+ "time"
+)
+
+// ContextDialer implements DialContext akin to net.Dialer#DialContext.
+type ContextDialer interface {
+ DialContext(ctx context.Context, network, address string) (net.Conn, error)
+}
+
+// TimeoutDialer implements DialTimeout much like net.DialTimeout.
+type TimeoutDialer interface {
+ DialTimeout(network, address string, timeout time.Duration) (net.Conn, error)
+}
+
+// Dial works like net.Dial but using a Dialer derived from the configured proxy environment.
+func Dial(network, address string) (net.Conn, error) {
+ var d = FromEnvironment()
+ return d.Dial(network, address)
+}
+
+// DialTimeout works like net.DialTimeout but using a Dialer derived from the configured proxy environment.
+// Custom dialers (registered via RegisterDialerType) that do not implement TimeoutDialer (or ContextDialer)
+// can leak a goroutine for as long as it takes the underlying Dialer implementation to timeout.
+func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) {
+ var d = FromEnvironment()
+ if td, ok := d.(TimeoutDialer); ok {
+ return td.DialTimeout(network, address, timeout)
+ }
+
+ ctx, cancel := context.WithTimeout(context.Background(), timeout)
+ defer cancel()
+
+ if cd, ok := d.(ContextDialer); ok {
+ return cd.DialContext(ctx, network, address)
+ }
+ return dialContext(ctx, network, address, d)
+}
+
+// DialContext works like DialContext on net.Dialer but using a dialer derived from the configured proxy environment.
+// Custom dialers (registered via RegisterDialerType) that do not implement ContextDialer
+// can leak a goroutine for as long as it takes the underlying Dialer implementation to timeout.
+func DialContext(ctx context.Context, network, address string) (net.Conn, error) {
+ var d = FromEnvironment()
+ if td, ok := d.(ContextDialer); ok {
+ return td.DialContext(ctx, network, address)
+ }
+ return dialContext(ctx, network, address, d)
+}
+
+// WARNING: for custom dialers that only implement proxy.Dialer this will leak a goroutine
+// that will live as long as it takes the underlying Dialer implementation to timeout
+func dialContext(ctx context.Context, network, address string, d Dialer) (net.Conn, error) {
+ var (
+ conn net.Conn
+ done = make(chan struct{}, 1)
+ err error
+ )
+ go func() {
+ conn, err = d.Dial(network, address)
+ close(done)
+ if conn != nil && ctx.Err() != nil {
+ conn.Close()
+ }
+ }()
+ select {
+ case <-ctx.Done():
+ err = ctx.Err()
+ case <-done:
+ }
+ return conn, err
+}
diff --git a/proxy/dial_test.go b/proxy/dial_test.go
new file mode 100644
index 0000000..3ed659a
--- /dev/null
+++ b/proxy/dial_test.go
@@ -0,0 +1,288 @@
+package proxy
+
+import (
+ "context"
+ "fmt"
+ "net"
+ "os"
+ "testing"
+ "time"
+
+ "golang.org/x/net/internal/sockstest"
+)
+
+func TestDial(t *testing.T) {
+ t.Run("Direct", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("ALL_PROXY"); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := Dial(l.Addr().Network(), net.JoinHostPort("", port))
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("SOCKS5", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ p := fmt.Sprintf("socks5://%s", s.Addr().String())
+ if err = os.Setenv("ALL_PROXY", p); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := Dial(s.TargetAddr().Network(), s.TargetAddr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+}
+
+func TestDialContext(t *testing.T) {
+ t.Run("DirectWithCancel", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("ALL_PROXY"); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ c, err := DialContext(ctx, l.Addr().Network(), net.JoinHostPort("", port))
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("DirectWithTimeout", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("ALL_PROXY"); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+ c, err := DialContext(ctx, l.Addr().Network(), net.JoinHostPort("", port))
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("DirectWithTimeoutExceeded", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("ALL_PROXY"); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+ time.Sleep(time.Millisecond)
+ defer cancel()
+ c, err := DialContext(ctx, l.Addr().Network(), net.JoinHostPort("", port))
+ if err == nil {
+ defer c.Close()
+ t.Fatal("failed to timeout")
+ }
+ })
+ t.Run("SOCKS5", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if err = os.Setenv("ALL_PROXY", fmt.Sprintf("socks5://%s", s.Addr().String())); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := DialContext(context.Background(), s.TargetAddr().Network(), s.TargetAddr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("SOCKS5WithTimeout", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if err = os.Setenv("ALL_PROXY", fmt.Sprintf("socks5://%s", s.Addr().String())); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+ c, err := DialContext(ctx, s.TargetAddr().Network(), s.TargetAddr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("SOCKS5WithTimeoutExceeded", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if err = os.Setenv("ALL_PROXY", fmt.Sprintf("socks5://%s", s.Addr().String())); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+ time.Sleep(time.Millisecond)
+ defer cancel()
+ c, err := DialContext(ctx, s.TargetAddr().Network(), s.TargetAddr().String())
+ if err == nil {
+ defer c.Close()
+ t.Fatal("failed to timeout")
+ }
+ })
+}
+
+func TestDialTimeout(t *testing.T) {
+ t.Run("Direct", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("ALL_PROXY"); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := DialTimeout(l.Addr().Network(), net.JoinHostPort("", port), 5*time.Second)
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("DirectTooSlow", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("ALL_PROXY"); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := DialTimeout(l.Addr().Network(), net.JoinHostPort("", port), time.Nanosecond)
+ if err == nil {
+ defer c.Close()
+ t.Fatal("failed to timeout")
+ }
+ })
+ t.Run("SOCKS5", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ p := fmt.Sprintf("socks5://%s", s.Addr().String())
+ if err = os.Setenv("ALL_PROXY", p); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := DialTimeout(s.TargetAddr().Network(), s.TargetAddr().String(), 5*time.Second)
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("SOCKS5TooSlow", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ p := fmt.Sprintf("socks5://%s", s.Addr().String())
+ if err = os.Setenv("ALL_PROXY", p); err != nil {
+ t.Fatal(err)
+ }
+ if err = os.Unsetenv("all_proxy"); err != nil {
+ t.Fatal(err)
+ }
+ c, err := DialTimeout(s.TargetAddr().Network(), s.TargetAddr().String(), time.Nanosecond)
+ if err == nil {
+ defer c.Close()
+ t.Fatal("failed to timeout")
+ }
+ })
+}
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #2 to this change.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: d71cd4fe7a4ee78c4719c857673262b7e7138a22
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 380 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #3 to this change.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: 3ac08a0762edcb1e53ccddc4c3125e5678da39b6
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 374 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
Please note that I was aware of https://go-review.googlesource.com/c/net/+/37641 before submitting this PR/CR. With this in mind I chose to merely add the proxy.Dial, proxy.DialTimeout, and proxy.DialContext functions along with the proxy.ContextDialer single-method interface. The proxy.direct and proxy.PerHost types have been modified to also implement proxy.ContextDialer so as to curry the context as deeply into the stack as possible and therefore minimize goroutine leakage. Legacy implementations of proxy.Dialer, when invoked from DialTimeout or DialContext, will temporarily leak a goroutine for as long as the underlying dialer implementation takes to timeout. Without changing the public API this seemed to me the best that could be hoped for.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #4 to this change.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: ad1e7a3cb68d79190cfa7e61b96e1d39171b18c2
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 374 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
6 comments:
Patch Set #4, Line 1:
package proxyadd the standard copyright header to all files in this change
Patch Set #4, Line 9:
is a means to establish a connection, with context.dials using a context.
Also, mention that ctx is only used for returning the Conn, not the lifetime of the Conn. You can find similar words in the std docs.
Patch Set #4, Line 14:
// Dial works like net.Dial but using a Dialer derived from the configured proxy environment.reference where said environment is defined?
Patch Set #4, Line 15:
func Dial(network, address string) (net.Conn, error) {Maybe we make this take a Context and name it Dial and delete DialTimeout.
No reason to start out with deprecated APIs.
Patch Set #4, Line 24:
func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) {I don't think we need this. Just DialContext.
If anything, Timeout could be a field on a Dialer type later.
Patch Set #4, Line 18:
should consider implementing"should also implement"
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #5 to this change.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: 52034fa7a957be77557c12b3028bd13b60665b06
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #6 to this change.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: cc969317307d1a48526a474a0cd53cd4eaedbd8d
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
Jacob Blain Christen uploaded patch set #7 to the change originally created by Gerrit Bot.
proxy: add Dial (with context)
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialContext receivers. This a familiar
API, see net.Dialer.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: cc969317307d1a48526a474a0cd53cd4eaedbd8d
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #8 to this change.
proxy: add Dial, DialTimeout, and DialContext
The existing API does not allow client code to take advantage of Dialer
implementations that implement DialTimeout and DialContext receivers.
These functions provide a familiar API, see Dial and DialTimeout in the
net package.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: cc969317307d1a48526a474a0cd53cd4eaedbd8d
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
Patch Set 4:
(6 comments)
Brad, thank you for the feedback. I have incorporated your suggestions.
6 comments:
Patch Set #4, Line 1:
package proxyadd the standard copyright header to all files in this change
Done
Patch Set #4, Line 9:
is a means to establish a connection, with context.
dials using a context. […]
Ack
Patch Set #4, Line 14:
// Dial works like net.Dial but using a Dialer derived from the configured proxy environment.reference where said environment is defined?
Ack
Patch Set #4, Line 15:
func Dial(network, address string) (net.Conn, error) {
Maybe we make this take a Context and name it Dial and delete DialTimeout. […]
Okay, this put the onus on deciding what to do with a timeout value of zero back onto client code, where it belongs.
Patch Set #4, Line 24:
func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) {
I don't think we need this. Just DialContext. […]
Done
Patch Set #4, Line 18:
should consider implementing"should also implement"
Ack
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #9 to this change.
proxy: add Dial (with context)
The existing API does not allow client code to take advantage of Dialer implementations that implement DialContext receivers. This a familiar API, see net.Dialer.
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: cc969317307d1a48526a474a0cd53cd4eaedbd8d
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
1 comment:
Patch Set #4, Line 18:
should consider implementing
Ack
Done
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch set 9:Run-TryBot +1
4 comments:
Patch Set #9, Line 11:
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>Remove this line. Go doesn't use this convention. (Gerrit + GitHub PR bots enforce our CLA instead)
Add lines like:
Fixes golang/go#nnn
Or
Updates golang/go#nnn
For all applicable open issues. I haven't read them, but go look at:
https://github.com/golang/go/issues/27874
https://github.com/golang/go/issues/19354
https://github.com/golang/go/issues/17759etc
Patch Set #9, Line 35:
d DialerI'd move "d Dialer" to be the second parameter.
Patch Set #9, Line 26:
forward.Dial(network, address)shouldn't this also use that "func proxyDial"? Perhaps put it in a proxy/internal/proxyinternal package?
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 9:TryBot-Result +1
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #10 to this change.
proxy: add Dial (with context)
The existing API does not allow client code to take advantage of Dialer implementations that implement DialContext receivers. This a familiar API, see net.Dialer.
Fixes golang/go#27874
Fixes golang/go#19354
Fixes golang/go#17759
Fixes golang/go#13455
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: cc969317307d1a48526a474a0cd53cd4eaedbd8d
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
2 comments:
Patch Set #9, Line 11:
Signed-off-by: Jacob Blain Christen <dweo...@gmail.com>
Remove this line. Go doesn't use this convention. […]
Done
Add lines like: […]
Done
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
See my other comments on dial.go and socks5.go?
Jacob Blain Christen (Gerrit)
1 comment:
Patch Set #9, Line 26:
forward.Dial(network, address)
shouldn't this also use that "func proxyDial"? Perhaps put it in a proxy/internal/proxyinternal pack […]
I don't follow. The forward Dialer is an optional implementation to be used instead of net.Dialer.[Dial|DialContext]. See https://github.com/golang/net/blob/master/internal/socks/socks.go#L166 and https://github.com/golang/net/blob/master/internal/socks/socks.go#L220.
Are you saying that we should push down the newly minted ContextDialer interface into an internal package?
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
Patch Set 10:
See my other comments on dial.go and socks5.go?
Yes, I will make the argument ordering change but I have a reply re: your `func ProxyDial` comment
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #11 to this change.
proxy: add Dial (with context)
The existing API does not allow client code to take advantage of Dialer implementations that implement DialContext receivers. This a familiar API, see net.Dialer.
Fixes golang/go#27874
Fixes golang/go#19354
Fixes golang/go#17759
Fixes golang/go#13455
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: 72945cd4cc53f435072da1485eadae8f4f6ba1fb
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
adjusted argument order, as suggested, for dialContext()
1 comment:
Patch Set #9, Line 35:
d DialerI'd move "d Dialer" to be the second parameter.
Done
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
1 comment:
Patch Set #9, Line 26:
forward.Dial(network, address)
I don't follow. The forward Dialer is an optional implementation to be used instead of net.Dialer. […]
I'm saying we're not using the context on line 25 (the one with the underscore currently).
But in the other file in this CL you do the whole trick with the goroutine racing (and the temporary goroutine leak) when the dialer doesn't support ContextDialer, as is the case on this line.
So I think we should do the same thing here, so a the SOCKS dialer's DialContext respects the context, even if the underlying forward Dialer doesn't. (even if it means we leak a goroutine for a bit until the connection finally times out by the operating system or whatever)
But since you have that code already, I'd put it in an internal package where you can use it from here and from the other location.
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Jacob Blain Christen (Gerrit)
1 comment:
Patch Set #9, Line 26:
forward.Dial(network, address)
I'm saying we're not using the context on line 25 (the one with the underscore currently). […]
Ah, I see. I do like that idea but it is complicated by the proxy.Dialer interface type in the dialContext helper func signature. This is a mess that I was trying to avoid because it will cause some duplication honoring the context-less Dial receiver.
Yes, it all comes back to the proxy.Dialer being an interface type. I'll take a stab at implementing your suggestion.
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #12 to this change.
proxy: add Dial (with context)
The existing API does not allow client code to take advantage of Dialer implementations that implement DialContext receivers. This a familiar API, see net.Dialer.
Fixes golang/go#27874
Fixes golang/go#19354
Fixes golang/go#17759
Fixes golang/go#13455
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: b0a372707fc4c45772f19b1b886c8823dd613810
GitHub-Pull-Request: golang/net#38
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch set 12:Run-TryBot +1Code-Review +2
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=9a1750e0
Jacob Blain Christen (Gerrit)
1 comment:
Patch Set #9, Line 26:
forward.Dial(network, address)
Ah, I see. I do like that idea but it is complicated by the proxy. […]
Since dialContext was not exported and all uses of it are at the proxy package level I just decided to call it directly. This is a smaller change to the package-private API than pushing it down into an internal package.
Also, pushing it down into an internal package screamed for a test for the dialContext helper func which I can see I should probably implement now.
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 12:TryBot-Result +1
Brad Fitzpatrick (Gerrit)
1 comment:
Patch Set #9, Line 26:
forward.Dial(network, address)
Since dialContext was not exported and all uses of it are at the proxy package level I just decided […]
My mistake. I didn't notice this was in the same package. I forgot we split it off into net/internal/socks earlier.
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick merged this change.
proxy: add Dial (with context)
The existing API does not allow client code to take advantage of Dialer implementations that implement DialContext receivers. This a familiar API, see net.Dialer.
Fixes golang/go#27874
Fixes golang/go#19354
Fixes golang/go#17759
Fixes golang/go#13455
Change-Id: I0f247783d2037da28c9917db99adda51db1647bd
GitHub-Last-Rev: b0a372707fc4c45772f19b1b886c8823dd613810
GitHub-Pull-Request: golang/net#38
Reviewed-on: https://go-review.googlesource.com/c/net/+/168921
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
Run-TryBot: Brad Fitzpatrick <brad...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
---
A proxy/dial.go
A proxy/dial_test.go
M proxy/direct.go
M proxy/per_host.go
M proxy/per_host_test.go
M proxy/proxy.go
M proxy/socks5.go
7 files changed, 254 insertions(+), 18 deletions(-)
diff --git a/proxy/dial.go b/proxy/dial.go
new file mode 100644
index 0000000..811c2e4
--- /dev/null
+++ b/proxy/dial.go
@@ -0,0 +1,54 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package proxy
+
+import (
+ "context"
+ "net"
+)
+
+// A ContextDialer dials using a context.
+type ContextDialer interface {
+ DialContext(ctx context.Context, network, address string) (net.Conn, error)
+}
+
+// Dial works like DialContext on net.Dialer but using a dialer returned by FromEnvironment.
+//
+// The passed ctx is only used for returning the Conn, not the lifetime of the Conn.
+//
+// Custom dialers (registered via RegisterDialerType) that do not implement ContextDialer
+// can leak a goroutine for as long as it takes the underlying Dialer implementation to timeout.
+//
+// A Conn returned from a successful Dial after the context has been cancelled will be immediately closed.
+func Dial(ctx context.Context, network, address string) (net.Conn, error) {
+ d := FromEnvironment()
+ if xd, ok := d.(ContextDialer); ok {
+ return xd.DialContext(ctx, network, address)
+ }
+ return dialContext(ctx, d, network, address)
+}
+
+// WARNING: this can leak a goroutine for as long as the underlying Dialer implementation takes to timeout
+// A Conn returned from a successful Dial after the context has been cancelled will be immediately closed.
+func dialContext(ctx context.Context, d Dialer, network, address string) (net.Conn, error) {
+ var (
+ conn net.Conn
+ done = make(chan struct{}, 1)
+ err error
+ )
+ go func() {
+ conn, err = d.Dial(network, address)
+ close(done)
+ if conn != nil && ctx.Err() != nil {
+ conn.Close()
+ }
+ }()
+ select {
+ case <-ctx.Done():
+ err = ctx.Err()
+ case <-done:
+ }
+ return conn, err
+}
diff --git a/proxy/dial_test.go b/proxy/dial_test.go
new file mode 100644
index 0000000..3edab49
--- /dev/null
+++ b/proxy/dial_test.go
@@ -0,0 +1,131 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package proxy
+
+import (
+ "context"
+ "fmt"
+ "net"
+ "os"
+ "testing"
+ "time"
+
+ "golang.org/x/net/internal/sockstest"
+)
+
+func TestDial(t *testing.T) {
+ ResetProxyEnv()
+ t.Run("DirectWithCancel", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ c, err := Dial(ctx, l.Addr().Network(), net.JoinHostPort("", port))
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("DirectWithTimeout", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+ c, err := Dial(ctx, l.Addr().Network(), net.JoinHostPort("", port))
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("DirectWithTimeoutExceeded", func(t *testing.T) {
+ defer ResetProxyEnv()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer l.Close()
+ _, port, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+ time.Sleep(time.Millisecond)
+ defer cancel()
+ c, err := Dial(ctx, l.Addr().Network(), net.JoinHostPort("", port))
+ if err == nil {
+ defer c.Close()
+ t.Fatal("failed to timeout")
+ }
+ })
+ t.Run("SOCKS5", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if err = os.Setenv("ALL_PROXY", fmt.Sprintf("socks5://%s", s.Addr().String())); err != nil {
+ t.Fatal(err)
+ }
+ c, err := Dial(context.Background(), s.TargetAddr().Network(), s.TargetAddr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("SOCKS5WithTimeout", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if err = os.Setenv("ALL_PROXY", fmt.Sprintf("socks5://%s", s.Addr().String())); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+ c, err := Dial(ctx, s.TargetAddr().Network(), s.TargetAddr().String())
+ if err != nil {
+ t.Fatal(err)
+ }
+ c.Close()
+ })
+ t.Run("SOCKS5WithTimeoutExceeded", func(t *testing.T) {
+ defer ResetProxyEnv()
+ s, err := sockstest.NewServer(sockstest.NoAuthRequired, sockstest.NoProxyRequired)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if err = os.Setenv("ALL_PROXY", fmt.Sprintf("socks5://%s", s.Addr().String())); err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+ time.Sleep(time.Millisecond)
+ defer cancel()
+ c, err := Dial(ctx, s.TargetAddr().Network(), s.TargetAddr().String())
+ if err == nil {
+ defer c.Close()
+ t.Fatal("failed to timeout")
+ }
+ })
+}
diff --git a/proxy/direct.go b/proxy/direct.go
index 4c5ad88..26b51c3 100644
--- a/proxy/direct.go
+++ b/proxy/direct.go
@@ -5,6 +5,7 @@
package proxy
import (
+ "context"
"net"
)
@@ -13,6 +14,13 @@
// Direct is a direct proxy: one that makes network connections directly.
var Direct = direct{}
+// Dial directly invokes net.Dial with the supplied parameters.
func (direct) Dial(network, addr string) (net.Conn, error) {
return net.Dial(network, addr)
}
+
+// DialContext instantiates a net.Dialer and invokes its DialContext receiver with the supplied parameters.
+func (direct) DialContext(ctx context.Context, network, addr string) (net.Conn, error) {
+ var d net.Dialer
+ return d.DialContext(ctx, network, addr)
+}
diff --git a/proxy/per_host.go b/proxy/per_host.go
index 0689bb6..573fe79 100644
--- a/proxy/per_host.go
+++ b/proxy/per_host.go
@@ -5,6 +5,7 @@
package proxy
import (
+ "context"
"net"
"strings"
)
@@ -41,6 +42,20 @@
return p.dialerForRequest(host).Dial(network, addr)
}
+// DialContext connects to the address addr on the given network through either
+// defaultDialer or bypass.
+func (p *PerHost) DialContext(ctx context.Context, network, addr string) (c net.Conn, err error) {
+ host, _, err := net.SplitHostPort(addr)
+ if err != nil {
+ return nil, err
+ }
+ d := p.dialerForRequest(host)
+ if x, ok := d.(ContextDialer); ok {
+ return x.DialContext(ctx, network, addr)
+ }
+ return dialContext(ctx, d, network, addr)
+}
+
func (p *PerHost) dialerForRequest(host string) Dialer {
if ip := net.ParseIP(host); ip != nil {
for _, net := range p.bypassNetworks {
diff --git a/proxy/per_host_test.go b/proxy/per_host_test.go
index a7d8095..0447eb4 100644
--- a/proxy/per_host_test.go
+++ b/proxy/per_host_test.go
@@ -5,6 +5,7 @@
package proxy
import (
+ "context"
"errors"
"net"
"reflect"
@@ -21,10 +22,6 @@
}
func TestPerHost(t *testing.T) {
- var def, bypass recordingProxy
- perHost := NewPerHost(&def, &bypass)
- perHost.AddFromString("localhost,*.zone,127.0.0.1,10.0.0.1/8,1000::/16")
-
expectedDef := []string{
"example.com:123",
"1.2.3.4:123",
@@ -39,17 +36,41 @@
"[1000::]:123",
}
- for _, addr := range expectedDef {
- perHost.Dial("tcp", addr)
- }
- for _, addr := range expectedBypass {
- perHost.Dial("tcp", addr)
- }
+ t.Run("Dial", func(t *testing.T) {
+ var def, bypass recordingProxy
+ perHost := NewPerHost(&def, &bypass)
+ perHost.AddFromString("localhost,*.zone,127.0.0.1,10.0.0.1/8,1000::/16")
+ for _, addr := range expectedDef {
+ perHost.Dial("tcp", addr)
+ }
+ for _, addr := range expectedBypass {
+ perHost.Dial("tcp", addr)
+ }
- if !reflect.DeepEqual(expectedDef, def.addrs) {
- t.Errorf("Hosts which went to the default proxy didn't match. Got %v, want %v", def.addrs, expectedDef)
- }
- if !reflect.DeepEqual(expectedBypass, bypass.addrs) {
- t.Errorf("Hosts which went to the bypass proxy didn't match. Got %v, want %v", bypass.addrs, expectedBypass)
- }
+ if !reflect.DeepEqual(expectedDef, def.addrs) {
+ t.Errorf("Hosts which went to the default proxy didn't match. Got %v, want %v", def.addrs, expectedDef)
+ }
+ if !reflect.DeepEqual(expectedBypass, bypass.addrs) {
+ t.Errorf("Hosts which went to the bypass proxy didn't match. Got %v, want %v", bypass.addrs, expectedBypass)
+ }
+ })
+
+ t.Run("DialContext", func(t *testing.T) {
+ var def, bypass recordingProxy
+ perHost := NewPerHost(&def, &bypass)
+ perHost.AddFromString("localhost,*.zone,127.0.0.1,10.0.0.1/8,1000::/16")
+ for _, addr := range expectedDef {
+ perHost.DialContext(context.Background(), "tcp", addr)
+ }
+ for _, addr := range expectedBypass {
+ perHost.DialContext(context.Background(), "tcp", addr)
+ }
+
+ if !reflect.DeepEqual(expectedDef, def.addrs) {
+ t.Errorf("Hosts which went to the default proxy didn't match. Got %v, want %v", def.addrs, expectedDef)
+ }
+ if !reflect.DeepEqual(expectedBypass, bypass.addrs) {
+ t.Errorf("Hosts which went to the bypass proxy didn't match. Got %v, want %v", bypass.addrs, expectedBypass)
+ }
+ })
}
diff --git a/proxy/proxy.go b/proxy/proxy.go
index f6026b9..37d3cab 100644
--- a/proxy/proxy.go
+++ b/proxy/proxy.go
@@ -15,6 +15,7 @@
)
// A Dialer is a means to establish a connection.
+// Custom dialers should also implement ContextDialer.
type Dialer interface {
// Dial connects to the given address via the proxy.
Dial(network, addr string) (c net.Conn, err error)
diff --git a/proxy/socks5.go b/proxy/socks5.go
index 56345ec..c91651f 100644
--- a/proxy/socks5.go
+++ b/proxy/socks5.go
@@ -17,8 +17,14 @@
func SOCKS5(network, address string, auth *Auth, forward Dialer) (Dialer, error) {
d := socks.NewDialer(network, address)
if forward != nil {
- d.ProxyDial = func(_ context.Context, network string, address string) (net.Conn, error) {
- return forward.Dial(network, address)
+ if f, ok := forward.(ContextDialer); ok {
+ d.ProxyDial = func(ctx context.Context, network string, address string) (net.Conn, error) {
+ return f.DialContext(ctx, network, address)
+ }
+ } else {
+ d.ProxyDial = func(ctx context.Context, network string, address string) (net.Conn, error) {
+ return dialContext(ctx, forward, network, address)
+ }
}
}
if auth != nil {
To view, visit change 168921. To unsubscribe, or for help writing mail filters, visit settings.