Skip to content

fix(node-ws): forward response headers during upgrade#1873

Closed
gentamura wants to merge 2 commits intohonojs:mainfrom
gentamura:fix-node-ws-response-headers
Closed

fix(node-ws): forward response headers during upgrade#1873
gentamura wants to merge 2 commits intohonojs:mainfrom
gentamura:fix-node-ws-response-headers

Conversation

@gentamura
Copy link
Copy Markdown

@gentamura gentamura commented Apr 29, 2026

Summary

Forward Hono response headers to the WebSocket upgrade response so headers set by middleware (e.g. Set-Cookie, custom auth headers, WWW-Authenticate on reject) are no longer dropped during the handshake.

Previously, injectWebSocket discarded response.headers after init.app.request(...), so any header attached by Hono middleware never reached the client — both on successful upgrades and on rejected upgrades (4xx/5xx).

Changes

  • On successful upgrade, response headers are appended via wss.on('headers', ...) (the official ws API for injecting handshake headers). The listener is removed in finally; headers is emitted synchronously inside handleUpgrade, so it cannot leak across concurrent upgrades.
  • On rejected upgrade (no waiter / connectionSymbol mismatch), response headers are written into the manual socket.end(...) HTTP response.
  • Skip hop-by-hop headers per RFC 9110 §7.6.1 (connection, keep-alive, proxy-authenticate, proxy-authorization, te, trailer, transfer-encoding, upgrade), plus framing (content-length) and WebSocket handshake headers managed by ws (sec-websocket-accept, sec-websocket-extensions, sec-websocket-protocol) to avoid corrupting the handshake.

Verification

  • yarn workspace @hono/node-ws typecheck
  • yarn 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.md

New tests cover:

  • Custom response header set by middleware reaches the client on a successful upgrade.
  • Custom response header is preserved on a rejected (401) upgrade response.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 29, 2026

🦋 Changeset detected

Latest commit: bd7e67b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hono/node-ws Patch

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

@gentamura gentamura marked this pull request as draft April 29, 2026 08:00
- 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`.
@gentamura
Copy link
Copy Markdown
Author

Hi @yusukebe — first time contributing here, thanks for creating Hono and maintaining this ecosystem!

Quick context check on this PR: I noticed @hono/node-ws is being deprecated in #1862 in favor of @hono/node-server's built-in upgradeWebSocket. The same response-header drop bug exists in honojs/node-server/src/websocket.ts too — rejectUpgradeRequest writes only Connection: close / Content-Length: 0, and the success path doesn't hook wss.on('headers', ...), so headers set by Hono middleware (auth, Set-Cookie, etc.) never reach the client during the handshake.

I've opened a companion PR with the same fix against honojs/node-server: honojs/node-server#346. Happy to land both, or close this one if you'd prefer to only fix forward in node-server. Let me know which direction works best for the deprecation timeline.

@gentamura
Copy link
Copy Markdown
Author

Thanks for the quick turnaround and for merging the node-server fix, @yusukebe! Closing this PR as agreed — the fix lives forward in @hono/node-server (honojs/node-server#346), which is the right home given the deprecation. Looking forward to contributing again.

@gentamura gentamura closed this May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant