Skip to content

[v22.x backport] http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback - #60341

Closed
pimterry wants to merge 1 commit into
nodejs:v22.x-stagingfrom
pimterry:backport-59924-to-22
Closed

[v22.x backport] http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback#60341
pimterry wants to merge 1 commit into
nodejs:v22.x-stagingfrom
pimterry:backport-59924-to-22

Conversation

@pimterry

Copy link
Copy Markdown
Member

Backport for #59924.

This also backports the corresponding full code for the websocket test server into the tests. That was introduced in #59404 (which is marked dont-land-on-v22) but it's required for the tests for this change.

This is required to use HTTP/1 websockets on an HTTP/2 server, which is
fairly common as websockets over HTTP/2 is much less widely supported.

This was broken by the recent shouldUpgradeCallback HTTP/1 addition,
which wasn't correctly added to the corresponding allowHttp1 part of
the HTTP/2 implementation.

PR-URL: nodejs#59924
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Oct 21, 2025
@marco-ippolito marco-ippolito added v20.x and removed v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Oct 22, 2025
@marco-ippolito

marco-ippolito commented Oct 22, 2025

Copy link
Copy Markdown
Member

Actually this points to v22, perhaps the title is wrong?

@marco-ippolito marco-ippolito added v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. and removed v20.x labels Oct 22, 2025
@pimterry pimterry changed the title [v20.x backport] http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback [v22.x backport] http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback Oct 22, 2025
@pimterry

Copy link
Copy Markdown
Member Author

@marco-ippolito yes, you're totally right, now fixed - this is indeed for v22.

@pimterry pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 22, 2025
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 22, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

aduh95 pushed a commit that referenced this pull request Oct 22, 2025
This is required to use HTTP/1 websockets on an HTTP/2 server, which is
fairly common as websockets over HTTP/2 is much less widely supported.

This was broken by the recent shouldUpgradeCallback HTTP/1 addition,
which wasn't correctly added to the corresponding allowHttp1 part of
the HTTP/2 implementation.

PR-URL: #59924
Backport-PR-URL: #60341
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@aduh95

aduh95 commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

Landed in 23468fd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants