feat(ProxyAgent): match Curl behavior in HTTP->HTTP Proxy connections (#4180) #4433
+141
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…#4180) * feat(ProxyAgent): match Curl behaviour for http-http Proxy connections Curl does not send a CONNECT request for to a Proxy server, by default, for cleartext communications to an endpoint, via a cleartext connection to a Proxy. It permits forcing a CONNECT request to be sent via the --tunnelproxy parameter. This change modifies ProxyAgent's constructor to accept a `tunnelProxy` option, sends a CONNECT if either `tunnelProxy` is true, or either the Proxy or endpoint use a non-http: protocol. Disabling tunneling for HTTP->HTTP by default would be a breaking change, so currently, the tunneling behaviour requires an opt-out. This may change depending on feedback during code review. This adds a new test case which explicitly disables tunneling for an HTTP->HTTP connection, and asserts that no CONNECT message is sent to the server or proxy, and that the expected HTTP request is sent to the proxy. Closes #4083 * Part 2 This version tries to expose less sketchiness -- it's not particularly well organized yet, and I'm sure it could be cleaned up a lot. Instead of adding the "rawSocket" stuff to RequestOptions, there's a new wrapper ProxyClient added, which intercepts the CONNECT message and prevents it from being dispatched. Unfortunately the wrapper client isn't quite written in a way to manage all of the client-ness, so ProxyAgent is still responsible for updating the PATH of HTTP->HTTP Proxy requests to include the endpoint domain. It is messy though, admittedly. * remove rawSocket from Dispatcher type definition * Add some docs * rename to proxyTunnel to match CURL * Rename to in the docs, too * Try to clarify the docs a bit initially just wanted to fix a typo, but thought maybe the original explanation wasn't great. (cherry picked from commit 95fd9d3)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backports #4180 - Relates to #4430