Skip to content

Commit 287896c

Browse files
authored
[BUG] re-add topology-aware name validation (chroma-core#6217)
## Description of changes Enable database name validation with support for topology-prefixed names using the `+` delimiter (e.g., "topology+name"). The validation: - Allows at most one `+` character in the name - Validates both topology and name portions independently - Enforces 512 character limit for topology-prefixed names - Re-enables name validation in create_database endpoint ## Test plan CI + staging ## Migration plan Not necessary. ## Observability plan N/A ## Documentation Changes N/A Co-authored-by: AI
1 parent 2711d5b commit 287896c

2 files changed

Lines changed: 121 additions & 4 deletions

File tree

rust/frontend/src/server.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use chroma_metering::{
1313
use chroma_system::System;
1414
use chroma_tracing::add_tracing_middleware;
1515
use chroma_types::{
16-
decode_embeddings, maybe_decode_update_embeddings, AddCollectionRecordsPayload,
16+
decode_embeddings, maybe_decode_update_embeddings, validate_name, AddCollectionRecordsPayload,
1717
AddCollectionRecordsResponse, AttachFunctionRequest, AttachFunctionResponse, ChecklistResponse,
1818
Collection, CollectionConfiguration, CollectionMetadataUpdate, CollectionUuid,
1919
CountCollectionsRequest, CountCollectionsResponse, CountRequest, CountResponse,
@@ -725,9 +725,9 @@ async fn create_database(
725725
State(mut server): State<FrontendServer>,
726726
Json(CreateDatabasePayload { name }): Json<CreateDatabasePayload>,
727727
) -> Result<Json<CreateDatabaseResponse>, ServerError> {
728-
// if let Err(err) = validate_name(&name) {
729-
// return Err(InvalidDatabaseError::from(err).into());
730-
// }
728+
if let Err(err) = validate_name(&name) {
729+
return Err(InvalidDatabaseError::from(err).into());
730+
}
731731
server.metrics.create_database.add(1, &[]);
732732
tracing::info!(name: "create_database", tenant_name = %tenant, database_name = %name);
733733
server

rust/types/src/validators.rs

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,34 @@ pub(crate) fn validate_non_empty_metadata<V>(
4242

4343
pub fn validate_name(name: impl AsRef<str>) -> Result<(), ValidationError> {
4444
let name_str = name.as_ref();
45+
46+
// A topology is a valid name. A database name prefixed with a topology is a valid name. The
47+
// conjunction must be separated by a single `+` and not exceed the database name limits.
48+
// Thus, we recurse after validating no more plusses.
49+
if let Some((topo, name)) = name_str.split_once('+') {
50+
if name_str.len() > 512 {
51+
return Err(ValidationError::new("name").with_message(
52+
format!(
53+
"Expected a name containing 3-512 characters. Got: {}",
54+
name_str.len()
55+
)
56+
.into(),
57+
));
58+
}
59+
if name.chars().any(|c| c == '+') {
60+
return Err(ValidationError::new("name").with_message(
61+
"Expected a name to contain at most one topology: Got two `+` characters.".into(),
62+
));
63+
}
64+
assert!(
65+
!topo.chars().any(|c| c == '+'),
66+
"split once should not bypass the split character"
67+
);
68+
validate_name(topo)?;
69+
validate_name(name)?;
70+
return Ok(());
71+
}
72+
4573
if !ALNUM_RE.is_match(name_str) {
4674
return Err(ValidationError::new("name").with_message(format!("Expected a name containing 3-512 characters from [a-zA-Z0-9._-], starting and ending with a character in [a-zA-Z0-9]. Got: {name_str}").into()));
4775
}
@@ -373,6 +401,95 @@ mod tests {
373401
use crate::operator::Key;
374402
use crate::{MetadataValue, SparseVector};
375403

404+
#[test]
405+
fn valid_simple_name() {
406+
assert!(validate_name("abc").is_ok());
407+
assert!(validate_name("my_collection").is_ok());
408+
assert!(validate_name("my-collection").is_ok());
409+
assert!(validate_name("my.collection").is_ok());
410+
assert!(validate_name("MyCollection123").is_ok());
411+
}
412+
413+
#[test]
414+
fn invalid_simple_name_too_short() {
415+
assert!(validate_name("ab").is_err());
416+
assert!(validate_name("a").is_err());
417+
assert!(validate_name("").is_err());
418+
}
419+
420+
#[test]
421+
fn invalid_simple_name_bad_start_or_end() {
422+
assert!(validate_name("_abc").is_err());
423+
assert!(validate_name("-abc").is_err());
424+
assert!(validate_name(".abc").is_err());
425+
assert!(validate_name("abc_").is_err());
426+
assert!(validate_name("abc-").is_err());
427+
assert!(validate_name("abc.").is_err());
428+
}
429+
430+
#[test]
431+
fn invalid_simple_name_double_period() {
432+
assert!(validate_name("abc..def").is_err());
433+
assert!(validate_name("my..collection").is_err());
434+
}
435+
436+
#[test]
437+
fn invalid_simple_name_ip_address() {
438+
assert!(validate_name("192.168.0.1").is_err());
439+
assert!(validate_name("127.0.0.1").is_err());
440+
}
441+
442+
#[test]
443+
fn valid_topology_prefixed_name() {
444+
assert!(validate_name("topo+name").is_ok());
445+
assert!(validate_name("my_topology+my_collection").is_ok());
446+
assert!(validate_name("region1+database").is_ok());
447+
assert!(validate_name("abc+def").is_ok());
448+
}
449+
450+
#[test]
451+
fn invalid_topology_prefixed_name_multiple_plus() {
452+
assert!(validate_name("a+b+c").is_err());
453+
assert!(validate_name("topo+name+extra").is_err());
454+
assert!(validate_name("one+two+three").is_err());
455+
}
456+
457+
#[test]
458+
fn invalid_topology_prefixed_name_invalid_topology() {
459+
// Topology part must be valid (3+ chars, start/end with alnum)
460+
assert!(validate_name("ab+valid").is_err());
461+
assert!(validate_name("_bad+valid").is_err());
462+
assert!(validate_name("bad_+valid").is_err());
463+
}
464+
465+
#[test]
466+
fn invalid_topology_prefixed_name_invalid_name() {
467+
// Name part must be valid (3+ chars, start/end with alnum)
468+
assert!(validate_name("valid+ab").is_err());
469+
assert!(validate_name("valid+_bad").is_err());
470+
assert!(validate_name("valid+bad_").is_err());
471+
}
472+
473+
#[test]
474+
fn invalid_topology_prefixed_name_too_long() {
475+
// Total length exceeds 512
476+
let long_topo = "a".repeat(256);
477+
let long_name = "b".repeat(258);
478+
let too_long = format!("{}+{}", long_topo, long_name);
479+
assert!(too_long.len() > 512);
480+
assert!(validate_name(&too_long).is_err());
481+
}
482+
483+
#[test]
484+
fn valid_topology_prefixed_name_at_limit() {
485+
// Total length exactly 512
486+
let topo = "a".repeat(255);
487+
let name = "b".repeat(256);
488+
let at_limit = format!("{}+{}", topo, name);
489+
assert_eq!(at_limit.len(), 512);
490+
assert!(validate_name(&at_limit).is_ok());
491+
}
492+
376493
#[test]
377494
fn test_metadata_validation() {
378495
// Valid metadata

0 commit comments

Comments
 (0)