fix: add some randomness to the cache busting string generator

darrachequesne · darrachequesne · commit b624c5083256 · 2024-06-05T13:44:55.000+02:00
The yeast() method could generate the same string twice when used in two different iframes, which can cause Safari to only send one HTTP request (deduplication) and trigger an HTTP 400 error afterwards since the two iframes share the same session ID. This new method, combining 5 chars from the timestamp and 3 chars from Math.random() should be sufficient for our use case. Related: socketio/engine.io#690 See also: 874484c
diff --git a/lib/contrib/yeast.ts b/lib/contrib/yeast.ts
diff --git a/lib/transports/polling.ts b/lib/transports/polling.ts
@@ -1,5 +1,5 @@
 import { Transport } from "../transport.js";
-import { yeast } from "../contrib/yeast.js";
+import { randomString } from "../util.js";
 import { encodePayload, decodePayload } from "engine.io-parser";
 import debugModule from "debug"; // debug()
 
@@ -164,7 +164,7 @@ export abstract class Polling extends Transport {
 
     // cache busting is forced
     if (false !== this.opts.timestampRequests) {
-      query[this.opts.timestampParam] = yeast();
+      query[this.opts.timestampParam] = randomString();
     }
 
     if (!this.supportsBinary && !query.sid) {
diff --git a/lib/transports/websocket.ts b/lib/transports/websocket.ts
@@ -1,6 +1,5 @@
 import { Transport } from "../transport.js";
-import { yeast } from "../contrib/yeast.js";
-import { pick } from "../util.js";
+import { pick, randomString } from "../util.js";
 import { encodePacket } from "engine.io-parser";
 import type { Packet, RawData } from "engine.io-parser";
 import { globalThisShim as globalThis, nextTick } from "../globals.node.js";
@@ -140,7 +139,7 @@ export abstract class BaseWS extends Transport {
 
     // append timestamp to URI
     if (this.opts.timestampRequests) {
-      query[this.opts.timestampParam] = yeast();
+      query[this.opts.timestampParam] = randomString();
     }
 
     // communicate binary support capabilities
diff --git a/lib/util.ts b/lib/util.ts
@@ -53,3 +53,13 @@ function utf8Length(str) {
   }
   return length;
 }
+
+/**
+ * Generates a random 8-characters string.
+ */
+export function randomString() {
+  return (
+    Date.now().toString(36).substring(3) +
+    Math.random().toString(36).substring(2, 5)
+  );
+}
diff --git a/test/engine.io-client.js b/test/engine.io-client.js
@@ -1,5 +1,6 @@
 const expect = require("expect.js");
 const { Socket, protocol } = require("..");
+const { randomString } = require("../build/cjs/util.js");
 
 const expectedPort =
   typeof location !== "undefined" && "https:" === location.protocol
@@ -99,4 +100,14 @@ describe("engine.io-client", () => {
     expect(client.hostname).to.be("::1");
     expect(client.port).to.be(expectedPort);
   });
+
+  it("should generate a random string", () => {
+    const a = randomString();
+    const b = randomString();
+    const c = randomString();
+
+    expect(a.length).to.eql(8);
+    expect(a).to.not.equal(b);
+    expect(b).to.not.equal(c);
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -53,3 +53,13 @@ function utf8Length(str) {`
`53`	`53`	`}`
`54`	`54`	`return length;`
`55`	`55`	`}`
	`56`	`+`
	`57`	`+/**`
	`58`	`+ * Generates a random 8-characters string.`
	`59`	`+ */`
	`60`	`+export function randomString() {`
	`61`	`+ return (`
	`62`	`+ Date.now().toString(36).substring(3) +`
	`63`	`+ Math.random().toString(36).substring(2, 5)`
	`64`	`+ );`
	`65`	`+}`