From 735a7ce6feaa44dc1c91ccb2861abcd228d03f4f Mon Sep 17 00:00:00 2001 From: Brendan Clement Date: Wed, 3 Jun 2026 09:57:45 -0700 Subject: [PATCH] fix: validate branch inputs (empty names, negative versions) --- nodejs/__test__/table.test.ts | 10 ++++++ nodejs/src/table.rs | 11 ++++++- python/python/tests/test_table.py | 12 ++++++++ rust/lancedb/src/table.rs | 51 +++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/nodejs/__test__/table.test.ts b/nodejs/__test__/table.test.ts index a066a100f..6364382c5 100644 --- a/nodejs/__test__/table.test.ts +++ b/nodejs/__test__/table.test.ts @@ -133,6 +133,16 @@ describe.each([arrow15, arrow16, arrow17, arrow18])( expect(await (await db.openTable("some_table")).countRows()).toBe(1); }); + it("rejects invalid branch inputs", async () => { + const branches = await table.branches(); + await expect(branches.create("")).rejects.toThrow("non-empty"); + await expect(branches.checkout("")).rejects.toThrow("non-empty"); + await expect(branches.delete("")).rejects.toThrow("non-empty"); + await expect(branches.create("bad", "main", -1)).rejects.toThrow( + "non-negative", + ); + }); + it("should show table stats", async () => { await table.add([{ id: 1 }, { id: 2 }]); await table.add([{ id: 1 }]); diff --git a/nodejs/src/table.rs b/nodejs/src/table.rs index dc8f73e22..acbe45aeb 100644 --- a/nodejs/src/table.rs +++ b/nodejs/src/table.rs @@ -1179,7 +1179,16 @@ impl Branches { // "main" and None are two spellings of the root branch; normalize so // from_ref = "main" behaves identically to the default. let from_ref = from_ref.filter(|b| b != "main"); - let from = Ref::Version(from_ref, from_version.map(|v| v as u64)); + // Reject a negative version up front; `as u64` would silently wrap it + // into a huge version number. + let from_version = from_version + .map(|v| { + u64::try_from(v).map_err(|_| { + napi::Error::from_reason("from_version must be a non-negative integer") + }) + }) + .transpose()?; + let from = Ref::Version(from_ref, from_version); let table = self .inner .create_branch(&name, from) diff --git a/python/python/tests/test_table.py b/python/python/tests/test_table.py index b3d0f11c7..2dd550566 100644 --- a/python/python/tests/test_table.py +++ b/python/python/tests/test_table.py @@ -942,6 +942,18 @@ def test_branches(tmp_path): assert "exp" not in table.branches.list() +def test_branch_name_validation(tmp_path): + db = lancedb.connect(tmp_path) + table = db.create_table("t", [{"id": 1}]) + + with pytest.raises(ValueError, match="non-empty"): + table.branches.create("") + with pytest.raises(ValueError, match="non-empty"): + table.branches.checkout("") + with pytest.raises(ValueError, match="non-empty"): + table.branches.delete("") + + def test_branches_preserve_namespace(tmp_path): pytest.importorskip( "lance" diff --git a/rust/lancedb/src/table.rs b/rust/lancedb/src/table.rs index c7a584203..e93ef4e6c 100644 --- a/rust/lancedb/src/table.rs +++ b/rust/lancedb/src/table.rs @@ -1930,6 +1930,15 @@ impl NativeTable { } } + fn validate_branch_name(name: &str, field: &str) -> Result<()> { + if name.is_empty() { + return Err(Error::InvalidInput { + message: format!("{field} must be a non-empty string"), + }); + } + Ok(()) + } + /// Opens an existing Table using a namespace client. /// /// This method uses `DatasetBuilder::from_namespace` to open the table, which @@ -2726,6 +2735,10 @@ impl BaseTable for NativeTable { name: &str, from: lance::dataset::refs::Ref, ) -> Result> { + Self::validate_branch_name(name, "branch name")?; + if let lance::dataset::refs::Ref::Version(Some(from_branch), _) = &from { + Self::validate_branch_name(from_branch, "from_ref")?; + } let mut ds = (*self.dataset.get().await?).clone(); let branch_ds = ds.create_branch(name, from, None).await?; let dataset = dataset::DatasetConsistencyWrapper::new_latest( @@ -2736,6 +2749,7 @@ impl BaseTable for NativeTable { } async fn checkout_branch(&self, name: &str) -> Result> { + Self::validate_branch_name(name, "branch name")?; let branch_ds = self.dataset.get().await?.checkout_branch(name).await?; let dataset = dataset::DatasetConsistencyWrapper::new_latest( branch_ds, @@ -2749,6 +2763,7 @@ impl BaseTable for NativeTable { } async fn delete_branch(&self, name: &str) -> Result<()> { + Self::validate_branch_name(name, "branch name")?; let mut ds = (*self.dataset.get().await?).clone(); ds.delete_branch(name).await?; Ok(()) @@ -3544,6 +3559,42 @@ mod tests { assert!(!branches.contains_key("exp")); } + #[tokio::test] + async fn test_branch_name_validation() { + let tmp_dir = tempdir().unwrap(); + let uri = tmp_dir.path().to_str().unwrap(); + let conn = ConnectBuilder::new(uri).execute().await.unwrap(); + let table = conn + .create_table("my_table", some_sample_data()) + .execute() + .await + .unwrap(); + + // every entry point rejects an empty name instead of passing it down + assert!(matches!( + table.create_branch("", 1u64).await, + Err(Error::InvalidInput { .. }) + )); + assert!(matches!( + table.checkout_branch("").await, + Err(Error::InvalidInput { .. }) + )); + assert!(matches!( + table.delete_branch("").await, + Err(Error::InvalidInput { .. }) + )); + // an empty source branch is rejected too + assert!(matches!( + table + .create_branch( + "ok", + lance::dataset::refs::Ref::Version(Some(String::new()), None) + ) + .await, + Err(Error::InvalidInput { .. }) + )); + } + #[tokio::test] async fn test_create_index() { use arrow_array::RecordBatch;