Skip to content

x/net/proxy: does not support dialing with a given Context #19354

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.

Sign up for GitHub

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

Jump to bottom
Closed
walken-google opened this issue Mar 2, 2017 · 8 comments
Closed

x/net/proxy: does not support dialing with a given Context #19354

walken-google opened this issue Mar 2, 2017 · 8 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@walken-google
Copy link
Contributor

The x/net/proxy package doesn't support dialing a proxy with a given Context.

This also impacts the net/http package because a Client can dial through a socks5 proxy if specified (usualy through ProxyFromEnvironment). If the socks5 proxy blocks before connecting the socket, the http Client could get stuck without a way to abort the connection by cancelling a Context.
@walken-google
Copy link
Contributor Author

The net/http use of socks5 proxy is new in master - wasn't present in go 1.8 - sorry I forgot to mention that in the initial comment.

@bradfitz bradfitz changed the title x/net/proxy does not support dialing with a given Context x/net/proxy: does not support dialing with a given Context Mar 2, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Mar 2, 2017
@gopherbot
Copy link
Contributor

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

@Fardinak Fardinak mentioned this issue Mar 23, 2017
Closed
@mikioh mikioh mentioned this issue Mar 24, 2017
Closed
@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

@bradfitz bradfitz self-assigned this Jun 14, 2017
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 14, 2017
@mikioh mikioh modified the milestones: Go1.10, Go1.9 Jun 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Dec 13, 2017
gopherbot pushed a commit to golang/net that referenced this issue Apr 6, 2018
@cixtor
internal/{socks,sockstest}: new packages
61147c4 
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>
gopherbot pushed a commit that referenced this issue Apr 6, 2018
@cixtor @bradfitz
net/http: replace SOCKS client implementation
4acb305 
This change replaces the vendored socks client implementation with the
bundle of golang.org/x/net/internal/socks package which contains fixes
for 19354 and 11682.

golang.org/x/net/internal/socks becomes socks_bundle.go.

At git rev 61147c4. (golang.org/cl/38278)

Updates #11682.
Updates #18508.
Updates #19354.
Fixes #19688.
Updates #21333.

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

mikioh commented Apr 6, 2018

From Go 1.11, the net/http package supports SOCKS dialing with contexts. This issue is just for the x/net/proxy package.

@mikioh mikioh modified the milestones: Go1.11, Unreleased Apr 6, 2018
@mikioh mikioh removed the NeedsFix The path to resolution is known, but the work has not been done. label Apr 6, 2018
@mjgarton
Copy link
Contributor

I updated the CL at https://go-review.googlesource.com/c/net/+/37641 to address this.

The required change is now much simpler, thanks to the new internal socks package that already supports Contexts, but the issue of full backwards compatibility still needs a definitive decision it seems.

@GongT
Copy link

GongT commented Jul 10, 2018

What about net.Dialer ?

var d net.Dialer
d.DialContext(ctx, network, address)

@dweomer dweomer mentioned this issue May 1, 2019
Closed
@gopherbot
Copy link
Contributor

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
@dweomer
proxy: add Dial (with context)

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
72945cd 
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
@dweomer
proxy: add Dial (with context)

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
b0a3727 
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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

proxy: add Dial (with context) dweomer/golang-net
6 participants
@bradfitz @mikioh @GongT @gopherbot @mjgarton @walken-google