From f9da2abeab16401fa0ad2fba02efb3858d6f2ffd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:50:01 +0000 Subject: [PATCH 1/5] install: hash patch contents with SHA-1 instead of Wyhash11 A constructed Wyhash11 collision let two distinct patch files share one `_patch_hash=` cache folder, so the second project installed the first's patched package. Derive the u64 from SHA-1 instead; the folder-suffix / tag-file interface is unchanged. See #32741. Adds the patch-cache collision regression test, plus a SQL prepared-statement collision regression test covering the name-keyed statement cache (the MySQL/Postgres hash-collision fix itself landed separately on main). --- src/install/patch_install.rs | 10 +- test/cli/install/bun-install-patch.test.ts | 65 +++++++++ ...sql-statement-cache-hash-collision.test.ts | 133 ++++++++++++++++++ 3 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 test/js/sql/sql-statement-cache-hash-collision.test.ts diff --git a/src/install/patch_install.rs b/src/install/patch_install.rs index 4b41e75de78c..6dc36b38607e 100644 --- a/src/install/patch_install.rs +++ b/src/install/patch_install.rs @@ -12,7 +12,6 @@ use bun_semver::String as SemverString; use bun_sys::{self as sys, Fd, FdExt}; use bun_threading::IntrusiveWorkTask as _; use bun_threading::thread_pool::{Batch, Node as ThreadPoolNode, Task as ThreadPoolTask}; -use bun_wyhash::Wyhash11; use crate::package_install::PackageInstall; use crate::package_manager; @@ -694,7 +693,10 @@ impl PatchTask { sys::Result::Ok(f) => f, }; - let mut hasher = Wyhash11::init(0); + // SHA-1/64 (not Wyhash11): a constructed wyhash collision would let + // two distinct patches share one `_patch_hash=` cache folder. + // Truncation keeps the u64 folder-suffix / tag-file interface. + let mut hasher = bun_sha_hmac::sha::hashers::SHA1::init(); // what's a good number for this? page size i guess const STACK_SIZE: usize = 16384; @@ -723,7 +725,9 @@ impl PatchTask { read += slice.len(); } - Some(hasher.final_()) + let mut digest = [0u8; bun_sha_hmac::sha::hashers::SHA1::DIGEST]; + hasher.r#final(&mut digest); + Some(u64::from_le_bytes(digest[0..8].try_into().unwrap())) } pub(crate) fn schedule(&mut self, batch: &mut Batch) { diff --git a/test/cli/install/bun-install-patch.test.ts b/test/cli/install/bun-install-patch.test.ts index 0720577c1e3f..04bcf0ba4cd5 100644 --- a/test/cli/install/bun-install-patch.test.ts +++ b/test/cli/install/bun-install-patch.test.ts @@ -1023,3 +1023,68 @@ index 0000000000000000000000000000000000000000..2f9a147b6e5d17254f1bfce0d4e109a2 expect(await Bun.file(join(filedir, "bun.lock")).text()).not.toContain("patchedDependencies"); }); }); + +describe("patchedDependencies contents_hash", () => { + test("two distinct patches that collided under the old wyhash contents_hash do not share a cache entry", async () => { + // https://github.com/oven-sh/bun/issues/32741 + // Both patches create node_modules/is-odd/m.js with a different payload. + // Under Wyhash11(seed=0) their raw bytes hash to 0x429d7ca64c60f3d1, so + // before this change projB reused projA's cached patched package (and + // observed AAAAAAAA) instead of applying its own patch. + const header = + "diff --git a/m.js b/m.js\n" + + "new file mode 100644\n" + + "index 0000000..1111111\n" + + "--- /dev/null\n" + + "+++ b/m.js\n" + + "@@ -0,0 +1 @@\n"; + const patchA = header + `+module.exports="xxx07QaaaaaU18fmtAHABCDEFGHIJKLMNOPAAAAAAAAgMsUw5DUklmnopqrstuvwxyz";\n`; + const patchB = header + `+module.exports="xxx07QaaaaaU18fmtAHABCDEFGHIJKLMNOPBBBBBBBBgMsUw5DUklmnopqrstuvwxyz";\n`; + // Regenerating this pair at runtime would require the internal Wyhash11 + // (not exposed to JS), so the colliding pair is fixed. Both patches are + // the same length and differ only in the 8-byte payload. + expect(patchA.length).toBe(patchB.length); + expect(patchA).not.toBe(patchB); + + const mkProject = (name: string, patch: string) => + tempDirWithFiles(`patch-hash-${name}`, { + "package.json": JSON.stringify({ + name, + patchedDependencies: { "is-odd@3.0.1": "patches/p.patch" }, + dependencies: { "is-odd": "3.0.1" }, + }), + patches: { "p.patch": patch }, + }); + + const sharedCache = tempDirWithFiles("patch-hash-cache", {}); + const dirA = mkProject("proj-a", patchA); + const dirB = mkProject("proj-b", patchB); + + const install = async (cwd: string) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd, + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: String(sharedCache) }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); + }; + + await install(dirA); + expect(await Bun.file(join(dirA, "node_modules", "is-odd", "m.js")).text()).toContain("AAAAAAAA"); + + await install(dirB); + const mB = await Bun.file(join(dirB, "node_modules", "is-odd", "m.js")).text(); + expect(mB).toContain("BBBBBBBB"); + expect(mB).not.toContain("AAAAAAAA"); + + // A non-colliding control patch (different size, different content) has + // always gone to its own cache entry. + const dirC = mkProject("proj-ctl", header + `+module.exports="control payload";\n`); + await install(dirC); + expect(await Bun.file(join(dirC, "node_modules", "is-odd", "m.js")).text()).toContain("control payload"); + }); +}); diff --git a/test/js/sql/sql-statement-cache-hash-collision.test.ts b/test/js/sql/sql-statement-cache-hash-collision.test.ts new file mode 100644 index 000000000000..d9cd1dd5f609 --- /dev/null +++ b/test/js/sql/sql-statement-cache-hash-collision.test.ts @@ -0,0 +1,133 @@ +// https://github.com/oven-sh/bun/issues/32741 +// +// The per-connection prepared-statement cache was keyed on a wyhash of +// `signature.name` (= query text + one suffix per bound param), so two +// distinct queries whose names collide under that hash shared one server-side +// statement. The cache now keys on the name bytes themselves; these are the +// constructed colliding inputs that broke the old key. +import { SQL } from "bun"; +import { expect, test } from "bun:test"; +import { describeWithContainer, isDockerEnabled } from "harness"; +import { constructStdCollision } from "../../cli/install/wyhash-std-collision"; + +// signature.name = SQL text + ".null" for one null-bound param. The free +// 8-byte word sits inside a string literal, so the two queries return +// different `v` values while their names collide under std.Wyhash(0). +function collidingQueryPair(placeholder: string, freeB = "BBBBBBBB") { + // Every printable ASCII char that is valid inside a single-quoted SQL + // string literal. A wide charset lets the collision search land an in-set + // kill word in far fewer iterations, which matters under debug JSC. + let charset = ""; + for (let c = 0x20; c < 0x7f; c++) if (c !== 0x27 && c !== 0x5c) charset += String.fromCharCode(c); + const paramSuffix = ".null"; + const r = constructStdCollision({ + seed: 0n, + prefixStr: "SELECT '", + suffixStr: `' AS v, ${placeholder} AS p${paramSuffix}`, + charset, + freeA: "AAAAAAAA", + freeB, + padFillCh: "x", + }); + const sqlA = r.str1.slice(0, -paramSuffix.length); + const sqlB = r.str2.slice(0, -paramSuffix.length); + const enc = (s: string) => new TextEncoder().encode(s); + // Self-verify the pair collides under `Bun.hash.wyhash` (seed 0), the hash + // that keyed the cache before the fix; that collision is what makes this + // pair the regression input. + if (Bun.hash.wyhash(enc(r.str1), 0n) !== Bun.hash.wyhash(enc(r.str2), 0n)) { + throw new Error("constructed pair does not collide under wyhash"); + } + return { sqlA, sqlB }; +} + +async function assertDistinctStatements(sql: SQL, label: string, placeholder: string) { + const { sqlA, sqlB } = collidingQueryPair(placeholder); + expect(sqlA).not.toBe(sqlB); + expect(sqlA).toContain("AAAAAAAA"); + expect(sqlB).toContain("BBBBBBBB"); + + // Query A populates the statement cache; query B, byte-distinct but + // wyhash-colliding, must prepare its own statement and return its own value. + const [[rA], [rB], [rC]] = await Promise.all([ + sql.unsafe(sqlA, [null]), + sql.unsafe(sqlB, [null]), + sql.unsafe(`SELECT 'CONTROL' AS v, ${placeholder} AS p`, [null]), + ]); + expect({ a: rA.v.includes("AAAAAAAA"), b: rB.v.includes("BBBBBBBB"), c: rC.v }).toEqual({ + a: true, + b: true, + c: "CONTROL", + }); + // Before the fix, rB.v contained "AAAAAAAA" (A's statement was reused). + expect(rB.v).not.toContain("AAAAAAAA"); + + // Re-running A still hits its cached statement (nothing was evicted) and + // still returns A's own value. + const [rA2] = await sql.unsafe(sqlA, [null]); + expect(rA2.v).toBe(rA.v); + + // Re-running B hits B's own cached statement and still returns B's value. + const [rB2] = await sql.unsafe(sqlB, [null]); + expect(rB2.v).toBe(rB.v); + + console.log(`${label}: A="${rA.v.includes("AAAAAAAA") ? "A" : "?"}", B="${rB.v.includes("BBBBBBBB") ? "B" : "?"}"`); +} + +if (isDockerEnabled() || process.env.BUN_TEST_SERVICE_mysql_plain) { + describeWithContainer("mysql", { image: "mysql_plain" }, container => { + test("MySQL: hash-colliding prepared statements are not confused", async () => { + await container.ready; + await using sql = new SQL({ + url: `mysql://root@${container.host}:${container.port}/bun_sql_test`, + max: 1, + }); + await assertDistinctStatements(sql, "mysql", "?"); + }); + }); +} + +if (isDockerEnabled() || process.env.BUN_TEST_SERVICE_postgres_plain) { + describeWithContainer("postgres", { image: "postgres_plain" }, container => { + test("Postgres: hash-colliding prepared statements are not confused", async () => { + await container.ready; + await using sql = new SQL({ + url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, + max: 1, + }); + await assertDistinctStatements(sql, "postgres", "$1"); + }); + + test("Postgres: a hash-colliding query that fails to parse does not evict or free the cached statement", async () => { + await container.ready; + await using sql = new SQL({ + url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, + max: 1, + }); + // The bad query's 8-byte free word breaks out of the string literal, so + // Postgres rejects it at Parse; its name wyhash-collides with the good + // query's, which is the input that confused the old hash-keyed cache. + const { sqlA: good, sqlB: bad } = collidingQueryPair("$1", "'||qq9z("); + const [rGood] = await sql.unsafe(good, [null]); + expect(rGood.v).toContain("AAAAAAAA"); + + // Before the collision fix this resolved with the good query's row + // instead of erroring, because it reused the cached Prepared statement. + const err = await sql.unsafe(bad, [null]).then( + () => null, + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/syntax error|unterminated|multiple commands/i); + + Bun.gc(true); + + // The connection must survive the parse failure: the good query still + // hits its cached statement and returns its own row, and a fresh query + // still works. + const [rGood2] = await sql.unsafe(good, [null]); + const [rCtl] = await sql.unsafe(`SELECT 'CONTROL' AS v, $1 AS p`, [null]); + expect({ good: rGood2.v.includes("AAAAAAAA"), control: rCtl.v }).toEqual({ good: true, control: "CONTROL" }); + }); + }); +} From a337310123e0669af29fdab3503af4fbbad3cd02 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:19:46 +0000 Subject: [PATCH 2/5] install: hash the whole patch file, not just its first chunk read_fill_buf always reads from file offset 0, so the chunked loop re-hashed the same leading bytes until it had counted the file's size. Any two patches with an identical first chunk therefore hashed equal. Read with pread_all at an explicit running offset, 64 KiB at a time. Also moves the patch-hash tests onto tempDir, which replaced tempDirWithFiles in this file upstream. --- src/install/patch_install.rs | 21 +++---- test/cli/install/bun-install-patch.test.ts | 70 ++++++++++++++++++++-- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/install/patch_install.rs b/src/install/patch_install.rs index 6dc36b38607e..5595a8a24642 100644 --- a/src/install/patch_install.rs +++ b/src/install/patch_install.rs @@ -698,13 +698,14 @@ impl PatchTask { // Truncation keeps the u64 folder-suffix / tag-file interface. let mut hasher = bun_sha_hmac::sha::hashers::SHA1::init(); - // what's a good number for this? page size i guess - const STACK_SIZE: usize = 16384; - let mut stack = [0u8; STACK_SIZE]; - let mut read: usize = 0; - while (read as u64) < size { - let slice: &mut [u8] = match file.read_fill_buf(&mut stack[..]) { - sys::Result::Ok(slice) => slice, + // `read_fill_buf` always reads from file offset 0, so looping over it + // re-hashes the first chunk; track the file offset explicitly. + const CHUNK_SIZE: usize = 64 * 1024; + let mut chunk = vec![0u8; CHUNK_SIZE]; + let mut offset: u64 = 0; + while offset < size { + let n = match file.pread_all(&mut chunk[..], offset) { + sys::Result::Ok(n) => n, sys::Result::Err(e) => { log.add_error_fmt( None, @@ -718,11 +719,11 @@ impl PatchTask { return None; } }; - if slice.is_empty() { + if n == 0 { break; } - hasher.update(slice); - read += slice.len(); + hasher.update(&chunk[..n]); + offset += n as u64; } let mut digest = [0u8; bun_sha_hmac::sha::hashers::SHA1::DIGEST]; diff --git a/test/cli/install/bun-install-patch.test.ts b/test/cli/install/bun-install-patch.test.ts index 04bcf0ba4cd5..d8e99fc73f46 100644 --- a/test/cli/install/bun-install-patch.test.ts +++ b/test/cli/install/bun-install-patch.test.ts @@ -1047,7 +1047,7 @@ describe("patchedDependencies contents_hash", () => { expect(patchA).not.toBe(patchB); const mkProject = (name: string, patch: string) => - tempDirWithFiles(`patch-hash-${name}`, { + tempDir(`patch-hash-${name}`, { "package.json": JSON.stringify({ name, patchedDependencies: { "is-odd@3.0.1": "patches/p.patch" }, @@ -1056,9 +1056,11 @@ describe("patchedDependencies contents_hash", () => { patches: { "p.patch": patch }, }); - const sharedCache = tempDirWithFiles("patch-hash-cache", {}); - const dirA = mkProject("proj-a", patchA); - const dirB = mkProject("proj-b", patchB); + using sharedCache = tempDir("patch-hash-cache", {}); + using projA = mkProject("proj-a", patchA); + using projB = mkProject("proj-b", patchB); + const dirA = String(projA); + const dirB = String(projB); const install = async (cwd: string) => { await using proc = Bun.spawn({ @@ -1083,8 +1085,66 @@ describe("patchedDependencies contents_hash", () => { // A non-colliding control patch (different size, different content) has // always gone to its own cache entry. - const dirC = mkProject("proj-ctl", header + `+module.exports="control payload";\n`); + using projC = mkProject("proj-ctl", header + `+module.exports="control payload";\n`); + const dirC = String(projC); await install(dirC); expect(await Bun.file(join(dirC, "node_modules", "is-odd", "m.js")).text()).toContain("control payload"); }); + + test("patches that differ only after the first 64 KiB get distinct cache entries", async () => { + // The content hash used to be computed by repeatedly reading from file + // offset 0, so any two patches with an identical leading chunk hashed the + // same no matter what followed. Both patches here share a >64 KiB prefix + // (a long comment line) and differ only in the final exported payload. + const prefixLen = 80 * 1024; + const padding = "+// " + Buffer.alloc(prefixLen, "p").toString() + "\n"; + const header = + "diff --git a/m.js b/m.js\n" + + "new file mode 100644\n" + + "index 0000000..1111111\n" + + "--- /dev/null\n" + + "+++ b/m.js\n" + + "@@ -0,0 +1,2 @@\n"; + const patchA = header + padding + `+module.exports="TAIL_AAAA";\n`; + const patchB = header + padding + `+module.exports="TAIL_BBBB";\n`; + expect(patchA.length).toBe(patchB.length); + expect(patchA).not.toBe(patchB); + + const mkProject = (name: string, patch: string) => + tempDir(`patch-tail-${name}`, { + "package.json": JSON.stringify({ + name, + patchedDependencies: { "is-odd@3.0.1": "patches/p.patch" }, + dependencies: { "is-odd": "3.0.1" }, + }), + patches: { "p.patch": patch }, + }); + + using sharedCache = tempDir("patch-tail-cache", {}); + using projA = mkProject("proj-a", patchA); + using projB = mkProject("proj-b", patchB); + const dirA = String(projA); + const dirB = String(projB); + + const install = async (cwd: string) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd, + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: String(sharedCache) }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); + }; + + await install(dirA); + expect(await Bun.file(join(dirA, "node_modules", "is-odd", "m.js")).text()).toContain("TAIL_AAAA"); + + await install(dirB); + const mB = await Bun.file(join(dirB, "node_modules", "is-odd", "m.js")).text(); + // Compare just the tail so a failure doesn't dump the 80 KiB padding. + expect({ hasB: mB.includes("TAIL_BBBB"), hasA: mB.includes("TAIL_AAAA") }).toEqual({ hasB: true, hasA: false }); + }); }); From 4d5002315bbfd3617277498b8aa4f673e8145b78 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:22:13 +0000 Subject: [PATCH 3/5] install: drop explanatory comments from calc_hash --- src/install/patch_install.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/install/patch_install.rs b/src/install/patch_install.rs index 5595a8a24642..962136b25b95 100644 --- a/src/install/patch_install.rs +++ b/src/install/patch_install.rs @@ -693,13 +693,8 @@ impl PatchTask { sys::Result::Ok(f) => f, }; - // SHA-1/64 (not Wyhash11): a constructed wyhash collision would let - // two distinct patches share one `_patch_hash=` cache folder. - // Truncation keeps the u64 folder-suffix / tag-file interface. let mut hasher = bun_sha_hmac::sha::hashers::SHA1::init(); - // `read_fill_buf` always reads from file offset 0, so looping over it - // re-hashes the first chunk; track the file offset explicitly. const CHUNK_SIZE: usize = 64 * 1024; let mut chunk = vec![0u8; CHUNK_SIZE]; let mut offset: u64 = 0; From 33a90598273ce8d1e8dcc03b331f13461c2afa36 Mon Sep 17 00:00:00 2001 From: robobun Date: Tue, 11 Aug 2026 23:51:48 +0000 Subject: [PATCH 4/5] sys: remove read_fill_buf; let describeWithContainer gate the sql test calc_hash was read_fill_buf's last caller. The helper also read from offset 0 on POSIX but from the cursor on Windows, which is what made looping over it hash only the first chunk there. describeWithContainer already skips when no container source is available, checking the env override and coordinator before isDockerEnabled() (which can throw on Linux CI); the outer guard inverted that order and had no coordinator check. --- src/sys/file.rs | 19 ---- ...sql-statement-cache-hash-collision.test.ts | 94 +++++++++---------- 2 files changed, 45 insertions(+), 68 deletions(-) diff --git a/src/sys/file.rs b/src/sys/file.rs index 3665e8711ab5..42758c842443 100644 --- a/src/sys/file.rs +++ b/src/sys/file.rs @@ -227,25 +227,6 @@ impl File { } }) } - /// Reads until - /// `buf` is full or EOF; returns the filled prefix. - pub fn read_fill_buf<'b>(&self, buf: &'b mut [u8]) -> Maybe<&'b mut [u8]> { - let mut read_amount: usize = 0; - while read_amount < buf.len() { - // POSIX uses pread() from offset 0 so a pre-advanced cursor - // doesn't truncate; Windows falls back to read(). - #[cfg(unix)] - let rc = pread(self.handle, &mut buf[read_amount..], read_amount as i64); - #[cfg(not(unix))] - let rc = read(self.handle, &mut buf[read_amount..]); - match rc { - Err(err) => return Err(err), - Ok(0) => break, - Ok(n) => read_amount += n, - } - } - Ok(&mut buf[..read_amount]) - } pub fn pwrite_all(&self, mut buf: &[u8], mut off: i64) -> Maybe<()> { while !buf.is_empty() { let n = pwrite(self.handle, buf, off)?; diff --git a/test/js/sql/sql-statement-cache-hash-collision.test.ts b/test/js/sql/sql-statement-cache-hash-collision.test.ts index d9cd1dd5f609..34d6f677a829 100644 --- a/test/js/sql/sql-statement-cache-hash-collision.test.ts +++ b/test/js/sql/sql-statement-cache-hash-collision.test.ts @@ -7,7 +7,7 @@ // constructed colliding inputs that broke the old key. import { SQL } from "bun"; import { expect, test } from "bun:test"; -import { describeWithContainer, isDockerEnabled } from "harness"; +import { describeWithContainer } from "harness"; import { constructStdCollision } from "../../cli/install/wyhash-std-collision"; // signature.name = SQL text + ".null" for one null-bound param. The free @@ -74,60 +74,56 @@ async function assertDistinctStatements(sql: SQL, label: string, placeholder: st console.log(`${label}: A="${rA.v.includes("AAAAAAAA") ? "A" : "?"}", B="${rB.v.includes("BBBBBBBB") ? "B" : "?"}"`); } -if (isDockerEnabled() || process.env.BUN_TEST_SERVICE_mysql_plain) { - describeWithContainer("mysql", { image: "mysql_plain" }, container => { - test("MySQL: hash-colliding prepared statements are not confused", async () => { - await container.ready; - await using sql = new SQL({ - url: `mysql://root@${container.host}:${container.port}/bun_sql_test`, - max: 1, - }); - await assertDistinctStatements(sql, "mysql", "?"); +describeWithContainer("mysql", { image: "mysql_plain" }, container => { + test("MySQL: hash-colliding prepared statements are not confused", async () => { + await container.ready; + await using sql = new SQL({ + url: `mysql://root@${container.host}:${container.port}/bun_sql_test`, + max: 1, }); + await assertDistinctStatements(sql, "mysql", "?"); }); -} +}); -if (isDockerEnabled() || process.env.BUN_TEST_SERVICE_postgres_plain) { - describeWithContainer("postgres", { image: "postgres_plain" }, container => { - test("Postgres: hash-colliding prepared statements are not confused", async () => { - await container.ready; - await using sql = new SQL({ - url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, - max: 1, - }); - await assertDistinctStatements(sql, "postgres", "$1"); +describeWithContainer("postgres", { image: "postgres_plain" }, container => { + test("Postgres: hash-colliding prepared statements are not confused", async () => { + await container.ready; + await using sql = new SQL({ + url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, + max: 1, }); + await assertDistinctStatements(sql, "postgres", "$1"); + }); - test("Postgres: a hash-colliding query that fails to parse does not evict or free the cached statement", async () => { - await container.ready; - await using sql = new SQL({ - url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, - max: 1, - }); - // The bad query's 8-byte free word breaks out of the string literal, so - // Postgres rejects it at Parse; its name wyhash-collides with the good - // query's, which is the input that confused the old hash-keyed cache. - const { sqlA: good, sqlB: bad } = collidingQueryPair("$1", "'||qq9z("); - const [rGood] = await sql.unsafe(good, [null]); - expect(rGood.v).toContain("AAAAAAAA"); + test("Postgres: a hash-colliding query that fails to parse does not evict or free the cached statement", async () => { + await container.ready; + await using sql = new SQL({ + url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, + max: 1, + }); + // The bad query's 8-byte free word breaks out of the string literal, so + // Postgres rejects it at Parse; its name wyhash-collides with the good + // query's, which is the input that confused the old hash-keyed cache. + const { sqlA: good, sqlB: bad } = collidingQueryPair("$1", "'||qq9z("); + const [rGood] = await sql.unsafe(good, [null]); + expect(rGood.v).toContain("AAAAAAAA"); - // Before the collision fix this resolved with the good query's row - // instead of erroring, because it reused the cached Prepared statement. - const err = await sql.unsafe(bad, [null]).then( - () => null, - (e: unknown) => e, - ); - expect(err).toBeInstanceOf(Error); - expect((err as Error).message).toMatch(/syntax error|unterminated|multiple commands/i); + // Before the collision fix this resolved with the good query's row + // instead of erroring, because it reused the cached Prepared statement. + const err = await sql.unsafe(bad, [null]).then( + () => null, + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/syntax error|unterminated|multiple commands/i); - Bun.gc(true); + Bun.gc(true); - // The connection must survive the parse failure: the good query still - // hits its cached statement and returns its own row, and a fresh query - // still works. - const [rGood2] = await sql.unsafe(good, [null]); - const [rCtl] = await sql.unsafe(`SELECT 'CONTROL' AS v, $1 AS p`, [null]); - expect({ good: rGood2.v.includes("AAAAAAAA"), control: rCtl.v }).toEqual({ good: true, control: "CONTROL" }); - }); + // The connection must survive the parse failure: the good query still + // hits its cached statement and returns its own row, and a fresh query + // still works. + const [rGood2] = await sql.unsafe(good, [null]); + const [rCtl] = await sql.unsafe(`SELECT 'CONTROL' AS v, $1 AS p`, [null]); + expect({ good: rGood2.v.includes("AAAAAAAA"), control: rCtl.v }).toEqual({ good: true, control: "CONTROL" }); }); -} +}); From 05b6dc36dafc030ee165c9d3ac224261966183d8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:04:51 +0000 Subject: [PATCH 5/5] test: share the patch-hash test helpers --- test/cli/install/bun-install-patch.test.ts | 129 ++++++++------------- 1 file changed, 50 insertions(+), 79 deletions(-) diff --git a/test/cli/install/bun-install-patch.test.ts b/test/cli/install/bun-install-patch.test.ts index d8e99fc73f46..663a31dd7eb3 100644 --- a/test/cli/install/bun-install-patch.test.ts +++ b/test/cli/install/bun-install-patch.test.ts @@ -1025,19 +1025,46 @@ index 0000000000000000000000000000000000000000..2f9a147b6e5d17254f1bfce0d4e109a2 }); describe("patchedDependencies contents_hash", () => { + // A patch that creates node_modules/is-odd/m.js; `hunk` is the @@ line. + const patchHeader = (hunk: string) => + "diff --git a/m.js b/m.js\n" + + "new file mode 100644\n" + + "index 0000000..1111111\n" + + "--- /dev/null\n" + + "+++ b/m.js\n" + + `${hunk}\n`; + + const mkProject = (name: string, patch: string) => + tempDir(`patch-hash-${name}`, { + "package.json": JSON.stringify({ + name, + patchedDependencies: { "is-odd@3.0.1": "patches/p.patch" }, + dependencies: { "is-odd": "3.0.1" }, + }), + patches: { "p.patch": patch }, + }); + + const install = async (cwd: string, cacheDir: string) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd, + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: cacheDir }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); + }; + + const installedMjs = (dir: string) => Bun.file(join(dir, "node_modules", "is-odd", "m.js")).text(); + test("two distinct patches that collided under the old wyhash contents_hash do not share a cache entry", async () => { // https://github.com/oven-sh/bun/issues/32741 - // Both patches create node_modules/is-odd/m.js with a different payload. - // Under Wyhash11(seed=0) their raw bytes hash to 0x429d7ca64c60f3d1, so + // Under Wyhash11(seed=0) both patches hash to 0x429d7ca64c60f3d1, so // before this change projB reused projA's cached patched package (and // observed AAAAAAAA) instead of applying its own patch. - const header = - "diff --git a/m.js b/m.js\n" + - "new file mode 100644\n" + - "index 0000000..1111111\n" + - "--- /dev/null\n" + - "+++ b/m.js\n" + - "@@ -0,0 +1 @@\n"; + const header = patchHeader("@@ -0,0 +1 @@"); const patchA = header + `+module.exports="xxx07QaaaaaU18fmtAHABCDEFGHIJKLMNOPAAAAAAAAgMsUw5DUklmnopqrstuvwxyz";\n`; const patchB = header + `+module.exports="xxx07QaaaaaU18fmtAHABCDEFGHIJKLMNOPBBBBBBBBgMsUw5DUklmnopqrstuvwxyz";\n`; // Regenerating this pair at runtime would require the internal Wyhash11 @@ -1046,49 +1073,24 @@ describe("patchedDependencies contents_hash", () => { expect(patchA.length).toBe(patchB.length); expect(patchA).not.toBe(patchB); - const mkProject = (name: string, patch: string) => - tempDir(`patch-hash-${name}`, { - "package.json": JSON.stringify({ - name, - patchedDependencies: { "is-odd@3.0.1": "patches/p.patch" }, - dependencies: { "is-odd": "3.0.1" }, - }), - patches: { "p.patch": patch }, - }); - using sharedCache = tempDir("patch-hash-cache", {}); using projA = mkProject("proj-a", patchA); using projB = mkProject("proj-b", patchB); - const dirA = String(projA); - const dirB = String(projB); - - const install = async (cwd: string) => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd, - env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: String(sharedCache) }, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("error:"); - expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); - }; + const cache = String(sharedCache); - await install(dirA); - expect(await Bun.file(join(dirA, "node_modules", "is-odd", "m.js")).text()).toContain("AAAAAAAA"); + await install(String(projA), cache); + expect(await installedMjs(String(projA))).toContain("AAAAAAAA"); - await install(dirB); - const mB = await Bun.file(join(dirB, "node_modules", "is-odd", "m.js")).text(); + await install(String(projB), cache); + const mB = await installedMjs(String(projB)); expect(mB).toContain("BBBBBBBB"); expect(mB).not.toContain("AAAAAAAA"); // A non-colliding control patch (different size, different content) has // always gone to its own cache entry. using projC = mkProject("proj-ctl", header + `+module.exports="control payload";\n`); - const dirC = String(projC); - await install(dirC); - expect(await Bun.file(join(dirC, "node_modules", "is-odd", "m.js")).text()).toContain("control payload"); + await install(String(projC), cache); + expect(await installedMjs(String(projC))).toContain("control payload"); }); test("patches that differ only after the first 64 KiB get distinct cache entries", async () => { @@ -1096,54 +1098,23 @@ describe("patchedDependencies contents_hash", () => { // offset 0, so any two patches with an identical leading chunk hashed the // same no matter what followed. Both patches here share a >64 KiB prefix // (a long comment line) and differ only in the final exported payload. - const prefixLen = 80 * 1024; - const padding = "+// " + Buffer.alloc(prefixLen, "p").toString() + "\n"; - const header = - "diff --git a/m.js b/m.js\n" + - "new file mode 100644\n" + - "index 0000000..1111111\n" + - "--- /dev/null\n" + - "+++ b/m.js\n" + - "@@ -0,0 +1,2 @@\n"; + const padding = "+// " + Buffer.alloc(80 * 1024, "p").toString() + "\n"; + const header = patchHeader("@@ -0,0 +1,2 @@"); const patchA = header + padding + `+module.exports="TAIL_AAAA";\n`; const patchB = header + padding + `+module.exports="TAIL_BBBB";\n`; expect(patchA.length).toBe(patchB.length); expect(patchA).not.toBe(patchB); - const mkProject = (name: string, patch: string) => - tempDir(`patch-tail-${name}`, { - "package.json": JSON.stringify({ - name, - patchedDependencies: { "is-odd@3.0.1": "patches/p.patch" }, - dependencies: { "is-odd": "3.0.1" }, - }), - patches: { "p.patch": patch }, - }); - using sharedCache = tempDir("patch-tail-cache", {}); using projA = mkProject("proj-a", patchA); using projB = mkProject("proj-b", patchB); - const dirA = String(projA); - const dirB = String(projB); - - const install = async (cwd: string) => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd, - env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: String(sharedCache) }, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("error:"); - expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); - }; + const cache = String(sharedCache); - await install(dirA); - expect(await Bun.file(join(dirA, "node_modules", "is-odd", "m.js")).text()).toContain("TAIL_AAAA"); + await install(String(projA), cache); + expect(await installedMjs(String(projA))).toContain("TAIL_AAAA"); - await install(dirB); - const mB = await Bun.file(join(dirB, "node_modules", "is-odd", "m.js")).text(); + await install(String(projB), cache); + const mB = await installedMjs(String(projB)); // Compare just the tail so a failure doesn't dump the 80 KiB padding. expect({ hasB: mB.includes("TAIL_BBBB"), hasA: mB.includes("TAIL_AAAA") }).toEqual({ hasB: true, hasA: false }); });