fix(node-ws): binary messages have wrong byteLength and allocates unnecessary `Buffer` instance (#639)

* fix: no more Buffer.from usage

* test: add test for binary messages

* chore: add changeset
pull/642/head
Quentin 2024-07-15 10:30:50 +02:00 committed by GitHub
parent 1f5858ec55
commit 2f307e6877
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 34 additions and 2 deletions

View File

@ -0,0 +1,5 @@
---
'@hono/node-ws': patch
---
Fixed wrong byteLength on binary messages

View File

@ -3,6 +3,7 @@ import type { ServerType } from '@hono/node-server/dist/types'
import { Hono } from 'hono'
import { WebSocket } from 'ws'
import { createNodeWebSocket } from '.'
import type { WSMessageReceive } from 'hono/ws'
describe('WebSocket helper', () => {
let app: Hono
@ -106,4 +107,31 @@ describe('WebSocket helper', () => {
connections.forEach((ws) => ws.close())
})
it('Should be able to send and receive binary content with good length', async () => {
const mainPromise = new Promise<WSMessageReceive>((resolve) =>
app.get(
'/',
upgradeWebSocket(() => ({
onMessage(data) {
resolve(data.data)
},
}))
)
)
const binaryData = new Uint8Array(Array.from({ length: 16 }).map((_, i) => i ** 2))
const ws = new WebSocket('ws://localhost:3030/')
await new Promise<void>((resolve) => ws.on('open', resolve))
ws.send(binaryData)
const receivedMessage = await mainPromise;
expect(receivedMessage).toBeInstanceOf(Buffer)
expect((receivedMessage as Buffer).byteLength).toBe(binaryData.length)
binaryData.forEach((val, idx) => {
expect((receivedMessage as Buffer).at(idx)).toBe(val)
})
})
})

View File

@ -94,10 +94,9 @@ export const createNodeWebSocket = (init: NodeWebSocketInit): NodeWebSocket => {
ws.on('message', (data, isBinary) => {
const datas = Array.isArray(data) ? data : [data]
for (const data of datas) {
const buff: Buffer = Buffer.from(data)
events.onMessage?.(
new MessageEvent('message', {
data: isBinary ? buff.buffer : buff.toString('utf-8'),
data: isBinary ? data : data.toString('utf-8'),
}),
ctx
)