fix(node-ws): forward response headers during upgrade#1873
fix(node-ws): forward response headers during upgrade#1873gentamura wants to merge 2 commits intohonojs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: bd7e67b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
- Add remaining RFC 9110 §7.6.1 hop-by-hop headers (transfer-encoding, te, trailer, keep-alive, proxy-authenticate, proxy-authorization) to the skip-list to prevent handshake corruption. - Reference RFC 9110 in inline comments and the changeset. - Note that the `headers` event is emitted synchronously inside `handleUpgrade`, so the shared listener cannot leak across concurrent upgrades. - Rename shadowed `headers` local in the reject path to `responseLines`.
|
Hi @yusukebe — first time contributing here, thanks for creating Hono and maintaining this ecosystem! Quick context check on this PR: I noticed I've opened a companion PR with the same fix against |
|
Thanks for the quick turnaround and for merging the node-server fix, @yusukebe! Closing this PR as agreed — the fix lives forward in |
Summary
Forward Hono response headers to the WebSocket upgrade response so headers set by middleware (e.g.
Set-Cookie, custom auth headers,WWW-Authenticateon reject) are no longer dropped during the handshake.Previously,
injectWebSocketdiscardedresponse.headersafterinit.app.request(...), so any header attached by Hono middleware never reached the client — both on successful upgrades and on rejected upgrades (4xx/5xx).Changes
wss.on('headers', ...)(the officialwsAPI for injecting handshake headers). The listener is removed infinally;headersis emitted synchronously insidehandleUpgrade, so it cannot leak across concurrent upgrades.socket.end(...)HTTP response.connection,keep-alive,proxy-authenticate,proxy-authorization,te,trailer,transfer-encoding,upgrade), plus framing (content-length) and WebSocket handshake headers managed byws(sec-websocket-accept,sec-websocket-extensions,sec-websocket-protocol) to avoid corrupting the handshake.Verification
yarn workspace @hono/node-ws typecheckyarn workspace @hono/node-ws test run(15/15 passing, including 2 new tests)yarn prettier --check packages/node-ws/src/index.ts packages/node-ws/src/index.test.ts .changeset/polite-cooks-report.mdNew tests cover: