http: add CONNECT method handling for default Host header when using proxy

[Archkon]

Fixes: #63945

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[pimterry]

Hi @Archkon! Thanks for this PR. It looks like there's a failing test (which pins the previous incorrect output) and the commit message is failing the linting. You'll need to fix the test, and update the commit message (git commit -s --amend) to fix the issues, and then force push to update this.

Otherwise the code looks good to me. My main concern is whether this is a breaking change: I agree the current behaviour is wrong, but it I'm not sure if changing it is likely to cause issues elsewhere in code that now expects this wrong Host value (even if the RFCs says they shouldn't) e.g. our own test suite.

What do @nodejs/http think? AFAICT the current behaviour has been in place for more than a decade. Tempted to lean semver-major just in case, even though it's fundamentally really a bug fix.

[Archkon]

Hi @Archkon! Thanks for this PR. It looks like there's a failing test (which pins the previous incorrect output) and the commit message is failing the linting. You'll need to fix the test, and update the commit message (git commit -s --amend) to fix the issues, and then force push to update this.

Otherwise the code looks good to me. My main concern is whether this is a breaking change: I agree the current behaviour is wrong, but it I'm not sure if changing it is likely to cause issues elsewhere in code that now expects this wrong Host value (even if the RFCs says they shouldn't) e.g. our own test suite.

What do @nodejs/http think? AFAICT the current behaviour has been in place for more than a decade. Tempted to lean semver-major just in case, even though it's fundamentally really a bug fix.

I just feel that many framework developers aren’t aware of this underlying bug, because they assume CONNECT method will ensure RFC-compliant handling.

[mcollina]

@pimterry we use the "baking for lts" label and ship, that allows us to asses ecosystem impact.

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74477/

[Archkon]

lgtm

Rebased this PR on the latest nodejs:main and resolved the conflict with #64108
@mcollina Could you please trigger a fresh CI run by applying request-ci? Thanks !

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74517/

[Archkon]

@pimterry I can't access to jenkins ci so I don't know what is wrong with ci issue . And should I resolve the these problems?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74528/

[pimterry]

@Archkon It looks like just flaky tests, nothing to worry about there. I've rerun it.

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/64114
✔  Done loading data for nodejs/node/pull/64114
----------------------------------- PR info ------------------------------------
Title      http: add CONNECT method handling for default Host header when using proxy (#64114)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     Archkon:main -> nodejs:main
Labels     http, baking-for-lts, needs-ci
Commits    2
 - http: add CONNECT method handling for default Host header with proxy
 - test: update CONNECT Host header expectation
Committers 1
 - Archkon <180910180+Archkon@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/64114
Fixes: https://github.com/nodejs/node/issues/63945
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64114
Fixes: https://github.com/nodejs/node/issues/63945
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 24 Jun 2026 14:44:48 GMT
   ✔  Approvals: 2
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/64114#pullrequestreview-4606267464
   ✔  - Tim Perry (@pimterry): https://github.com/nodejs/node/pull/64114#pullrequestreview-4618463647
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-07-03T07:35:39Z: https://ci.nodejs.org/job/node-test-pull-request/74528/
- Querying data for job/node-test-pull-request/74528/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 64114
From https://github.com/nodejs/node
 * branch                  refs/pull/64114/merge -> FETCH_HEAD
✔  Fetched commits as 8a35750a45fd..17099b210432
--------------------------------------------------------------------------------
[main 4e24bdeaac] http: add CONNECT method handling for default Host header with proxy
 Author: Archkon <180910180+Archkon@users.noreply.github.com>
 Date: Wed Jun 24 22:32:15 2026 +0800
 2 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-http-connect-default-host-header.js
[main 302981e1c2] test: update CONNECT Host header expectation
 Author: Archkon <180910180+Archkon@users.noreply.github.com>
 Date: Fri Jun 26 23:32:16 2026 +0800
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
(node:379) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
http: add CONNECT method handling for default Host header with proxy
Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>

PR-URL: #64114

Fixes: #63945

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Reviewed-By: Tim Perry <pimterry@gmail.com>
[detached HEAD 2f0c0977cc] http: add CONNECT method handling for default Host header with proxy

Author: Archkon <180910180+Archkon@users.noreply.github.com>

Date: Wed Jun 24 22:32:15 2026 +0800

2 files changed, 40 insertions(+), 1 deletion(-)

create mode 100644 test/parallel/test-http-connect-default-host-header.js

Rebasing (3/4)

Rebasing (4/4)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: update CONNECT Host header expectation
Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>

PR-URL: #64114

Fixes: #63945

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Reviewed-By: Tim Perry <pimterry@gmail.com>
[detached HEAD e24e5b4cd0] test: update CONNECT Host header expectation

Author: Archkon <180910180+Archkon@users.noreply.github.com>

Date: Fri Jun 26 23:32:16 2026 +0800

1 file changed, 1 insertion(+), 1 deletion(-)

Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/28658695913

[nodejs-github-bot]

Landed in 0e2126d