This is a proposal to add HTTP CONNECT support to x/net/proxy package.
Implement HTTP CONNECT dialer
As what’s done for socks5, we can add an implementation for HTTP CONNECT dialer, it dials to the proxy, and then does a HTTP CONNECT handshake.
Also, we can register this dialer to proxySchemes with key “http” and key “https” as default dialers. This should be done in an init() function, so that users can overwrite those with their custom dialers if they want.
Add function FromEnvironmentForScheme(scheme)
Like FromEnvironment(), this function returns a dialer.
The existing function FromEnvironment() checks the environment variable “all_proxy”, while in the case of HTTP CONNECT, we would want to use “http_proxy” and “https_proxy”.
The new function FromEnvironmentForScheme() checks https_proxy, http_proxy and all_proxy based on the scheme.
If scheme is "https", it checks https_proxy, http_proxy and all_proxy.
If scheme is "http", it checks http_proxy and all_proxy.
In other cases, it checks all_proxy.
Add DialContext() function
As requested in #17759.
Adding DialContext() to Dialer interface will be a breaking change though. What's proposed in #17759 seems like a good solution.
The text was updated successfully, but these errors were encountered:
Note that the standard library's Transport already supports CONNECT. And see #13290 which required us adding ProxyConnectHeader in b06c93e to let people set headers in their CONNECT request. Anything you add in x/net/proxy should support the same interface, letting people set additional headers.
In that case, the function should take a http.Header as additional headers.
And the signature should be func DoHTTPConnectHandshake(ctx context.Context, conn net.Conn, addr string, header http.Header) (net.Conn, error)
If the constructor takes a Dialer, but the Dialer doesn't implement DialContext, we still need to call Dial in another goroutine, and wait on context. And that goroutine may still leak.
If we add another interface ContextDialer, we would need a RegisterContextDialerType() to make sure the registered functions take ContextDialer other than Dialer. If we don't enforce this, and do a runtime type assertion instead, we may still end up with the same situation with leaked goroutines.
Would you consider making an API change for this package, probably in a future release? I would think breaking the compatibility by adding DialContext to the API would not be so unacceptable in this situation.