diff --git a/lib/websocket/frame.js b/lib/websocket/frame.js index 1df5e16934b..61bfd3915ce 100644 --- a/lib/websocket/frame.js +++ b/lib/websocket/frame.js @@ -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 diff --git a/test/websocket/frame.js b/test/websocket/frame.js new file mode 100644 index 00000000000..b4b73b7cf08 --- /dev/null +++ b/test/websocket/frame.js @@ -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 +})