Skip to content

Commit b7f252e

Browse files
mcollinaKhafraDevlpincaUlisesGascon
authored
Backport WebSocket maxPayloadSize fixes to v7.x (#5423) (#5428)
* feat: add configurable maxPayloadSize for WebSocket (#4955) (cherry picked from commit bd91f86) * test: fix flaky permessage-deflate limit timeout (#5229) (cherry picked from commit 9d82667) * fix(websocket): enforce max payload size across fragments Account for previously received fragment bytes when checking WebSocket payload size limits, so fragmented messages cannot exceed maxPayloadSize by splitting the payload across frames. Add coverage for cumulative fragmented payload size enforcement. (cherry picked from commit b4c287b) * websocket: handle empty fragments and stream limits Treat zero-byte frames as real fragments so fragmented messages can start with an empty frame and empty continuations still count toward maxFragments. Pass dispatcher WebSocket limits through to WebSocketStream's parser, add regression coverage for WebSocket and WebSocketStream fragment limits, make the fragment close tests wait for both endpoints, and fix the Client docs typo for maxFragments. (cherry picked from commit c5ed787) --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matthew Aitken <maitken033380023@gmail.com> Co-authored-by: Luigi Pinca <luigipinca@gmail.com> Co-authored-by: Ulises Gascon <ulisesgascongonzalez@gmail.com>
1 parent 25efa44 commit b7f252e

7 files changed

Lines changed: 269 additions & 10 deletions

File tree

docs/docs/api/Client.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ Returns: `Client`
2727
* **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB.
2828
* **maxResponseSize** `number | null` (optional) - Default: `-1` - The maximum length of response body in bytes. Set to `-1` to disable.
2929
* **webSocket** `WebSocketOptions` (optional) - WebSocket-specific configuration options.
30+
* **maxFragments** `number` (optional) - Default: `131072` - Maximum number of fragments in a message. Set to 0 to disable the limit.
3031
* **maxPayloadSize** `number` (optional) - Default: `134217728` (128 MB) - Maximum allowed payload size in bytes for WebSocket messages. Applied to uncompressed messages, compressed frame payloads, and decompressed (permessage-deflate) messages. Set to 0 to disable the limit.
3132
* **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections.
3233
* **connect** `ConnectOptions | Function | null` (optional) - Default: `null`.

lib/dispatcher/dispatcher-base.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class DispatcherBase extends Dispatcher {
2626

2727
get webSocketOptions () {
2828
return {
29+
maxFragments: this[kWebSocketOptions].maxFragments ?? 131072,
2930
maxPayloadSize: this[kWebSocketOptions].maxPayloadSize ?? 128 * 1024 * 1024
3031
}
3132
}

lib/web/websocket/receiver.js

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ const { closeWebSocketConnection } = require('./connection')
2020
const { PerMessageDeflate } = require('./permessage-deflate')
2121
const { MessageSizeExceededError } = require('../../core/errors')
2222

23+
function failWebsocketConnectionWithCode (ws, code, reason) {
24+
closeWebSocketConnection(ws, code, reason, Buffer.byteLength(reason))
25+
failWebsocketConnection(ws, reason)
26+
}
27+
2328
// This code was influenced by ws released under the MIT license.
2429
// Copyright (c) 2011 Einar Otto Stangvik <einaros@gmail.com>
2530
// Copyright (c) 2013 Arnout Kazemier and contributors
@@ -39,19 +44,23 @@ class ByteParser extends Writable {
3944
/** @type {Map<string, PerMessageDeflate>} */
4045
#extensions
4146

47+
/** @type {number} */
48+
#maxFragments
49+
4250
/** @type {number} */
4351
#maxPayloadSize
4452

4553
/**
4654
* @param {import('./websocket').WebSocket} ws
4755
* @param {Map<string, string>|null} extensions
48-
* @param {{ maxPayloadSize?: number }} [options]
56+
* @param {{ maxFragments?: number, maxPayloadSize?: number }} [options]
4957
*/
5058
constructor (ws, extensions, options = {}) {
5159
super()
5260

5361
this.ws = ws
5462
this.#extensions = extensions == null ? new Map() : extensions
63+
this.#maxFragments = options.maxFragments ?? 0
5564
this.#maxPayloadSize = options.maxPayloadSize ?? 0
5665

5766
if (this.#extensions.has('permessage-deflate')) {
@@ -75,9 +84,9 @@ class ByteParser extends Writable {
7584
if (
7685
this.#maxPayloadSize > 0 &&
7786
!isControlFrame(this.#info.opcode) &&
78-
this.#info.payloadLength > this.#maxPayloadSize
87+
this.#info.payloadLength + this.#fragmentsBytes > this.#maxPayloadSize
7988
) {
80-
failWebsocketConnection(this.ws, 'Payload size exceeds maximum allowed size')
89+
failWebsocketConnectionWithCode(this.ws, 1009, 'Payload size exceeds maximum allowed size')
8190
return false
8291
}
8392

@@ -242,10 +251,12 @@ class ByteParser extends Writable {
242251
this.#state = parserStates.INFO
243252
} else {
244253
if (!this.#info.compressed) {
245-
this.writeFragments(body)
254+
if (!this.writeFragments(body)) {
255+
return
256+
}
246257

247258
if (this.#maxPayloadSize > 0 && this.#fragmentsBytes > this.#maxPayloadSize) {
248-
failWebsocketConnection(this.ws, new MessageSizeExceededError().message)
259+
failWebsocketConnectionWithCode(this.ws, 1009, new MessageSizeExceededError().message)
249260
return
250261
}
251262

@@ -264,14 +275,17 @@ class ByteParser extends Writable {
264275
this.#info.fin,
265276
(error, data) => {
266277
if (error) {
267-
failWebsocketConnection(this.ws, error.message)
278+
const code = error instanceof MessageSizeExceededError ? 1009 : 1007
279+
failWebsocketConnectionWithCode(this.ws, code, error.message)
268280
return
269281
}
270282

271-
this.writeFragments(data)
283+
if (!this.writeFragments(data)) {
284+
return
285+
}
272286

273287
if (this.#maxPayloadSize > 0 && this.#fragmentsBytes > this.#maxPayloadSize) {
274-
failWebsocketConnection(this.ws, new MessageSizeExceededError().message)
288+
failWebsocketConnectionWithCode(this.ws, 1009, new MessageSizeExceededError().message)
275289
return
276290
}
277291

@@ -341,8 +355,17 @@ class ByteParser extends Writable {
341355
}
342356

343357
writeFragments (fragment) {
358+
if (
359+
this.#maxFragments > 0 &&
360+
this.#fragments.length === this.#maxFragments
361+
) {
362+
failWebsocketConnectionWithCode(this.ws, 1008, 'Too many message fragments')
363+
return false
364+
}
365+
344366
this.#fragmentsBytes += fragment.length
345367
this.#fragments.push(fragment)
368+
return true
346369
}
347370

348371
consumeFragments () {

lib/web/websocket/websocket.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,12 @@ class WebSocket extends EventTarget {
435435
// once this happens, the connection is open
436436
this[kResponse] = response
437437

438-
const maxPayloadSize = this[kController]?.dispatcher?.webSocketOptions?.maxPayloadSize
438+
const webSocketOptions = this[kController]?.dispatcher?.webSocketOptions
439+
const maxFragments = webSocketOptions?.maxFragments
440+
const maxPayloadSize = webSocketOptions?.maxPayloadSize
439441

440442
const parser = new ByteParser(this, parsedExtensions, {
443+
maxFragments,
441444
maxPayloadSize
442445
})
443446
parser.on('drain', onParserDrain)

test/websocket/fragments.js

Lines changed: 182 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
const assert = require('node:assert')
44
const { test, after } = require('node:test')
55
const { WebSocketServer } = require('ws')
6-
const { WebSocket } = require('../..')
6+
const { WebSocket, Agent } = require('../..')
77
const diagnosticsChannel = require('node:diagnostics_channel')
88

99
test('Fragmented frame with a ping frame in the middle of it', () => {
@@ -40,3 +40,184 @@ test('Fragmented frame with a ping frame in the middle of it', () => {
4040
})
4141
})
4242
})
43+
44+
test('Too many fragments (uncompressed)', (t, done) => {
45+
function maybeDone () {
46+
if (++maybeDone.callCount === 2) {
47+
agent.close()
48+
server.close(done)
49+
}
50+
}
51+
52+
maybeDone.callCount = 0
53+
54+
const agent = new Agent({
55+
webSocket: {
56+
maxFragments: 3
57+
}
58+
})
59+
60+
const server = new WebSocketServer({ port: 0 }, () => {
61+
const { port } = server.address()
62+
const client = new WebSocket(`ws://127.0.0.1:${port}`, {
63+
dispatcher: agent
64+
})
65+
66+
client.addEventListener('error', () => {
67+
assert.ok(true)
68+
})
69+
70+
client.addEventListener('close', (event) => {
71+
assert.strictEqual(event.code, 1006)
72+
maybeDone()
73+
})
74+
})
75+
76+
server.on('connection', (ws) => {
77+
ws.on('close', (code, reason) => {
78+
assert.strictEqual(code, 1008)
79+
assert.strictEqual(reason.toString(), 'Too many message fragments')
80+
maybeDone()
81+
})
82+
83+
const fragment = Buffer.from('a')
84+
const options = { fin: false }
85+
86+
ws.send(fragment, options)
87+
ws.send(fragment, options)
88+
ws.send(fragment, options)
89+
ws.send(fragment, options)
90+
})
91+
})
92+
93+
test('Too many fragments (compressed)', (t, done) => {
94+
function maybeDone () {
95+
if (++maybeDone.callCount === 2) {
96+
agent.close()
97+
server.close(done)
98+
}
99+
}
100+
101+
maybeDone.callCount = 0
102+
103+
const agent = new Agent({
104+
webSocket: {
105+
maxFragments: 3
106+
}
107+
})
108+
109+
const server = new WebSocketServer({
110+
perMessageDeflate: { threshold: 0 },
111+
port: 0
112+
}, () => {
113+
const { port } = server.address()
114+
const client = new WebSocket(`ws://127.0.0.1:${port}`, {
115+
dispatcher: agent
116+
})
117+
118+
client.addEventListener('error', () => {
119+
assert.ok(true)
120+
})
121+
122+
client.addEventListener('close', (event) => {
123+
assert.strictEqual(event.code, 1006)
124+
maybeDone()
125+
})
126+
})
127+
128+
server.on('connection', (ws) => {
129+
ws.on('close', (code, reason) => {
130+
assert.strictEqual(code, 1008)
131+
assert.strictEqual(reason.toString(), 'Too many message fragments')
132+
maybeDone()
133+
})
134+
135+
const fragment = Buffer.from('a')
136+
const options = { fin: false }
137+
138+
ws.send(fragment, options)
139+
ws.send(fragment, options)
140+
ws.send(fragment, options)
141+
ws.send(fragment, options)
142+
})
143+
})
144+
145+
test('Empty first fragment followed by non-empty continuation delivers the message', () => {
146+
// RFC 6455 §5.4 allows zero-byte fragments. A conforming server that opens
147+
// a fragmented message with an empty frame must be honored: the parser must
148+
// recognize the in-progress fragmented message when the continuation arrives.
149+
const server = new WebSocketServer({ port: 0 })
150+
151+
server.on('connection', (ws) => {
152+
ws.send('', { fin: false })
153+
ws.send('hello', { fin: true })
154+
})
155+
156+
after(() => {
157+
for (const client of server.clients) {
158+
client.close()
159+
}
160+
161+
server.close()
162+
})
163+
164+
const ws = new WebSocket(`ws://localhost:${server.address().port}`)
165+
166+
return new Promise((resolve) => {
167+
ws.addEventListener('message', ({ data }) => {
168+
assert.strictEqual(data, 'hello')
169+
170+
ws.close()
171+
resolve()
172+
})
173+
})
174+
})
175+
176+
test('Too many empty fragments triggers close 1008', (t, done) => {
177+
function maybeDone () {
178+
if (++maybeDone.callCount === 2) {
179+
agent.close()
180+
server.close(done)
181+
}
182+
}
183+
184+
maybeDone.callCount = 0
185+
186+
const agent = new Agent({
187+
webSocket: {
188+
maxFragments: 3
189+
}
190+
})
191+
192+
const server = new WebSocketServer({ port: 0 }, () => {
193+
const { port } = server.address()
194+
const client = new WebSocket(`ws://127.0.0.1:${port}`, {
195+
dispatcher: agent
196+
})
197+
198+
client.addEventListener('error', () => {
199+
assert.ok(true)
200+
})
201+
202+
client.addEventListener('close', (event) => {
203+
assert.strictEqual(event.code, 1006)
204+
maybeDone()
205+
})
206+
})
207+
208+
server.on('connection', (ws) => {
209+
ws.on('close', (code, reason) => {
210+
assert.strictEqual(code, 1008)
211+
assert.strictEqual(reason.toString(), 'Too many message fragments')
212+
maybeDone()
213+
})
214+
215+
const fragment = ''
216+
const options = { fin: false }
217+
218+
ws.send(fragment, options) // Text frame fin=0, len=0
219+
ws.send(fragment, options) // Continuation fin=0, len=0
220+
ws.send(fragment, options) // Continuation fin=0, len=0
221+
ws.send(fragment, options) // Continuation fin=0, len=0
222+
})
223+
})

test/websocket/permessage-deflate-limit.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,3 +420,47 @@ test('Raw uncompressed payload over 64-bit extended limit is rejected', async (t
420420
assert.strictEqual(messageReceived, false, 'Raw uncompressed message over limit should be rejected')
421421
assert.strictEqual(client.readyState, WebSocket.CLOSED, 'Connection should be closed after exceeding limit')
422422
})
423+
424+
test('cumulative payload size', (t, done) => {
425+
const LIMIT = 100
426+
const FRAGMENT_SIZE = 60
427+
const NUM_FRAGMENTS = 10
428+
429+
const server = new WebSocketServer({ port: 0 })
430+
431+
server.on('connection', (ws) => {
432+
const socket = ws._socket
433+
const payload = Buffer.alloc(FRAGMENT_SIZE, 0x41)
434+
435+
for (let i = 0; i < NUM_FRAGMENTS; i++) {
436+
const fin = i === NUM_FRAGMENTS - 1 ? 0x80 : 0x00
437+
const opcode = i === 0 ? 0x02 : 0x00
438+
const header = Buffer.alloc(2)
439+
header[0] = fin | opcode
440+
header[1] = FRAGMENT_SIZE
441+
socket.write(header)
442+
socket.write(payload)
443+
}
444+
})
445+
446+
const agent = new Agent({
447+
webSocket: {
448+
maxPayloadSize: LIMIT
449+
}
450+
})
451+
452+
const client = new WebSocket(`ws://127.0.0.1:${server.address().port}`, { dispatcher: agent })
453+
454+
t.after(async () => {
455+
client.close()
456+
server.close()
457+
await agent.close()
458+
})
459+
460+
client.onmessage = () => assert.fail('message should not be received')
461+
462+
client.addEventListener('error', (event) => {
463+
assert.ok(event)
464+
done()
465+
})
466+
})

types/client.d.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@ export declare namespace Client {
106106
bytesRead?: number
107107
}
108108
export interface WebSocketOptions {
109+
/**
110+
* Maximum number of fragments in a message.
111+
* Set to 0 to disable the limit.
112+
* @default 131072
113+
*/
114+
maxFragments?: number;
109115
/**
110116
* Maximum allowed payload size in bytes for WebSocket messages.
111117
* Applied to uncompressed messages, compressed frame payloads, and decompressed (permessage-deflate) messages.

0 commit comments

Comments
 (0)