Skip to content

Commit

Permalink
fix: add some randomness to the cache busting string generator
Browse files Browse the repository at this point in the history
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
  • Loading branch information
darrachequesne committed Jun 5, 2024
1 parent c087dc5 commit b624c50
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 67 deletions.
62 changes: 0 additions & 62 deletions lib/contrib/yeast.ts

This file was deleted.

4 changes: 2 additions & 2 deletions lib/transports/polling.ts
Original file line number Diff line number Diff line change
@@ -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()

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions lib/transports/websocket.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
11 changes: 11 additions & 0 deletions test/engine.io-client.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
});
});

0 comments on commit b624c50

Please sign in to comment.