mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-23 20:22:37 +00:00
fix(node): prevent table listing limits from wrapping (#4492)
## Summary Prevent large JavaScript table-listing limits from wrapping during the native `u32` conversion. With two tables, `limit: 2 ** 32` currently returns no tables and `2 ** 32 + 1` returns only one. Validate nonnegative safe integers using the existing helper and clamp valid limits above `u32::MAX` before calling the native binding. Apply the same normalization to `tableNames` and `listTables`, including namespace overloads. Undefined, zero, small limits and `u32::MAX` keep their existing behavior. ## Validation Built the real Node native extension from main `0aa842b17f13d1f8ffd2851bc57c222bc5a376ba` with the repository's `pnpm build`, using locked dependencies and reusable build caches. No mocked/released native extension or disabled TypeScript diagnostics. With identical regression tests and that native library in fresh Jest processes: - Baseline: 40 passed, 8 failed; candidate: **48 passed**, no failures. - Both arms: `pnpm run tsc` and `pnpm run docs` passed. - Changed-file Biome check passed. Tests cover wrapping limits, invalid numbers, zero/small/u32 limits, defaults and both overload forms. Validation is the focused connection suite, not the complete repository suite. No performance claim. AI-assisted implementation and verification; the patch and exact tested source bytes were reviewed before publication. Signed-off-by: aden <aden@example.com> Co-authored-by: aden <aden@example.com>
This commit is contained in:
@@ -336,6 +336,56 @@ describe("given a connection", () => {
|
||||
expect(seen).toEqual(created);
|
||||
});
|
||||
|
||||
it.each([2 ** 32, 2 ** 32 + 1, Number.MAX_SAFE_INTEGER])(
|
||||
"should not wrap a large table listing limit %s",
|
||||
async (limit) => {
|
||||
await db.createTable("a", [{ id: 1 }]);
|
||||
await db.createTable("b", [{ id: 2 }]);
|
||||
for (const names of [
|
||||
db.tableNames({ limit }),
|
||||
db.tableNames([], { limit }),
|
||||
]) {
|
||||
await expect(names).resolves.toEqual(["a", "b"]);
|
||||
}
|
||||
for (const page of [
|
||||
db.listTables({ limit }),
|
||||
db.listTables([], { limit }),
|
||||
]) {
|
||||
await expect(page).resolves.toEqual({ tables: ["a", "b"] });
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
-1,
|
||||
1.5,
|
||||
Number.NaN,
|
||||
Number.POSITIVE_INFINITY,
|
||||
Number.NEGATIVE_INFINITY,
|
||||
Number.MAX_SAFE_INTEGER + 1,
|
||||
])("should reject an invalid table listing limit %s", async (limit) => {
|
||||
await expect(db.tableNames({ limit })).rejects.toThrow(/limit/);
|
||||
await expect(db.tableNames([], { limit })).rejects.toThrow(/limit/);
|
||||
await expect(db.listTables({ limit })).rejects.toThrow(/limit/);
|
||||
await expect(db.listTables([], { limit })).rejects.toThrow(/limit/);
|
||||
});
|
||||
|
||||
it("should preserve zero, small and unsigned-32-bit table listing limits", async () => {
|
||||
await db.createTable("a", [{ id: 1 }]);
|
||||
await db.createTable("b", [{ id: 2 }]);
|
||||
for (const [limit, names] of [
|
||||
[0, []],
|
||||
[1, ["a"]],
|
||||
[2 ** 32 - 1, ["a", "b"]],
|
||||
] as const) {
|
||||
await expect(db.tableNames({ limit })).resolves.toEqual(names);
|
||||
const page = await db.listTables({ limit });
|
||||
expect(page.tables).toEqual(names);
|
||||
}
|
||||
await expect(db.tableNames()).resolves.toEqual(["a", "b"]);
|
||||
await expect(db.listTables()).resolves.toEqual({ tables: ["a", "b"] });
|
||||
});
|
||||
|
||||
it("should list all names and use a default page size of 100", async () => {
|
||||
const names = Array.from(
|
||||
{ length: 130 },
|
||||
|
||||
@@ -726,7 +726,7 @@ export class LocalConnection extends Connection {
|
||||
return this.inner.tableNames(
|
||||
namespacePath ?? [],
|
||||
tableNamesOptions?.startAfter,
|
||||
tableNamesOptions?.limit,
|
||||
normalizeTableListingLimit(tableNamesOptions?.limit),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -836,7 +836,7 @@ export class LocalConnection extends Connection {
|
||||
return this.inner.listTables(
|
||||
namespacePath ?? [],
|
||||
listTablesOptions?.pageToken,
|
||||
listTablesOptions?.limit,
|
||||
normalizeTableListingLimit(listTablesOptions?.limit),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1107,6 +1107,13 @@ export class LocalConnection extends Connection {
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeTableListingLimit(limit?: number): number | undefined {
|
||||
validateNonNegativeInteger(limit, "limit");
|
||||
// The native binding takes u32: larger JS numbers otherwise wrap to zero or
|
||||
// a smaller page size. A listing limit is an upper bound, so saturate it.
|
||||
return limit === undefined ? undefined : Math.min(limit, 0xffffffff);
|
||||
}
|
||||
|
||||
/**
|
||||
* Takes storage options and makes all the keys snake case.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user