Przeglądaj źródła

fix: update upgrade handler to write error responses without closing the socket

Yuki Takei 2 tygodni temu
rodzic
commit
df4e69d3da

+ 0 - 2
apps/app/src/server/service/yjs/upgrade-handler.spec.ts

@@ -94,7 +94,6 @@ describe('UpgradeHandler', () => {
       expect(result.statusCode).toBe(400);
       expect(result.statusCode).toBe(400);
     }
     }
     expect(socket.write).toHaveBeenCalledWith(expect.stringContaining('400'));
     expect(socket.write).toHaveBeenCalledWith(expect.stringContaining('400'));
-    expect(socket.destroy).toHaveBeenCalled();
   });
   });
 
 
   it('should reject with 403 when user has no page access', async () => {
   it('should reject with 403 when user has no page access', async () => {
@@ -114,7 +113,6 @@ describe('UpgradeHandler', () => {
       expect(result.statusCode).toBe(403);
       expect(result.statusCode).toBe(403);
     }
     }
     expect(socket.write).toHaveBeenCalledWith(expect.stringContaining('403'));
     expect(socket.write).toHaveBeenCalledWith(expect.stringContaining('403'));
-    expect(socket.destroy).toHaveBeenCalled();
   });
   });
 
 
   it('should reject with 401 when unauthenticated user has no page access', async () => {
   it('should reject with 401 when unauthenticated user has no page access', async () => {

+ 7 - 6
apps/app/src/server/service/yjs/upgrade-handler.ts

@@ -57,15 +57,16 @@ const extractPageId = (url: string | undefined): string | null => {
 };
 };
 
 
 /**
 /**
- * Rejects a WebSocket upgrade request with an HTTP error response.
+ * Writes an HTTP error response to the socket.
+ * Does NOT close the socket — the caller (yjs.ts) manages socket lifecycle
+ * so that guardSocket can safely intercept end/destroy during async auth.
  */
  */
-const rejectUpgrade = (
+const writeErrorResponse = (
   socket: Duplex,
   socket: Duplex,
   statusCode: number,
   statusCode: number,
   message: string,
   message: string,
 ): void => {
 ): void => {
   socket.write(`HTTP/1.1 ${statusCode} ${message}\r\n\r\n`);
   socket.write(`HTTP/1.1 ${statusCode} ${message}\r\n\r\n`);
-  socket.destroy();
 };
 };
 
 
 export type UpgradeResult =
 export type UpgradeResult =
@@ -89,7 +90,7 @@ export const createUpgradeHandler = (sessionConfig: SessionConfig) => {
     const pageId = extractPageId(request.url);
     const pageId = extractPageId(request.url);
     if (pageId == null) {
     if (pageId == null) {
       logger.warn('Invalid URL path for Yjs upgrade', { url: request.url });
       logger.warn('Invalid URL path for Yjs upgrade', { url: request.url });
-      rejectUpgrade(socket, 400, 'Bad Request');
+      writeErrorResponse(socket, 400, 'Bad Request');
       return { authorized: false, statusCode: 400 };
       return { authorized: false, statusCode: 400 };
     }
     }
 
 
@@ -100,7 +101,7 @@ export const createUpgradeHandler = (sessionConfig: SessionConfig) => {
       await runMiddleware(passportSession as ConnectMiddleware, request);
       await runMiddleware(passportSession as ConnectMiddleware, request);
     } catch (err) {
     } catch (err) {
       logger.warn('Session/passport middleware failed on upgrade', { err });
       logger.warn('Session/passport middleware failed on upgrade', { err });
-      rejectUpgrade(socket, 401, 'Unauthorized');
+      writeErrorResponse(socket, 401, 'Unauthorized');
       return { authorized: false, statusCode: 401 };
       return { authorized: false, statusCode: 401 };
     }
     }
 
 
@@ -117,7 +118,7 @@ export const createUpgradeHandler = (sessionConfig: SessionConfig) => {
         pageId,
         pageId,
         userId: user?._id,
         userId: user?._id,
       });
       });
-      rejectUpgrade(socket, statusCode, message);
+      writeErrorResponse(socket, statusCode, message);
       return { authorized: false, statusCode };
       return { authorized: false, statusCode };
     }
     }
 
 

+ 0 - 8
apps/app/src/server/service/yjs/yjs.ts

@@ -201,14 +201,6 @@ export const initializeYjsService = (
     throw new Error('YjsService is already initialized');
     throw new Error('YjsService is already initialized');
   }
   }
 
 
-  if (httpServer == null) {
-    throw new Error("'httpServer' is required to initialize YjsService");
-  }
-
-  if (io == null) {
-    throw new Error("'io' is required to initialize YjsService");
-  }
-
   _instance = new YjsService(httpServer, io, sessionConfig);
   _instance = new YjsService(httpServer, io, sessionConfig);
 };
 };