Skip to content

Commit 7e0555d

Browse files
ATOM00blueyusukebe
andauthored
fix(deno): echo negotiated WebSocket subprotocol in upgrade response (#4955)
* fix(deno): echo negotiated WebSocket subprotocol in upgrade response The Deno adapter did not set the `Sec-WebSocket-Protocol` header on the upgrade response, so browsers such as Chrome rejected connections that requested a subprotocol. Read the first value from the request's `Sec-WebSocket-Protocol` header and pass it as the `protocol` option to `Deno.upgradeWebSocket`, while still honoring an explicitly provided `protocol` option. Closes #4439 * add a test --------- Co-authored-by: ATOM00blue <219721791+ATOM00blue@users.noreply.github.com> Co-authored-by: Yusuke Wada <yusuke@kamawada.com>
1 parent f0ed246 commit 7e0555d

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

‎src/adapter/deno/websocket.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,75 @@ describe('WebSockets', () => {
7676
)
7777
expect(await messagePromise).toBe(data)
7878
})
79+
80+
it('Should pass the first Sec-WebSocket-Protocol value as the protocol option', async () => {
81+
app.get(
82+
'/ws',
83+
upgradeWebSocket(() => ({}))
84+
)
85+
const socket = new EventTarget() as WebSocket
86+
let passedOptions: Deno.UpgradeWebSocketOptions | undefined
87+
Deno.upgradeWebSocket = (_req, options) => {
88+
passedOptions = options
89+
return {
90+
response: new Response(),
91+
socket,
92+
}
93+
}
94+
await app.request('/ws', {
95+
headers: {
96+
upgrade: 'websocket',
97+
'sec-websocket-protocol': 'rivet, rivet_target.actor, rivet_actor.19c4f9038947cdcb',
98+
},
99+
})
100+
expect(passedOptions?.protocol).toBe('rivet')
101+
})
102+
103+
it('Should not set the protocol option when Sec-WebSocket-Protocol is absent', async () => {
104+
app.get(
105+
'/ws',
106+
upgradeWebSocket(() => ({}))
107+
)
108+
const socket = new EventTarget() as WebSocket
109+
let passedOptions: Deno.UpgradeWebSocketOptions | undefined
110+
Deno.upgradeWebSocket = (_req, options) => {
111+
passedOptions = options
112+
return {
113+
response: new Response(),
114+
socket,
115+
}
116+
}
117+
await app.request('/ws', {
118+
headers: {
119+
upgrade: 'websocket',
120+
},
121+
})
122+
expect(passedOptions?.protocol).toBeUndefined()
123+
})
124+
125+
it('Should let an explicit protocol option take precedence over the request header', async () => {
126+
app.get(
127+
'/ws',
128+
upgradeWebSocket(() => ({}), { protocol: 'user-chosen' })
129+
)
130+
const socket = new EventTarget() as WebSocket
131+
let passedOptions: Deno.UpgradeWebSocketOptions | undefined
132+
Deno.upgradeWebSocket = (_req, options) => {
133+
passedOptions = options
134+
return {
135+
response: new Response(),
136+
socket,
137+
}
138+
}
139+
await app.request('/ws', {
140+
headers: {
141+
upgrade: 'websocket',
142+
'sec-websocket-protocol': 'header-value',
143+
},
144+
})
145+
expect(passedOptions?.protocol).toBe('user-chosen')
146+
})
147+
79148
it('Should call next() when header does not have upgrade', async () => {
80149
const next = vi.fn()
81150
await upgradeWebSocket(() => ({}))(

‎src/adapter/deno/websocket.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,15 @@ export const upgradeWebSocket: UpgradeWebSocket<WebSocket, Deno.UpgradeWebSocket
77
return
88
}
99

10-
const { response, socket } = Deno.upgradeWebSocket(c.req.raw, options ?? {})
10+
// Echo the negotiated subprotocol back to the client. Browsers (e.g. Chrome)
11+
// reject the connection when a subprotocol was requested but the response is
12+
// missing the `Sec-WebSocket-Protocol` header.
13+
const subprotocol = c.req.header('sec-websocket-protocol')?.split(',')[0]?.trim()
14+
15+
const { response, socket } = Deno.upgradeWebSocket(c.req.raw, {
16+
...(subprotocol ? { protocol: subprotocol } : {}),
17+
...options,
18+
})
1119

1220
const wsContext: WSContext<WebSocket> = new WSContext({
1321
close: (code, reason) => socket.close(code, reason),

0 commit comments

Comments
 (0)