-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: x/net/internal/socks, socks_bundle.go in net/http: new package #19688
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
Comments
CL https://golang.org/cl/38278 mentions this issue. |
Do you propose breaking people currently using https://godoc.org/golang.org/x/net/proxy#SOCKS5 ? |
Two issues:
I think we should do (1) but not expose new socks details to users unless and until they need them. |
And (2) can be a different issues. |
I don't understand your question. I didn't say anything about the package x/net/proxy API, also CL 38278 doesn't break the existing applications. What's the purpose of this question?
I'm fine with this. |
Are there other opinions on preferring x/net/internal/socks than x/net/socks pacakge? I'm really fine either way because what I want to see is a) no troublesome for the use of http over socks, b) a concrete socks client implementation package for my use case. |
I'm okay with socks (not internal) if it can start minimal, and if it has package docs saying that it has no compatibility promises. |
Let's just do internal please, until someone actually needs socks exposed. The new package in CL 38278 has an enormous amount of API for a trivial thing. I'd really like to keep the trivial thing trivial. |
Sorry, what you are saying "trivial stuff" is important for my use case; especially accepting bind command and external authentication mechanisms, and notifying a server bound address on a proxy connection because the value of SOCKS to me is that it glues transport protocols together. I'd want to use x/net/internal/socks by vendoring, but if you dislike that approach, I'm happy to drop this proposal and CL 38278. |
So you're saying it can't be minimal. Internal it is. |
(But I will still say no to unnecessary stuff like "bind" for now.) |
It sounds like there are details to work out in the CL but we agree that x/net/socks should exist. |
CL https://golang.org/cl/41031 mentions this issue. |
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
The commit f6698cf introduced the golang.org/x/net/proxy package into the standard library. At the same time, it also carried #19354. As far as I can see, the net/http package only requires a SOCKS client implementation, not a generic forward proxy client implementation. Moreover, the existing/introduced proxy package has several issues that will cause future troubleshooting difficulties like #11682.
I'd like to propose to do the following during Go 1.9 development cycle:
The text was updated successfully, but these errors were encountered: