rust/lancedb
1
0
Fork 0
mirror of https://github.com/lancedb/lancedb.git synced 2026-11-13 22:26:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: check table, namespace and Function identifiers the same way

Browse Source 
Extends the Secret component rules to the other two objects that join a name
into a route, because the failures are the same and were reachable from the
public Rust API for all three.

  drop_table("../jobs", &[])       -> /v1/jobs/drop/
  drop_table("a$b", &[])           -> /v1/table/a$b/drop/
  drop_table("b", &["a"])          -> /v1/table/a$b/drop/   the same URL

The first leaves the table route entirely. The second and third are one URL
for two identities -- and `a$b` is not a legal table name, so what the caller
asked for silently becomes a different table that may exist. Neither is an
error the service could return: it is asked about the wrong object, or not
asked at all.

The dot-segment rule moves into `validate_table_name` and
`validate_namespace_name`, so the local path in `database/listing.rs` and the
service -- which imports both -- inherit it with the client. `.` and `..` pass
every character test and still cannot address anything: they are resolved as
relative path segments, and after percent-decoding, so `%2E%2E` is resolved
exactly as `..` is.

`build_table_identifier` and `build_namespace_identifier` become fallible and
validate each component before joining. That covers every remote table route
from one place rather than each call site. Both also refuse a component holding
the configured delimiter, which is the rule that cannot live in a shared
validator: the default `$` is outside every charset here, but a configured
delimiter may be any string and the readable choices -- `.`, `-`, `_` -- are
all inside them.

Functions need only the dot-segment rule. Their identifier is percent-encoded,
which covers the rest, and they have no namespace to join -- so there is no
boundary a delimiter could erase. `..` is unreserved, so it survives encoding
untouched and would post a registration body to `/v1/create`.

One test changed rather than being added to. `test_table_names_with_nested_namespace`
had its mock return `ns1$ns2$table1` from a namespace listing; the service
returns bare names there, as the sibling test and the service's own tests both
show. The client joins the namespace back on to build each table's identifier,
so the joined form was being joined twice into `ns1$ns2$ns1$ns2$table1`. Nothing
caught it because nothing validated the result.

Tests: a dot-only or delimiter-bearing table name, namespace segment and
Function name are each refused before a request is built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
Jonathan M Hsieh
2026-09-15 19:33:15 +00:00
co-authored by Claude Opus 5
parent 7898699208
commit 912195cee2
2 changed files with 225 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
rust/lancedb/src/remote/db.rs
+198 -35
View File
@@ -34,6 +34,7 @@ use crate::remote::job::{RemoteJob, job_state_to_client};
use crate::remote::util::stream_as_body;
use crate::secrets::SecretInfo;
use crate::table::BaseTable;
use crate::utils::{validate_namespace_name, validate_table_name};
use super::client::{
ClientConfig, HeaderProvider, HttpSend, RequestResultExt, RestfulLanceDbClient, Sender,
@@ -452,7 +453,7 @@ impl<S: HttpSend> RemoteDatabase<S> {
name: &str,
namespace_path: &[String],
) -> Result<(String, Response)> {
let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter);
let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter)?;
let cache_key = build_cache_key(name, namespace_path);
let req = self.client.post(&format!("/v1/table/{}/drop/", identifier));
let (request_id, resp) = self.client.send(req).await?;
@@ -477,7 +478,7 @@ impl<S: HttpSend> RemoteDatabase<S> {
request: &TableNamesRequest,
) -> Result<(Vec<String>, ServerVersion)> {
let namespace_id =
build_namespace_identifier(&request.namespace_path, &self.client.id_delimiter);
build_namespace_identifier(&request.namespace_path, &self.client.id_delimiter)?;
let path = format!("/v1/namespace/{}/table/list", namespace_id);
let mut names = Vec::new();
@@ -584,14 +585,75 @@ impl From<&CreateTableMode> for &'static str {
}
}
fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) -> String {
if !namespace.is_empty() {
let mut parts = namespace.to_vec();
parts.push(name.to_string());
parts.join(delimiter)
} else {
name.to_string()
fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) -> Result<String> {
for segment in namespace {
validate_namespace_name(segment)?;
reject_delimiter(segment, delimiter, "namespace segment")?;
}
validate_table_name(name)?;
reject_delimiter(name, delimiter, "table name")?;
if namespace.is_empty() {
return Ok(name.to_string());
}
let mut parts = namespace.to_vec();
parts.push(name.to_string());
Ok(parts.join(delimiter))
}
/// The path segment addressing one Function.
///
/// A Function has no namespace yet, so there is no join to reverse and nothing
/// a delimiter could split wrongly. What remains is the part encoding cannot
/// fix: `..` is unreserved, so it survives percent-encoding untouched and is
/// then resolved as a relative path segment -- `create_function` for a Function
/// so named would post its registration body to `/v1/create`.
fn build_function_identifier(name: &str) -> Result<String> {
reject_dot_only_component("Function name", name)?;
Ok(urlencoding::encode(name).into_owned())
}
/// The dot-segment rule, for identifiers that have no other validator to hang
/// it on.
///
/// Table and namespace names reach this through [`validate_table_name`] and
/// [`validate_namespace_name`], which own the rest of their grammar. A Function
/// name has no client-side grammar, so only this part applies.
fn reject_dot_only_component(what: &str, value: &str) -> Result<()> {
if !value.is_empty() && value.chars().all(|character| character == '.') {
return Err(Error::InvalidInput {
message: format!(
"invalid {what} '{value}': a component of only periods is read as a relative \
path and cannot address an object"
),
});
}
Ok(())
}
/// Refuse a component holding the delimiter that is about to separate it from
/// the others.
///
/// The join is what decides identity, and it is only reversible while no
/// component contains the separator. The default `$` is outside every name
/// charset here, so this never fires for it. A *configured* delimiter may be
/// any string, and the readable choices -- `.`, `-`, `_` -- are all inside
/// those charsets: under `id_delimiter="."` the namespace `["prod.vision"]`
/// and the namespace `["prod", "vision"]` join to one identifier, and a caller
/// addressing the first would reach the object belonging to the second.
///
/// There is no escaping of the joined form that recovers a boundary the
/// delimiter has already erased, so this refuses rather than encodes.
fn reject_delimiter(component: &str, delimiter: &str, what: &str) -> Result<()> {
if !delimiter.is_empty() && component.contains(delimiter) {
return Err(Error::InvalidInput {
message: format!(
"{what} '{component}' contains the configured identifier delimiter \
'{delimiter}', so the namespace path and the name it joins could not be told \
apart"
),
});
}
Ok(())
}
/// A Secret's path identifier: its namespace path and name, each percent-encoded,
@@ -656,13 +718,16 @@ fn build_secret_namespace_identifier(namespace: &[String], delimiter: &str) -> S
.join(delimiter)
}
fn build_namespace_identifier(namespace: &[String], delimiter: &str) -> String {
fn build_namespace_identifier(namespace: &[String], delimiter: &str) -> Result<String> {
for segment in namespace {
validate_namespace_name(segment)?;
reject_delimiter(segment, delimiter, "namespace segment")?;
}
if namespace.is_empty() {
// According to the namespace spec, use delimiter to represent root namespace
delimiter.to_string()
} else {
namespace.join(delimiter)
return Ok(delimiter.to_string());
}
Ok(namespace.join(delimiter))
}
/// Build a secure cache key using length prefixes.
@@ -803,7 +868,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&request.name,
&request.namespace_path,
&self.client.id_delimiter,
);
)?;
let req = self
.client
.post(&format!("/v1/materialized_view/{identifier}/create"))
@@ -846,7 +911,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
name: &str,
namespace_path: &[String],
) -> Result<Job> {
let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter);
let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter)?;
let request = self
.client
.post(&format!("/v1/materialized_view/{identifier}/drop"));
@@ -884,7 +949,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
page_token: Option<String>,
}
let namespace_id = build_namespace_identifier(namespace_path, &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(namespace_path, &self.client.id_delimiter)?;
let path = format!("/v1/namespace/{namespace_id}/materialized_view/list");
let mut views = Vec::new();
let mut page_token: Option<String> = None;
@@ -920,7 +985,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&self,
request: FunctionRegistrationRequest,
) -> Result<Job<FunctionVersion>> {
let function_id = urlencoding::encode(&request.name);
let function_id = build_function_identifier(&request.name)?;
let req = self
.client
.post(&format!("/v1/function/{function_id}/create"))
@@ -946,7 +1011,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
}
async fn get_function(&self, name: &str, version: &str) -> Result<FunctionVersion> {
let function_id = urlencoding::encode(name);
let function_id = build_function_identifier(name)?;
let req = self
.client
.post(&format!("/v1/function/{function_id}/describe"))
@@ -959,7 +1024,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
}
async fn list_functions(&self) -> Result<Vec<FunctionVersion>> {
let namespace_id = build_namespace_identifier(&[], &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(&[], &self.client.id_delimiter)?;
let path = format!("/v1/namespace/{namespace_id}/function/list");
let mut functions = Vec::new();
let mut page_token: Option<String> = None;
@@ -1000,7 +1065,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
}
async fn drop_function(&self, name: &str, version: &str) -> Result<bool> {
let function_id = urlencoding::encode(name);
let function_id = build_function_identifier(name)?;
let req = self
.client
.post(&format!("/v1/function/{function_id}/drop"))
@@ -1210,7 +1275,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
for table in &tables {
let table_identifier =
build_table_identifier(table, &request.namespace_path, &self.client.id_delimiter);
build_table_identifier(table, &request.namespace_path, &self.client.id_delimiter)?;
let cache_key = build_cache_key(table, &request.namespace_path);
let remote_table = Arc::new(RemoteTable::new(
self.client.clone(),
@@ -1226,7 +1291,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
async fn list_tables(&self, request: ListTablesRequest) -> Result<ListTablesResponse> {
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
let mut req = self
.client
.get(&format!("/v1/namespace/{}/table/list", namespace_id));
@@ -1247,7 +1312,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
let namespace_vec = namespace_parts.to_vec();
for table in &response.tables {
let table_identifier =
build_table_identifier(table, &namespace_vec, &self.client.id_delimiter);
build_table_identifier(table, &namespace_vec, &self.client.id_delimiter)?;
let cache_key = build_cache_key(table, &namespace_vec);
let remote_table = Arc::new(RemoteTable::new(
self.client.clone(),
@@ -1269,7 +1334,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&request.name,
&request.namespace_path,
&self.client.id_delimiter,
);
)?;
let req = self
.client
.post(&format!("/v1/table/{}/create/", identifier))
@@ -1325,7 +1390,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&request.name,
&request.namespace_path,
&self.client.id_delimiter,
);
)?;
let cache_key = build_cache_key(&request.name, &request.namespace_path);
let table = Arc::new(RemoteTable::new(
self.client.clone(),
@@ -1344,7 +1409,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&request.target_table_name,
&request.target_namespace_path,
&self.client.id_delimiter,
);
)?;
let remote_request = RemoteCloneTableRequest {
source_location: request.source_uri,
@@ -1389,7 +1454,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&request.name,
&request.namespace_path,
&self.client.id_delimiter,
);
)?;
let cache_key = build_cache_key(&request.name, &request.namespace_path);
// We describe the table to confirm it exists before moving on.
@@ -1409,7 +1474,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
&request.name,
&request.namespace_path,
&self.client.id_delimiter,
);
)?;
let table = Arc::new(RemoteTable::new(
self.client.clone(),
request.name.clone(),
@@ -1437,7 +1502,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
new_namespace_path: &[String],
) -> Result<()> {
let current_identifier =
build_table_identifier(current_name, cur_namespace_path, &self.client.id_delimiter);
build_table_identifier(current_name, cur_namespace_path, &self.client.id_delimiter)?;
let current_cache_key = build_cache_key(current_name, cur_namespace_path);
let new_cache_key = build_cache_key(new_name, new_namespace_path);
@@ -1501,7 +1566,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
request: ListNamespacesRequest,
) -> Result<ListNamespacesResponse> {
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
let mut req = self
.client
.get(&format!("/v1/namespace/{}/list", namespace_id));
@@ -1523,7 +1588,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
request: CreateNamespaceRequest,
) -> Result<CreateNamespaceResponse> {
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
let mut req = self
.client
.post(&format!("/v1/namespace/{}/create", namespace_id));
@@ -1551,7 +1616,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
async fn drop_namespace(&self, request: DropNamespaceRequest) -> Result<DropNamespaceResponse> {
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
let mut req = self
.client
.post(&format!("/v1/namespace/{}/drop", namespace_id));
@@ -1582,7 +1647,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
request: DescribeNamespaceRequest,
) -> Result<DescribeNamespaceResponse> {
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter);
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
let req = self
.client
.post(&format!("/v1/namespace/{}/describe", namespace_id))
@@ -2420,7 +2485,11 @@ mod tests {
http::Response::builder()
.status(200)
.body(r#"{"tables": ["ns1$ns2$table1", "ns1$ns2$table2"]}"#)
// A namespace listing names the tables in that namespace; the
// namespace is the route, not part of each name. The client
// joins the two itself to build each table's identifier, so a
// listing that repeated the namespace would be joined twice.
.body(r#"{"tables": ["table1", "table2"]}"#)
.unwrap()
});
let names = conn
@@ -2429,7 +2498,7 @@ mod tests {
.execute()
.await
.unwrap();
assert_eq!(names, vec!["ns1$ns2$table1", "ns1$ns2$table2"]);
assert_eq!(names, vec!["table1", "table2"]);
}
#[tokio::test]
@@ -3464,6 +3533,100 @@ mod tests {
);
}
/// A table name is joined into the route the same way a Secret's is, so the
/// same two failures are reachable: a dot-only name leaves the table route
/// entirely, and a name holding the delimiter is indistinguishable from a
/// namespace boundary.
#[tokio::test]
async fn test_a_table_name_cannot_choose_its_own_route() {
use std::sync::{Arc, Mutex};
for name in ["..", ".", "../jobs", "a/b", "a$b"] {
let reached = Arc::new(Mutex::new(false));
let flag = reached.clone();
let conn = Connection::new_with_handler(move |_| {
*flag.lock().unwrap() = true;
http::Response::builder().status(200).body("{}").unwrap()
});
let error = conn
.drop_table(name, &[])
.await
.expect_err("an unaddressable table name must be refused");
assert!(!*reached.lock().unwrap(), "{name:?} reached the transport");
assert!(!error.to_string().is_empty(), "{name:?}");
}
}
/// The namespace half of the same join.
#[tokio::test]
async fn test_a_namespace_segment_cannot_choose_its_own_route() {
use std::sync::{Arc, Mutex};
for segment in ["..", "a/b", "a$b"] {
let reached = Arc::new(Mutex::new(false));
let flag = reached.clone();
let conn = Connection::new_with_handler(move |_| {
*flag.lock().unwrap() = true;
http::Response::builder().status(200).body("{}").unwrap()
});
let error = conn
.drop_table("t", &[segment.to_string()])
.await
.expect_err("an unaddressable namespace segment must be refused");
assert!(
!*reached.lock().unwrap(),
"{segment:?} reached the transport"
);
assert!(!error.to_string().is_empty(), "{segment:?}");
}
}
/// Under a configured delimiter that is inside the name charset, the one
/// segment `prod.vision` and the pair `prod`, `vision` would join to one
/// identifier. Refusing the first is what keeps them distinct.
#[tokio::test]
async fn test_a_configured_delimiter_is_refused_inside_a_table_component() {
use std::sync::{Arc, Mutex};
let reached = Arc::new(Mutex::new(false));
let flag = reached.clone();
let conn = Connection::new_with_handler_and_config(
move |_| {
*flag.lock().unwrap() = true;
http::Response::builder().status(200).body("{}").unwrap()
},
ClientConfig {
id_delimiter: Some(".".to_string()),
..Default::default()
},
);
let error = conn
.drop_table("t", &["prod.vision".to_string()])
.await
.expect_err("a segment holding the delimiter must be refused");
assert!(error.to_string().contains("delimiter"), "{error}");
assert!(!*reached.lock().unwrap(), "the request was sent anyway");
}
/// A Function name is percent-encoded, which covers everything but the dot
/// segment: `..` is unreserved, so it survives encoding and is then
/// resolved away, posting a registration body to `/v1/create`.
#[tokio::test]
async fn test_a_dot_only_function_name_is_refused() {
use std::sync::{Arc, Mutex};
for name in [".", "..", "..."] {
let reached = Arc::new(Mutex::new(false));
let flag = reached.clone();
let conn = Connection::new_with_handler(move |_| {
*flag.lock().unwrap() = true;
http::Response::builder().status(200).body("{}").unwrap()
});
let error = conn
.drop_function(name, "fv_1")
.await
.expect_err("a dot-only Function name must be refused");
assert!(error.to_string().contains("only periods"), "{error}");
assert!(!*reached.lock().unwrap(), "{name:?} reached the transport");
}
}
#[tokio::test]
async fn test_create_and_alter_secret_send_the_value_in_the_request_body() {
for (route, call) in [
rust/lancedb/src/utils/mod.rs
+27 -14
View File
@@ -81,6 +81,30 @@ impl PatchReadParam for ReadParams {
}
}
/// Refuse a path component that is periods alone.
///
/// Every character these names admit is unreserved, so a component needs no
/// escaping to sit in a URL path segment -- with one exception that is
/// positional rather than lexical. `.` and `..` are resolved as relative path
/// segments, and *after* percent-decoding, so `%2E%2E` is resolved exactly as
/// `..` is and no spelling of a dot-only component survives to reach a route.
///
/// An object so named could be created and then never addressed again, and on
/// the way there the request goes somewhere else: `drop_table("..")` resolves
/// to `/v1/drop/`, and a body meant for one route is delivered to whatever
/// handler is left at the other.
fn reject_dot_only(what: &str, value: &str) -> Result<()> {
if !value.is_empty() && value.chars().all(|character| character == '.') {
return Err(Error::InvalidInput {
message: format!(
"invalid {what} '{value}': a component of only periods is read as a relative \
path and cannot address an object"
),
});
}
Ok(())
}
/// Validate table name.
pub fn validate_table_name(name: &str) -> Result<()> {
if name.is_empty() {
@@ -97,6 +121,7 @@ pub fn validate_table_name(name: &str) -> Result<()> {
.to_string(),
});
}
reject_dot_only("table name", name)?;
Ok(())
}
@@ -126,6 +151,7 @@ pub fn validate_namespace_name(name: &str) -> Result<()> {
),
});
}
reject_dot_only("namespace name", name)?;
Ok(())
}
@@ -178,20 +204,7 @@ pub fn validate_secret_component(what: &str, value: &str) -> Result<()> {
),
});
}
// Every character above is unreserved, so a component needs no escaping to
// sit in a path segment -- with one exception that is positional rather
// than lexical. `.` and `..` are resolved as relative path segments, and
// after percent-decoding, so `%2E%2E` is resolved exactly as `..` is and no
// spelling of a dot-only component survives to reach a route. A Secret so
// named could be stored and then never addressed again.
if value.chars().all(|character| character == '.') {
return Err(Error::InvalidInput {
message: format!(
"invalid {what} '{value}': a component of only periods is read as a relative \
path and cannot address a Secret"
),
});
}
reject_dot_only(what, value)?;
Ok(())
}