From cc5cfe22a5a3b416d1e2aa12a0739c5919983efb Mon Sep 17 00:00:00 2001
From: Jan Thurau <jan@janthurau.de>
Date: Wed, 19 Oct 2022 18:50:46 +0200
Subject: [PATCH] Moves websocket error handling to the first possible place to
 avoid race condition (#425)

---
 packages/server/src/Connection.ts | 12 ------------
 packages/server/src/Hocuspocus.ts | 12 ++++++++++++
 tests/server/websocketError.ts    | 31 +++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/packages/server/src/Connection.ts b/packages/server/src/Connection.ts
index 83684a7c0..d7931554e 100644
--- a/packages/server/src/Connection.ts
+++ b/packages/server/src/Connection.ts
@@ -72,7 +72,6 @@ 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()
   }
@@ -197,17 +196,6 @@ export class Connection {
       })
   }
 
-  /**
-   * 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
diff --git a/packages/server/src/Hocuspocus.ts b/packages/server/src/Hocuspocus.ts
index 588b8b4f8..045b9cba4 100644
--- a/packages/server/src/Hocuspocus.ts
+++ b/packages/server/src/Hocuspocus.ts
@@ -176,6 +176,18 @@ export class Hocuspocus {
     const webSocketServer = new WebSocketServer({ noServer: true })
 
     webSocketServer.on('connection', async (incoming: WebSocket, request: IncomingMessage) => {
+
+      incoming.on('error', error => {
+        /**
+         * 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
+         */
+        this.debugger.log('Error emitted from webSocket instance:')
+        this.debugger.log(error)
+      })
+
       this.handleConnection(incoming, request, await this.getDocumentNameFromRequest(request))
     })
 
diff --git a/tests/server/websocketError.ts b/tests/server/websocketError.ts
index 35b4b2990..1ad32351a 100644
--- a/tests/server/websocketError.ts
+++ b/tests/server/websocketError.ts
@@ -1,4 +1,5 @@
 import test from 'ava'
+import { onAuthenticatePayload } from '@hocuspocus/server'
 import { newHocuspocus, newHocuspocusProvider } from '../utils'
 
 test('does not crash when invalid opcode is sent', async t => {
@@ -23,3 +24,33 @@ test('does not crash when invalid opcode is sent', async t => {
     })
   })
 })
+
+test('does not crash when invalid utf-8 sequence is sent', async t => {
+  await new Promise(resolve => {
+    const server = newHocuspocus({
+      async onAuthenticate(data: onAuthenticatePayload) {
+        return new Promise(resolve => {
+          setTimeout(resolve, 2000)
+        })
+      },
+    })
+
+    const provider = newHocuspocusProvider(server, {
+      token: 'test123',
+      onClose({ event }) {
+        t.is(event.code, 1002)
+        provider.destroy()
+      },
+      onDestroy() {
+        t.pass()
+        resolve(true)
+      },
+    })
+
+    setInterval(() => {
+      // @ts-ignore
+      provider.webSocket?._socket.write(Buffer.from([0x81, 0x04, 0xce, 0xba, 0xe1, 0xbd])) // eslint-disable-line
+    }, 500)
+
+  })
+})