From b205e7c628c327a10f778a8d6db217c63da96e97 Mon Sep 17 00:00:00 2001 From: adenzhou1350 Date: Fri, 23 Oct 2026 03:14:17 +0800 Subject: [PATCH] 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 Co-authored-by: aden --- nodejs/__test__/connection.test.ts | 50 ++++++++++++++++++++++++++++++ nodejs/lancedb/connection.ts | 11 +++++-- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/nodejs/__test__/connection.test.ts b/nodejs/__test__/connection.test.ts index e32e7e626..bbd0c3c52 100644 --- a/nodejs/__test__/connection.test.ts +++ b/nodejs/__test__/connection.test.ts @@ -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 }, diff --git a/nodejs/lancedb/connection.ts b/nodejs/lancedb/connection.ts index 44fde96f1..c93414fab 100644 --- a/nodejs/lancedb/connection.ts +++ b/nodejs/lancedb/connection.ts @@ -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. */