Skip to content

Conversation

@pimterry
Copy link
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 <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot
Copy link
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 Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. 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
Copy link
Member

marco-ippolito commented Oct 22, 2025

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 Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. 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
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
Collaborator

@nodejs-github-bot
Copy link
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 <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@aduh95
Copy link
Contributor

aduh95 commented Oct 23, 2025

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