fix: fix cookie management with WebSocket (Node.js only)

darrachequesne · darrachequesne · commit e105551ef17f · 2024-06-03T09:58:56.000+02:00
Before this commit, the cookies were only sent with the HTTP long-polling transport, and not when upgrading to WebSocket. See also: 5fc88a6
diff --git a/lib/globals.node.ts b/lib/globals.node.ts
@@ -68,7 +68,7 @@ export function parse(setCookieString: string): Cookie {
 }
 
 export class CookieJar {
-  private cookies = new Map<string, Cookie>();
+  private _cookies = new Map<string, Cookie>();
 
   public parseCookies(values: string[]) {
     if (!values) {
@@ -77,21 +77,27 @@ export class CookieJar {
     values.forEach((value) => {
       const parsed = parse(value);
       if (parsed) {
-        this.cookies.set(parsed.name, parsed);
+        this._cookies.set(parsed.name, parsed);
       }
     });
   }
 
+  get cookies() {
+    const now = Date.now();
+    this._cookies.forEach((cookie, name) => {
+      if (cookie.expires?.getTime() < now) {
+        this._cookies.delete(name);
+      }
+    });
+    return this._cookies.entries();
+  }
+
   public addCookies(xhr: any) {
     const cookies = [];
 
-    this.cookies.forEach((cookie, name) => {
-      if (cookie.expires?.getTime() < Date.now()) {
-        this.cookies.delete(name);
-      } else {
-        cookies.push(`${name}=${cookie.value}`);
-      }
-    });
+    for (const [name, cookie] of this.cookies) {
+      cookies.push(`${name}=${cookie.value}`);
+    }
 
     if (cookies.length) {
       xhr.setDisableHeaderCheck(true);
@@ -100,12 +106,8 @@ export class CookieJar {
   }
 
   public appendCookies(headers: Headers) {
-    this.cookies.forEach((cookie, name) => {
-      if (cookie.expires?.getTime() < Date.now()) {
-        this.cookies.delete(name);
-      } else {
-        headers.append("cookie", `${name}=${cookie.value}`);
-      }
-    });
+    for (const [name, cookie] of this.cookies) {
+      headers.append("cookie", `${name}=${cookie.value}`);
+    }
   }
 }
diff --git a/lib/socket.ts b/lib/socket.ts
@@ -6,7 +6,11 @@ import { Emitter } from "@socket.io/component-emitter";
 import { protocol } from "engine.io-parser";
 import type { Packet, BinaryType, PacketType, RawData } from "engine.io-parser";
 import { CloseDetails, Transport } from "./transport.js";
-import { defaultBinaryType } from "./globals.node.js";
+import {
+  CookieJar,
+  createCookieJar,
+  defaultBinaryType,
+} from "./globals.node.js";
 import debugModule from "debug"; // debug()
 
 const debug = debugModule("engine.io-client:socket"); // debug()
@@ -322,6 +326,10 @@ export class SocketWithoutUpgrade extends Emitter<
   private readonly hostname: string;
   private readonly port: string | number;
   private readonly transportsByName: Record<string, TransportCtor>;
+  /**
+   * The cookie jar will store the cookies sent by the server (Node. js only).
+   */
+  /* private */ readonly _cookieJar: CookieJar;
 
   static priorWebsocketSuccess: boolean;
   static protocol = protocol;
@@ -444,6 +452,10 @@ export class SocketWithoutUpgrade extends Emitter<
       }
     }
 
+    if (this.opts.withCredentials) {
+      this._cookieJar = createCookieJar();
+    }
+
     this.open();
   }
 
diff --git a/lib/transport.ts b/lib/transport.ts
@@ -2,9 +2,9 @@ import { decodePacket } from "engine.io-parser";
 import type { Packet, RawData } from "engine.io-parser";
 import { Emitter } from "@socket.io/component-emitter";
 import { installTimerFunctions } from "./util.js";
-import debugModule from "debug"; // debug()
-import { SocketOptions } from "./socket.js";
+import type { Socket, SocketOptions } from "./socket.js";
 import { encode } from "./contrib/parseqs.js";
+import debugModule from "debug"; // debug()
 
 const debug = debugModule("engine.io-client:transport"); // debug()
 
@@ -48,7 +48,7 @@ export abstract class Transport extends Emitter<
   protected opts: SocketOptions;
   protected supportsBinary: boolean;
   protected readyState: TransportState;
-  protected socket: any;
+  protected socket: Socket;
   protected setTimeoutFn: typeof setTimeout;
 
   /**
diff --git a/lib/transports/polling-fetch.ts b/lib/transports/polling-fetch.ts
@@ -10,16 +10,6 @@ import { CookieJar, createCookieJar } from "../globals.node.js";
  * @see https://caniuse.com/fetch
  */
 export class Fetch extends Polling {
-  private readonly cookieJar?: CookieJar;
-
-  constructor(opts) {
-    super(opts);
-
-    if (this.opts.withCredentials) {
-      this.cookieJar = createCookieJar();
-    }
-  }
-
   override doPoll() {
     this._fetch()
       .then((res) => {
@@ -56,18 +46,16 @@ export class Fetch extends Polling {
       headers.set("content-type", "text/plain;charset=UTF-8");
     }
 
-    this.cookieJar?.appendCookies(headers);
+    this.socket._cookieJar?.appendCookies(headers);
 
     return fetch(this.uri(), {
       method: isPost ? "POST" : "GET",
       body: isPost ? data : null,
       headers,
       credentials: this.opts.withCredentials ? "include" : "omit",
     }).then((res) => {
-      if (this.cookieJar) {
-        // @ts-ignore getSetCookie() was added in Node.js v19.7.0
-        this.cookieJar.parseCookies(res.headers.getSetCookie());
-      }
+      // @ts-ignore getSetCookie() was added in Node.js v19.7.0
+      this.socket._cookieJar?.parseCookies(res.headers.getSetCookie());
 
       return res;
     });
diff --git a/lib/transports/polling-xhr.node.ts b/lib/transports/polling-xhr.node.ts
@@ -12,7 +12,11 @@ const XMLHttpRequest = XMLHttpRequestModule.default || XMLHttpRequestModule;
  */
 export class XHR extends BaseXHR {
   request(opts: Record<string, any> = {}) {
-    Object.assign(opts, { xd: this.xd, cookieJar: this.cookieJar }, this.opts);
+    Object.assign(
+      opts,
+      { xd: this.xd, cookieJar: this.socket?._cookieJar },
+      this.opts
+    );
     return new Request(
       (opts) => new XMLHttpRequest(opts),
       this.uri(),
diff --git a/lib/transports/polling-xhr.ts b/lib/transports/polling-xhr.ts
@@ -17,7 +17,6 @@ function empty() {}
 
 export abstract class BaseXHR extends Polling {
   protected readonly xd: boolean;
-  protected readonly cookieJar?: CookieJar;
 
   private pollXhr: any;
 
@@ -44,10 +43,6 @@ export abstract class BaseXHR extends Polling {
           opts.hostname !== location.hostname) ||
         port !== opts.port;
     }
-
-    if (this.opts.withCredentials) {
-      this.cookieJar = createCookieJar();
-    }
   }
 
   /**
@@ -344,7 +339,7 @@ export class XHR extends BaseXHR {
   }
 
   request(opts: Record<string, any> = {}) {
-    Object.assign(opts, { xd: this.xd, cookieJar: this.cookieJar }, this.opts);
+    Object.assign(opts, { xd: this.xd }, this.opts);
     return new Request(newRequest, this.uri(), opts as RequestOptions);
   }
 }
diff --git a/lib/transports/websocket.node.ts b/lib/transports/websocket.node.ts
@@ -16,6 +16,17 @@ export class WS extends BaseWS {
     protocols: string | string[] | undefined,
     opts: Record<string, any>
   ) {
+    if (this.socket?._cookieJar) {
+      opts.headers = opts.headers || {};
+
+      opts.headers.cookie =
+        typeof opts.headers.cookie === "string"
+          ? [opts.headers.cookie]
+          : opts.headers.cookie || [];
+      for (const [name, cookie] of this.socket._cookieJar.cookies) {
+        opts.headers.cookie.push(`${name}=${cookie.value}`);
+      }
+    }
     return new WebSocket(uri, protocols, opts);
   }
 

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,6 @@ function empty() {}`
`17`	`17`
`18`	`18`	`export abstract class BaseXHR extends Polling {`
`19`	`19`	`protected readonly xd: boolean;`
`20`		`- protected readonly cookieJar?: CookieJar;`
`21`	`20`
`22`	`21`	`private pollXhr: any;`
`23`	`22`
`@@ -44,10 +43,6 @@ export abstract class BaseXHR extends Polling {`
`44`	`43`	`opts.hostname !== location.hostname) \|\|`
`45`	`44`	`port !== opts.port;`
`46`	`45`	`}`
`47`		`-`
`48`		`- if (this.opts.withCredentials) {`
`49`		`- this.cookieJar = createCookieJar();`
`50`		`- }`
`51`	`46`	`}`
`52`	`47`
`53`	`48`	`/**`
`@@ -344,7 +339,7 @@ export class XHR extends BaseXHR {`
`344`	`339`	`}`
`345`	`340`
`346`	`341`	`request(opts: Record<string, any> = {}) {`
`347`		`- Object.assign(opts, { xd: this.xd, cookieJar: this.cookieJar }, this.opts);`
	`342`	`+ Object.assign(opts, { xd: this.xd }, this.opts);`
`348`	`343`	`return new Request(newRequest, this.uri(), opts as RequestOptions);`
`349`	`344`	`}`
`350`	`345`	`}`