Skip to content

Commit

Permalink
fix (Connection): Add error handler to websocket instance (#393)
Browse files Browse the repository at this point in the history
* add error handler to websocket instance to avoid crashing when receiving an invalid websocket frame
  • Loading branch information
jamesopti committed Sep 2, 2022
1 parent bdff442 commit 8b62783
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
12 changes: 12 additions & 0 deletions packages/server/src/Connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class Connection {
this.webSocket.on('close', this.close.bind(this))
this.webSocket.on('message', this.handleMessage.bind(this))
this.webSocket.on('pong', () => { this.pongReceived = true })
this.webSocket.on('error', this.handleError.bind(this))

this.sendCurrentAwareness()
}
Expand Down Expand Up @@ -175,6 +176,17 @@ export class Connection {
).apply(this.document, this)
}

/**
* Handle a ws instance error, which is required to prevent
* the server from crashing when one happens
* See https://github.com/websockets/ws/issues/1777#issuecomment-660803472
* @private
*/
private handleError(error: any): void {
this.logger.log('Error emitted from webSocket instance:')
this.logger.log(error)
}

/**
* Get the underlying connection instance
* @deprecated
Expand Down
25 changes: 25 additions & 0 deletions tests/server/websocketError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import test from 'ava'
import { newHocuspocus, newHocuspocusProvider } from '../utils'

test('does not crash when invalid opcode is sent', async t => {
await new Promise(resolve => {
const server = newHocuspocus()

const provider = newHocuspocusProvider(server, {
onSynced() {
// Send a bad opcode via the low level internal _socket
// Inspired by https://github.com/websockets/ws/blob/975382178f8a9355a5a564bb29cb1566889da9ba/test/websocket.test.js#L553-L589
// @ts-ignore
provider.webSocket?._socket.write(Buffer.from([0x00, 0x00])) // eslint-disable-line
},
onClose({ event }) {
t.is(event.code, 1002)
provider.destroy()
},
onDestroy() {
t.pass()
resolve(true)
},
})
})
})

0 comments on commit 8b62783

Please sign in to comment.