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

x/net/proxy: provide DialContext #17759

Closed
chowey opened this issue Nov 3, 2016 · 6 comments
Closed

x/net/proxy: provide DialContext #17759

chowey opened this issue Nov 3, 2016 · 6 comments

Comments

@chowey
Copy link

chowey commented Nov 3, 2016

With go1.7, the net package now provides a DialContext function. This is the recommended way to use http.Transport.

x/net/proxy should do the same so we can use it when creating our http.Transports for use with SOCKS proxies.

@josharian josharian changed the title x/net/proxy should provide DialContext x/net/proxy: provide DialContext Nov 3, 2016
@dsnet dsnet added this to the Unreleased milestone Nov 3, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Nov 8, 2016

I don't remember that code, but sounds reasonable.

@artyom
Copy link
Member

artyom commented Dec 22, 2016

I need this as well so I can try to implement context support for cancelation in this package.

I'd like to discuss API changes before doing any implementation. My conservative proposal (hopefully completely backwards-compatible) is to leave Dialer interface unchanged and introduce new interface:

type ContextDialer interface {
	Dialer
	DialContext(ctx context.Context, network, address string) (net.Conn, error)
}

Then all functions of this package that take Dialer argument and return Dialer may be changed to return ContextDialer type instead and do type assertion on passed Dialer argument value to check whether it implements ContextDialer.

As example, FromURL signature would change from:

func FromURL(u *url.URL, forward proxy.Dialer) (proxy.Dialer, error)

to

func FromURL(u *url.URL, forward proxy.Dialer) (proxy.ContextDialer, error)

@Fardinak
Copy link

Possible duplicate of #19354.
This seems to be covered by the issue above, which is in the Go1.9 milestone and also in review at https://golang.org/cl/37641.

/cc @bradfitz

@gopherbot
Copy link

CL https://golang.org/cl/38278 mentions this issue.

gopherbot pushed a commit to golang/net that referenced this issue Apr 6, 2018
This change factors out the code related to SOCKS protocol version 5
from the golang/x/net/proxy package and provides new SOCKS-specific
API to fix the following:
- inflexbility of forward proxy connection setup; e.g., no support for
  context-based deadline or canceling, no support for dial deadline,
  no support for working with external authentication mechanisms,
- useless error values for troubleshooting.

The new package socks is supposed to be used by the net/http package
of standard library and proxy package of golang.org/x/net repository.

Fixes golang/go#11682.
Updates golang/go#17759.
Updates golang/go#19354.
Updates golang/go#19688.
Fixes golang/go#21333.

Change-Id: I24098ac8522dcbdceb03d534147c5101ec9e7350
Reviewed-on: https://go-review.googlesource.com/38278
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@mjgarton
Copy link
Contributor

@gopherbot
Copy link

Change https://golang.org/cl/168921 mentions this issue: proxy: add Dial (with context)

dweomer added a commit to dweomer/golang-net that referenced this issue May 1, 2019
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
dweomer added a commit to dweomer/golang-net that referenced this issue May 2, 2019
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
@golang golang locked and limited conversation to collaborators May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants