feat(ProxyAgent): match Curl behavior in HTTP->HTTP Proxy connections

[caitp]

By default, Curl does not send a CONNECT request to a non-secure Proxy with a non-secure endpoint. This can be overridden using the --proxytunnel or -p 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 behavior 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

This relates to...

#4083

Rationale

Closer alignment with widely used industry standard libraries (specifically Curl)

Changes

• A new tunnelProxy option is added to the ProxyAgent dispatcher which requests whether or not CONNECT requests be sent to an HTTP->HTTP Proxy
• A hackish rawSocket option is added to Request. This is awkward, advice on a cleaner/better way to do this would be appreciated. This conveys to a dispatcher several layers down that it should avoid proceeding with a CONNECT request and instead fulfill the connect() call with just the socket.
• A new test case showcasing the altered behavior when opts.tunnelProxy===false

Features

The new tunnelProxy option, which permits matching Curl (however, currently this sort of works in the opposite way to Curl, as tunneling occurs by default and requires opt-out rather than opt-in)

Bug Fixes

#4083

Breaking Changes and Deprecations

It would technically be a breaking change to fully match Curl by opting out of tunneling by default. That is (in the first iteration of this patch) not done.

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[mcollina]

cc @joyeecheung

[metcoder95]

lgtm, just documentation left

[caitp]

I may have accidentaly messed up the merge, so replacing that with a force push

[metcoder95]

tests does not seems happy

[caitp]

tests does not seems happy

seems to be from new stuff imported by dcf82a7, the same test fails with this whole PR reverted

[mcollina]

lgtm

[joyeecheung]

Was trying to update the test expectations in Node.js now that this gets fixed in nodejs/node#58859, but I see that it still gets a CONNECT for the HTTP test. I think the problem is that tunnelProxy: false is an agent-wide setting, but what we want is for it to be automatic - false for HTTP traffic, true for HTTPS, I think that's the default behavior of curl as well.

[metcoder95]

cc: @caitp

[caitp]

I think this is right there in the PR description :P "Disabling tunneling for HTTP->HTTP by default would be a breaking change, so currently, the tunneling behavior requires an opt-out. This may change depending on feedback during code review."

There is already logic to tunnel if an endpoint or the proxy itself uses a secure protocol -- it should be the same logic as in Curl --- the only thing needed to change this to be the default is just flipping the default flag value to false, and then tunneling for unsecure protocols would become opt-in.

[metcoder95]

If that's the case, we can keep this until further major where the flag is turned true by default

[caitp]

I should add, there do seem to be some issues with it, some of which have been raised by Joyee in the past, so I am working on an update to fix it up a bit -- maybe flipping the flag by default makes sense after that

[github-actions]

The backport to v6.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-v6.x v6.x
# Navigate to the new working tree
cd .worktrees/backport-v6.x
# Create a new branch
git switch --create backport-4180-to-v6.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 95fd9d3feebcdede11a4e6b66f57aac34abf482f
# Push it to GitHub
git push --set-upstream origin backport-4180-to-v6.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-v6.x

Then, create a pull request where the base branch is v6.x and the compare/head branch is backport-4180-to-v6.x.