install: hash patch contents with SHA-1 instead of Wyhash11

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,15 +693,14 @@ impl PatchTask {
             sys::Result::Ok(f) => f,
         };
 
-        let mut hasher = Wyhash11::init(0);
+        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,
+        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,
@@ -716,14 +714,16 @@ 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;
         }
 
-        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) {

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)?;

test/cli/install/bun-install-patch.test.ts

@@ -1023,3 +1023,99 @@ index 0000000000000000000000000000000000000000..2f9a147b6e5d17254f1bfce0d4e109a2
     expect(await Bun.file(join(filedir, "bun.lock")).text()).not.toContain("patchedDependencies");
   });
 });
+
+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
+    // 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 = 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
+    // (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);
+
+    using sharedCache = tempDir("patch-hash-cache", {});
+    using projA = mkProject("proj-a", patchA);
+    using projB = mkProject("proj-b", patchB);
+    const cache = String(sharedCache);
+
+    await install(String(projA), cache);
+    expect(await installedMjs(String(projA))).toContain("AAAAAAAA");
+
+    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`);
+    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 () => {
+    // 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 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);
+
+    using sharedCache = tempDir("patch-tail-cache", {});
+    using projA = mkProject("proj-a", patchA);
+    using projB = mkProject("proj-b", patchB);
+    const cache = String(sharedCache);
+
+    await install(String(projA), cache);
+    expect(await installedMjs(String(projA))).toContain("TAIL_AAAA");
+
+    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 });
+  });
+});

test/js/sql/sql-statement-cache-hash-collision.test.ts

@@ -0,0 +1,129 @@
+// 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 } 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" : "?"}"`);
+}
+
+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("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" });
+  });
+});