diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 3189871b..f2d3fc23 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -144,7 +144,7 @@ impl Client, - /// The namespaces known to the state. The key is the lowercased namespace. + /// The namespaces known to the state. The key is the namespace. #[serde(skip_serializing_if = "IndexMap::is_empty")] namespaces: IndexMap, } @@ -164,20 +161,8 @@ impl LogState { } /// Gets the namespace state. - pub fn namespace_state(&self, namespace: &str) -> Result, &str> { - if let Some(def) = self.namespaces.get(&namespace.to_ascii_lowercase()) { - if def.namespace == namespace { - // namespace exact match, return namespace state - Ok(Some(&def.state)) - } else { - // namespace matches a namespace but differ in a case sensitive way, - // so return error with existing namespace - Err(&def.namespace) - } - } else { - // namespace is not defined - Ok(None) - } + pub fn namespace_state(&self, namespace: &str) -> Option<&NamespaceState> { + self.namespaces.get(namespace).map(|def| &def.state) } /// Checks the key has permission to sign checkpoints. @@ -411,30 +396,15 @@ impl LogState { }); } - let namespace_lowercase = namespace.to_ascii_lowercase(); - - if let Some(def) = self.namespaces.get(&namespace_lowercase) { - if namespace == def.namespace { - // namespace matches exactly - Err(ValidationError::NamespaceAlreadyDefined { - namespace: namespace.to_string(), - }) - } else { - // namespace matches an existing namespace but differs in a case sensitive way - Err(ValidationError::NamespaceConflict { - namespace: namespace.to_string(), - existing: def.namespace.to_string(), - }) - } + if self.namespaces.contains_key(namespace) { + // namespace is already defined + Err(ValidationError::NamespaceAlreadyDefined { + namespace: namespace.to_string(), + }) } else { // namespace is not defined - self.namespaces.insert( - namespace_lowercase, - NamespaceDefinition { - namespace: namespace.to_string(), - state, - }, - ); + self.namespaces + .insert(namespace.to_string(), NamespaceDefinition { state }); Ok(()) } @@ -647,14 +617,12 @@ mod tests { ( "my-namespace".to_string(), NamespaceDefinition { - namespace: "my-namespace".to_string(), state: NamespaceState::Defined, }, ), ( "imported-namespace".to_string(), NamespaceDefinition { - namespace: "imported-namespace".to_string(), state: NamespaceState::Imported { registry: "registry.example.com".to_string(), }, @@ -716,7 +684,7 @@ mod tests { // This validation should fail match state.validate(&envelope).unwrap_err() { - ValidationError::NamespaceConflict { .. } => {} + ValidationError::InvalidNamespace { .. } => {} _ => panic!("expected a different error"), } } diff --git a/crates/protocol/src/registry.rs b/crates/protocol/src/registry.rs index c7761be5..42e53e85 100644 --- a/crates/protocol/src/registry.rs +++ b/crates/protocol/src/registry.rs @@ -145,6 +145,7 @@ impl PackageName { // Validate the namespace and name parts are valid kebab strings if KebabStr::new(&name[colon + 1..]).is_some() && Self::is_valid_namespace(&name[..colon]) + && name[colon + 1..].chars().all(|c| !c.is_ascii_uppercase()) { return Ok(Self { package_name: name, @@ -153,7 +154,7 @@ impl PackageName { } } - bail!("invalid package name `{name}`: expected format is `:`") + bail!("invalid package name `{name}`: expected format is `:` using lowercased characters") } /// Gets the namespace part of the package identifier. @@ -168,7 +169,7 @@ impl PackageName { /// Check if string is a valid namespace. pub fn is_valid_namespace(namespace: &str) -> bool { - KebabStr::new(namespace).is_some() + KebabStr::new(namespace).is_some() && namespace.chars().all(|c| !c.is_ascii_uppercase()) } } diff --git a/crates/server/src/datastore/memory.rs b/crates/server/src/datastore/memory.rs index ac0194bf..d8553ea3 100644 --- a/crates/server/src/datastore/memory.rs +++ b/crates/server/src/datastore/memory.rs @@ -734,7 +734,7 @@ impl DataStore for MemoryDataStore { .state .namespace_state(package_name.namespace()) { - Ok(Some(state)) => match state { + Some(state) => match state { operator::NamespaceState::Defined => {} operator::NamespaceState::Imported { .. } => { return Err(DataStoreError::PackageNamespaceImported( @@ -742,17 +742,11 @@ impl DataStore for MemoryDataStore { )) } }, - Ok(None) => { + None => { return Err(DataStoreError::PackageNamespaceNotDefined( package_name.namespace().to_string(), )) } - Err(existing_namespace) => { - return Err(DataStoreError::PackageNamespaceConflict { - namespace: package_name.namespace().to_string(), - existing: existing_namespace.to_string(), - }) - } } // verify package name is unique in a case insensitive way diff --git a/crates/server/src/datastore/postgres/mod.rs b/crates/server/src/datastore/postgres/mod.rs index 4e3ebcc8..af647cde 100644 --- a/crates/server/src/datastore/postgres/mod.rs +++ b/crates/server/src/datastore/postgres/mod.rs @@ -976,7 +976,7 @@ impl DataStore for PostgresDataStore { // verify namespace is defined and not imported match validator.namespace_state(package_name.namespace()) { - Ok(Some(state)) => match state { + Some(state) => match state { operator::NamespaceState::Defined => {} operator::NamespaceState::Imported { .. } => { return Err(DataStoreError::PackageNamespaceImported( @@ -984,17 +984,11 @@ impl DataStore for PostgresDataStore { )) } }, - Ok(None) => { + None => { return Err(DataStoreError::PackageNamespaceNotDefined( package_name.namespace().to_string(), )) } - Err(existing_namespace) => { - return Err(DataStoreError::PackageNamespaceConflict { - namespace: package_name.namespace().to_string(), - existing: existing_namespace.to_string(), - }) - } } // verify package name is unique in a case insensitive way diff --git a/tests/memory/mod.rs b/tests/memory/mod.rs index daa15ab0..9747d796 100644 --- a/tests/memory/mod.rs +++ b/tests/memory/mod.rs @@ -91,12 +91,6 @@ async fn it_rejects_unknown_signing_key() -> Result<()> { test_unknown_signing_key(&config).await } -#[tokio::test(flavor = "multi_thread", worker_threads = 1)] -async fn it_rejects_package_name_conflict() -> Result<()> { - let (_server, config) = spawn_server(&root().await?, None, None, None).await?; - test_publishing_name_conflict(&config).await -} - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn it_rejects_invalid_signature() -> Result<()> { let (_server, config) = spawn_server(&root().await?, None, None, None).await?; diff --git a/tests/postgres/mod.rs b/tests/postgres/mod.rs index bd348440..8ca146fc 100644 --- a/tests/postgres/mod.rs +++ b/tests/postgres/mod.rs @@ -49,7 +49,6 @@ async fn it_works_with_postgres() -> TestResult { test_wit_publishing(&config).await?; test_wasm_content_policy(&config).await?; test_unauthorized_signing_key(&config).await?; - test_publishing_name_conflict(&config).await?; // This is tested below where a different server is used that // allows any signing key //test_unknown_signing_key(&config).await?; @@ -62,7 +61,6 @@ async fn it_works_with_postgres() -> TestResult { PackageName::new("test:yankee")?, PackageName::new("test:wit-package")?, PackageName::new("test:unauthorized-key")?, - PackageName::new("test:name")?, ]; // There should be two log entries in the registry diff --git a/tests/server.rs b/tests/server.rs index 672f3faf..755ec986 100644 --- a/tests/server.rs +++ b/tests/server.rs @@ -336,35 +336,6 @@ async fn test_unknown_signing_key(config: &Config) -> Result<()> { Ok(()) } -async fn test_publishing_name_conflict(config: &Config) -> Result<()> { - let client = create_client(config)?; - let signing_key = test_signing_key(); - - publish_component( - &client, - &PackageName::new("test:name")?, - "0.1.0", - "(component)", - true, - &signing_key, - ) - .await?; - - // should be rejected - publish_component( - &client, - &PackageName::new("test:NAME")?, - "0.1.1", - "(component)", - true, - &signing_key, - ) - .await - .expect_err("expected publish to fail"); - - Ok(()) -} - async fn test_invalid_signature(config: &Config) -> Result<()> { const PACKAGE_NAME: &str = "test:invalid-signature";