fix(ws): Limit server emitted error close message size

enisdenjo · Oct 27, 2021 · 50620df · 50620df
1 parent 994c9cb
commit 50620df
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 15 deletions.
diff --git a/src/__tests__/use.ts b/src/__tests__/use.ts
@@ -28,7 +28,7 @@ afterAll(() => {
   console.error = consoleError;
 });
 
-for (const { tServer, startTServer } of tServers) {
+for (const { tServer, itForWS, startTServer } of tServers) {
   describe(tServer, () => {
     it('should allow connections with valid protocols only', async () => {
       const { url } = await startTServer();
@@ -354,24 +354,56 @@ for (const { tServer, startTServer } of tServers) {
       });
     });
 
-    it /* itForWS */.todo(
-      'should report server errors to clients by closing the connection',
-      // async () => {
-      //   const { url, ws } = await startTServer();
+    // uWebSocket.js cannot have errors emitted on the server instance
+    // TODO-db-211027 fastify-websocket
+    itForWS(
+      'should report server emitted errors to clients by closing the connection',
+      async () => {
+        const { url, server } = await startTServer();
 
-      //   const client = await createTClient(url);
+        const client = await createTClient(url);
 
-      //   const emittedError = new Error("I'm a teapot");
-      //   ws.emit('error', emittedError);
+        const emittedError = new Error("I'm a teapot");
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        server!.emit('error', emittedError);
 
-      //   await client.waitForClose((event) => {
-      //     expect(event.code).toBe(CloseCode.InternalServerError); // CloseCode.InternalServerError: Internal server error
-      //     expect(event.reason).toBe(emittedError.message);
-      //     expect(event.wasClean).toBeTruthy(); // because the server reported the error
-      //   });
-      // },
+        await client.waitForClose((event) => {
+          expect(event.code).toBe(CloseCode.InternalServerError); // CloseCode.InternalServerError: Internal server error
+          expect(event.reason).toBe(emittedError.message);
+          expect(event.wasClean).toBeTruthy(); // because the server reported the error
+        });
+      },
     );
 
+    // uWebSocket.js cannot have errors emitted on the server instance
+    // TODO-db-211027 fastify-websocket
+    itForWS('should limit the server emitted error message size', async () => {
+      const { url, server, waitForClient } = await startTServer();
+
+      const client = await createTClient(url);
+      client.ws.send(
+        stringifyMessage<MessageType.ConnectionInit>({
+          type: MessageType.ConnectionInit,
+        }),
+      );
+
+      await waitForClient();
+
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      server!.emit(
+        'error',
+        new Error(
+          'i am exactly 124 characters long i am exactly 124 characters long i am exactly 124 characters long i am exactly 124 characte',
+        ),
+      );
+
+      await client.waitForClose((event) => {
+        expect(event.code).toBe(CloseCode.InternalServerError);
+        expect(event.reason).toBe('Internal server error');
+        expect(event.wasClean).toBeTruthy(); // because the server reported the error
+      });
+    });
+
     it('should limit the internal server error message size', async () => {
       const { url } = await startTServer({
         onConnect: () => {

diff --git a/src/__tests__/utils/tservers.ts b/src/__tests__/utils/tservers.ts
@@ -38,6 +38,7 @@ export interface TServerClient {
 
 export interface TServer {
   url: string;
+  server: WebSocketServer | null; // null when uWS because it does not have a server instance
   getClients: () => TServerClient[];
   pong: (key?: string) => void;
   waitForClient: (
@@ -227,6 +228,7 @@ export async function startWSTServer(
 
   return {
     url: `ws://localhost:${port}${path}`,
+    server: wsServer,
     getClients() {
       return Array.from(wsServer.clients, toClient);
     },
@@ -393,6 +395,7 @@ export async function startUWSTServer(
 
   return {
     url: `ws://localhost:${port}${path}`,
+    server: null,
     // @ts-expect-error TODO-db-210410
     getClients: null,
     // @ts-expect-error TODO-db-210410
@@ -554,6 +557,7 @@ export async function startFastifyWSTServer(
 
   return {
     url: `ws://localhost:${port}${path}`,
+    server: fastify.websocketServer,
     getClients() {
       return Array.from(fastify.websocketServer.clients, toClient);
     },
@@ -651,20 +655,29 @@ export const tServers = [
   {
     tServer: 'ws' as const,
     startTServer: startWSTServer,
+    skipWS: it.skip,
+    skipUWS: it,
+    skipFastify: it,
     itForWS: it,
     itForUWS: it.skip,
     itForFastify: it.skip,
   },
   {
     tServer: 'uWebSockets.js' as const,
     startTServer: startUWSTServer,
+    skipWS: it,
+    skipUWS: it.skip,
+    skipFastify: it,
     itForWS: it.skip,
     itForUWS: it,
     itForFastify: it.skip,
   },
   {
     tServer: 'fastify-websocket' as const,
     startTServer: startFastifyWSTServer,
+    skipWS: it,
+    skipUWS: it,
+    skipFastify: it.skip,
     itForWS: it.skip,
     itForUWS: it.skip,
     itForFastify: it,

diff --git a/src/use/ws.ts b/src/use/ws.ts
@@ -60,7 +60,10 @@ export function useServer<
       try {
         client.close(
           CloseCode.InternalServerError,
-          isProd ? 'Internal server error' : err.message,
+          // close reason should fit in one frame https://datatracker.ietf.org/doc/html/rfc6455#section-5.2
+          isProd || err.message.length > 123
+            ? 'Internal server error'
+            : err.message,
         );
       } catch (err) {
         firstErr = firstErr ?? err;