mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-13 22:26:53 +00:00
fix: leave name length to the service, and check the delimiter on the config
Three review items.
The Secret character set no longer carries a length bound. `{1,255}` was the
only client-side length rule for any object -- tables, namespaces, Functions
and materialized views have never had one -- and it disagreed with the service,
which allows 512 bytes. A bound here could only be wrong in one direction or
the other: shorter refuses names the catalog would hold, longer says nothing.
How long a name may be is the service's to decide, as it already is everywhere
else.
`id_delimiter` is checked through `ClientConfig::validate` now, called as a
precondition before a client is built from the config rather than from inside
the struct literal that consumes it. A configuration mistake is reported where
it was made. The method is public so a caller assembling a config can ask the
same question without connecting.
It also refuses the exact relative segments. The root namespace is addressed by
the bare delimiter, so `id_delimiter="."` makes a root listing
`/v1/namespace/./secret/list`, which resolves to `/v1/namespace/secret/list` --
a different route, and one that exists. Only the exact values are refused: `.x.`
and `...` remain usable, for the same reason `...` is a usable name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
co-authored by
Claude Opus 5
parent
8b8479dc21
commit
9bb2be56a8
@@ -461,6 +461,18 @@ fn validate_id_delimiter(delimiter: String) -> Result<String> {
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
// The root namespace is addressed by the bare delimiter, so a delimiter that
|
||||
// is itself a relative segment makes the root listing route disappear:
|
||||
// `/v1/namespace/./secret/list` resolves to `/v1/namespace/secret/list`.
|
||||
if delimiter == "." || delimiter == ".." {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"id_delimiter '{delimiter}' is a relative path segment: the root namespace is \
|
||||
addressed by the delimiter alone, so a request for it would resolve to a \
|
||||
different route"
|
||||
),
|
||||
});
|
||||
}
|
||||
if let Some(unsafe_character) = delimiter
|
||||
.chars()
|
||||
.find(|character| matches!(character, '/' | '?' | '#' | '%' | '\\'))
|
||||
@@ -476,6 +488,19 @@ fn validate_id_delimiter(delimiter: String) -> Result<String> {
|
||||
Ok(delimiter)
|
||||
}
|
||||
|
||||
impl ClientConfig {
|
||||
/// Check the settings that a request cannot be built correctly without.
|
||||
///
|
||||
/// Called before a client is constructed from this, so a configuration
|
||||
/// mistake is reported where it was made rather than as a confusing
|
||||
/// response later. Public so a caller assembling a config can ask the same
|
||||
/// question without connecting.
|
||||
pub fn validate(&self) -> Result<()> {
|
||||
validate_id_delimiter(self.id_delimiter.clone().unwrap_or_else(|| "$".to_string()))?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl RestfulLanceDbClient<Sender> {
|
||||
fn get_timeout(passed: Option<Duration>, env_var: &str) -> Result<Option<Duration>> {
|
||||
if let Some(passed) = passed {
|
||||
@@ -501,6 +526,10 @@ impl RestfulLanceDbClient<Sender> {
|
||||
client_config: ClientConfig,
|
||||
read_consistency_interval: Option<Duration>,
|
||||
) -> Result<Self> {
|
||||
// Before anything is built from it: a bad delimiter is a mistake in the
|
||||
// caller's configuration, and saying so here names it as one.
|
||||
client_config.validate()?;
|
||||
|
||||
// Get the timeouts
|
||||
let timeout =
|
||||
Self::get_timeout(client_config.timeout_config.timeout, "LANCE_CLIENT_TIMEOUT")?;
|
||||
@@ -600,12 +629,10 @@ impl RestfulLanceDbClient<Sender> {
|
||||
host,
|
||||
retry_config,
|
||||
sender: Sender,
|
||||
id_delimiter: validate_id_delimiter(
|
||||
client_config
|
||||
.id_delimiter
|
||||
.clone()
|
||||
.unwrap_or("$".to_string()),
|
||||
)?,
|
||||
id_delimiter: client_config
|
||||
.id_delimiter
|
||||
.clone()
|
||||
.unwrap_or("$".to_string()),
|
||||
header_provider: client_config.header_provider,
|
||||
read_consistency_interval,
|
||||
max_bytes_per_request,
|
||||
@@ -1169,7 +1196,7 @@ mod tests {
|
||||
/// component contains one.
|
||||
#[test]
|
||||
fn test_a_delimiter_that_is_path_syntax_is_refused() {
|
||||
for delimiter in ["/", "?", "#", "%", "a/b", ""] {
|
||||
for delimiter in ["/", "?", "#", "%", "a/b", "", ".", "..", "/../../function/"] {
|
||||
let error = super::validate_id_delimiter(delimiter.to_string())
|
||||
.expect_err("a delimiter that is path syntax must be refused");
|
||||
assert!(
|
||||
@@ -1178,7 +1205,9 @@ mod tests {
|
||||
);
|
||||
}
|
||||
// The default, and other separators that are not path syntax, stand.
|
||||
for delimiter in ["$", ".", "-", "_", "|", "::"] {
|
||||
// A period is fine inside a longer delimiter -- only the exact relative
|
||||
// segments are a problem.
|
||||
for delimiter in ["$", "-", "_", "|", "::", ".x.", "..."] {
|
||||
super::validate_id_delimiter(delimiter.to_string())
|
||||
.unwrap_or_else(|error| panic!("{delimiter:?} must be accepted: {error}"));
|
||||
}
|
||||
|
||||
@@ -157,15 +157,19 @@ pub fn validate_namespace_name(name: &str) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The characters a Secret name or a Secret namespace segment may contain, and
|
||||
/// the longest one either may be.
|
||||
/// The characters a Secret name or a Secret namespace segment may contain.
|
||||
///
|
||||
/// Deliberately the set the service admits, and no positional rule on top of
|
||||
/// it: a segment may begin with `_`, `-` or `.`, because LanceDB namespaces
|
||||
/// already do, and anything narrower would put Secrets out of reach inside
|
||||
/// namespaces that already exist.
|
||||
///
|
||||
/// No length bound either. How long a name may be is the service's to decide,
|
||||
/// the way it is for a table, and a bound here could only disagree with it --
|
||||
/// one that is shorter refuses names the catalog would hold, and one that is
|
||||
/// longer says nothing.
|
||||
static SECRET_NAME_REGEX: std::sync::LazyLock<regex::Regex> =
|
||||
std::sync::LazyLock::new(|| regex::Regex::new(r"^[A-Za-z0-9_.\-]{1,255}$").unwrap());
|
||||
std::sync::LazyLock::new(|| regex::Regex::new(r"^[A-Za-z0-9_.\-]+$").unwrap());
|
||||
|
||||
/// Validate one component of a Secret identifier: a Secret name, or one segment
|
||||
/// of the namespace path holding it.
|
||||
@@ -202,7 +206,7 @@ pub fn validate_secret_component(what: &str, value: &str) -> Result<()> {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"invalid {what} '{value}': it may contain only alphanumeric characters, \
|
||||
underscores, hyphens and periods, and may be at most 255 bytes"
|
||||
underscores, hyphens and periods"
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user