Repository navigation
fix: validate sortable timestamp range and prevent state poisoning - #57
jdymitarai wants to merge 2 commits into
Conversation
Validate candidate timestamps against SORTABLE_TIME_MAX and defer generator state updates until validation passes, preventing out-of-range IDs and generator poisoning from transient clock glitches. Fixes suhailopensource#55 Signed-off-by: jdymitarai <o10040115@gmail.com>
|
Thanks @jdymitarai, the fix looks good! One change please: remove the two Resetting makes the generator forget its last ID, so new IDs can come out in the wrong order. Also, the issue was wrong about "recovering" after 2^48−1, sorry. It should keep throwing there. Please replace the "recovers on subsequent calls" test with these two: it("keeps throwing after spilling past 2^48 - 1", () => {
let now = SORTABLE_TIME_MAX;
const gen = createSortableId({
now: () => now,
alphabet: "01",
randomSize: 1,
});
expect(() => {
for (let i = 0; i < 3; i++) gen("evt");
}).toThrow(RangeError);
now = Date.now();
expect(() => gen("evt")).toThrow(RangeError);
const fresh = createSortableId({
now: () => now,
alphabet: "01",
randomSize: 1,
});
expect(() => fresh("evt")).not.toThrow();
});
it("keeps ids in order after a bad clock reading", () => {
let now = 2_000;
const gen = createSortableId({ now: () => now });
const a = gen("evt");
now = 1_000;
const b = gen("evt");
now = 2 ** 48 + 5;
expect(() => gen("evt")).toThrow(RangeError);
now = 1_000;
const c = gen("evt");
expect(b > a).toBe(true);
expect(c > b).toBe(true);
});Both fail now and pass after the change. Thanks! |
|
Updated as requested: removed the \lastTime/\lastRandom\ resets and the \ ry/catch\ around \encodeTime\ so bad timestamps leave existing generator state untouched, and replaced the spill test with the two new test cases. All 163 tests, typechecks, and package verifications pass cleanly. |
The sortable generator previously allowed timestamps exceeding SORTABLE_TIME_MAX to be encoded because it only checked whether the numeric value fit within the configured character width, leading to IDs that getTimestamp subsequently rejected as undefined. In addition, lastTime and lastRandom state were updated prior to validation, causing any transient out-of-range clock reading or monotonic spill-over to permanently poison subsequent ID generation in long-running processes. This change computes candidate timestamp and random values first, validates that the timestamp does not exceed SORTABLE_TIME_MAX and fits character width, throws a RangeError on invalid values, and defers saving generator state until validation passes. All local tests, typechecks, packaging audits, and smoke tests pass cleanly.
Fixes #55