Skip to content

Commit

Permalink
Fix bug in 16-bit frame length when buffer is a subarray (nodejs#2106)
Browse files Browse the repository at this point in the history
  • Loading branch information
jawj authored and crysmags committed Feb 27, 2024
1 parent c1adfaa commit b89dd77
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/websocket/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class WebsocketFrameSend {
buffer[1] = payloadLength

if (payloadLength === 126) {
new DataView(buffer.buffer).setUint16(2, bodyLength)
buffer.writeUInt16BE(bodyLength, 2)
} else if (payloadLength === 127) {
// Clear extended payload length
buffer[2] = buffer[3] = 0
Expand Down
24 changes: 24 additions & 0 deletions test/websocket/frame.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

const { test } = require('tap')
const { WebsocketFrameSend } = require('../../lib/websocket/frame')
const { opcodes } = require('../../lib/websocket/constants')

test('Writing 16-bit frame length value at correct offset when buffer has a non-zero byteOffset', (t) => {
/*
When writing 16-bit frame lengths, a `DataView` was being used without setting a `byteOffset` into the buffer:
i.e. `new DataView(buffer.buffer)` instead of `new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength)`.
Small `Buffers` returned by `allocUnsafe` are usually returned from the buffer pool, and thus have a non-zero `byteOffset`.
Invalid frames were therefore being returned in that case.
*/
t.plan(3)

const payloadLength = 126 // 126 bytes is the smallest payload to trigger a 16-bit length field
const smallBuffer = Buffer.allocUnsafe(1) // make it very likely that the next buffer returned by allocUnsafe DOESN'T have a zero byteOffset
const payload = Buffer.allocUnsafe(payloadLength).fill(0)
const frame = new WebsocketFrameSend(payload).createFrame(opcodes.BINARY)

t.equal(frame[2], payloadLength >>> 8)
t.equal(frame[3], payloadLength & 0xff)
t.equal(smallBuffer.length, 1) // ensure smallBuffer can't be garbage-collected too soon
})

0 comments on commit b89dd77

Please sign in to comment.