diff --git a/data_types/src/namespace_name.rs b/data_types/src/namespace_name.rs index e86dcfb216..040e8ee9d4 100644 --- a/data_types/src/namespace_name.rs +++ b/data_types/src/namespace_name.rs @@ -7,6 +7,13 @@ use thiserror::Error; /// A `RangeInclusive` is a closed interval, covering [1, 64] const LENGTH_CONSTRAINT: RangeInclusive = 1..=64; +/// Whitelisted chars for a [`NamespaceName`] name. +/// +/// '/' | '_' | '-' are utilized by the platforms. +fn is_whitelisted(c: char) -> bool { + c.is_alphanumeric() || matches!(c, '/' | '_' | '-') +} + /// Errors returned when attempting to construct a [`NamespaceName`] from an org /// & bucket string pair. #[derive(Debug, Error)] @@ -40,7 +47,7 @@ pub enum NamespaceNameError { /// The provided namespace name contains an unacceptable character. #[error( "namespace name '{}' contains invalid character, character number {} \ - is a control which is not allowed", + is not whitelisted", name, bad_char_offset )] @@ -91,7 +98,7 @@ impl<'a> NamespaceName<'a> { // // NOTE: If changing these characters, please update the error message // above. - if let Some(bad_char_offset) = name.chars().position(|c| c.is_control()) { + if let Some(bad_char_offset) = name.chars().position(|c| !is_whitelisted(c)) { return Err(NamespaceNameError::BadChars { bad_char_offset, name: name.to_string(), @@ -197,20 +204,32 @@ mod tests { #[test] fn test_org_bucket_map_db_contains_underscore_and_percent() { - let got = NamespaceName::from_org_and_bucket("my%5Forg", "bucket").unwrap(); - assert_eq!(got.as_str(), "my%5Forg_bucket"); + let err = NamespaceName::from_org_and_bucket("my%5Forg", "bucket"); + assert!(matches!( + err, + Err(OrgBucketMappingError::InvalidNamespaceName { .. }) + )); - let got = NamespaceName::from_org_and_bucket("my%5Forg_", "bucket").unwrap(); - assert_eq!(got.as_str(), "my%5Forg__bucket"); + let err = NamespaceName::from_org_and_bucket("my%5Forg_", "bucket"); + assert!(matches!( + err, + Err(OrgBucketMappingError::InvalidNamespaceName { .. }) + )); } #[test] - fn test_bad_namespace_name_is_encoded() { - let got = NamespaceName::from_org_and_bucket("org", "bucket?").unwrap(); - assert_eq!(got.as_str(), "org_bucket?"); + fn test_bad_namespace_name_fails_validation() { + let err = NamespaceName::from_org_and_bucket("org", "bucket?"); + assert!(matches!( + err, + Err(OrgBucketMappingError::InvalidNamespaceName { .. }) + )); - let got = NamespaceName::from_org_and_bucket("org!", "bucket").unwrap(); - assert_eq!(got.as_str(), "org!_bucket"); + let err = NamespaceName::from_org_and_bucket("org!", "bucket"); + assert!(matches!( + err, + Err(OrgBucketMappingError::InvalidNamespaceName { .. }) + )); } #[test] @@ -251,30 +270,50 @@ mod tests { #[test] fn test_bad_chars_null() { let got = NamespaceName::new("example\x00").unwrap_err(); - assert_eq!(got.to_string() , "namespace name 'example\x00' contains invalid character, character number 7 is a control which is not allowed"); + assert_eq!(got.to_string() , "namespace name 'example\x00' contains invalid character, character number 7 is not whitelisted"); } #[test] fn test_bad_chars_high_control() { let got = NamespaceName::new("\u{007f}example").unwrap_err(); - assert_eq!(got.to_string() , "namespace name '\u{007f}example' contains invalid character, character number 0 is a control which is not allowed"); + assert_eq!(got.to_string() , "namespace name '\u{007f}example' contains invalid character, character number 0 is not whitelisted"); } #[test] fn test_bad_chars_tab() { let got = NamespaceName::new("example\tdb").unwrap_err(); - assert_eq!(got.to_string() , "namespace name 'example\tdb' contains invalid character, character number 7 is a control which is not allowed"); + assert_eq!(got.to_string() , "namespace name 'example\tdb' contains invalid character, character number 7 is not whitelisted"); } #[test] fn test_bad_chars_newline() { let got = NamespaceName::new("my_example\ndb").unwrap_err(); - assert_eq!(got.to_string() , "namespace name 'my_example\ndb' contains invalid character, character number 10 is a control which is not allowed"); + assert_eq!(got.to_string() , "namespace name 'my_example\ndb' contains invalid character, character number 10 is not whitelisted"); + } + + #[test] + fn test_bad_chars_whitespace() { + let got = NamespaceName::new("my_example db").unwrap_err(); + assert_eq!(got.to_string() , "namespace name 'my_example db' contains invalid character, character number 10 is not whitelisted"); + } + + #[test] + fn test_bad_chars_single_quote() { + let got = NamespaceName::new("my_example'db").unwrap_err(); + assert_eq!(got.to_string() , "namespace name 'my_example\'db' contains invalid character, character number 10 is not whitelisted"); } #[test] fn test_ok_chars() { - let db = NamespaceName::new("my-example-db_with_underscores and spaces").unwrap(); - assert_eq!(&*db, "my-example-db_with_underscores and spaces"); + let db = + NamespaceName::new("my-example-db_with_underscores/and/fwd/slash/AndCaseSensitive") + .unwrap(); + assert_eq!( + &*db, + "my-example-db_with_underscores/and/fwd/slash/AndCaseSensitive" + ); + + let db = NamespaceName::new("a_ã_京").unwrap(); + assert_eq!(&*db, "a_ã_京"); } } diff --git a/router/src/server/http/write/multi_tenant.rs b/router/src/server/http/write/multi_tenant.rs index 4cc218ec97..734777adb9 100644 --- a/router/src/server/http/write/multi_tenant.rs +++ b/router/src/server/http/write/multi_tenant.rs @@ -78,6 +78,7 @@ fn parse_v2(req: &Request) -> Result #[cfg(test)] mod tests { use assert_matches::assert_matches; + use data_types::NamespaceNameError; use super::*; use crate::server::http::write::Precision; @@ -189,12 +190,11 @@ mod tests { test_parse_v2!( encoded_quotation, query_string = "?org=cool'confusing&bucket=bucket", - want = Ok(WriteParams { - namespace, - .. - }) => { - assert_eq!(namespace.as_str(), "cool'confusing_bucket"); - } + want = Err(Error::MultiTenantError( + MultiTenantExtractError::InvalidOrgAndBucket( + OrgBucketMappingError::InvalidNamespaceName(NamespaceNameError::BadChars { .. }) + ) + )) ); test_parse_v2!( diff --git a/router/src/server/http/write/single_tenant/mod.rs b/router/src/server/http/write/single_tenant/mod.rs index 17853ea93a..b9adfe576b 100644 --- a/router/src/server/http/write/single_tenant/mod.rs +++ b/router/src/server/http/write/single_tenant/mod.rs @@ -381,9 +381,9 @@ mod tests { test_parse_v1!( encoded_quotation, query_string = "?db=ban'anas", - want = Ok(WriteParams{ namespace, precision: _ }) => { - assert_eq!(namespace.as_str(), "ban'anas"); - } + want = Err(Error::SingleTenantError( + SingleTenantExtractError::InvalidNamespace(NamespaceNameError::BadChars { .. }) + )) ); test_parse_v1!( @@ -483,17 +483,24 @@ mod tests { )) ); - // Do not encode potentially problematic input. test_parse_v2!( - no_encoding, + url_encoding, // URL-encoded input that is decoded in the HTTP layer - query_string = "?bucket=cool%2Fconfusing%F0%9F%8D%8C&prg=org", + query_string = "?bucket=cool%2Fconfusing&prg=org", want = Ok(WriteParams {namespace, ..}) => { // Yielding a not-encoded string as the namespace. - assert_eq!(namespace.as_str(), "cool/confusing🍌"); + assert_eq!(namespace.as_str(), "cool/confusing"); } ); + test_parse_v2!( + encoded_emoji, + query_string = "?bucket=confusing%F0%9F%8D%8C&prg=org", + want = Err(Error::SingleTenantError( + SingleTenantExtractError::InvalidNamespace(NamespaceNameError::BadChars { .. }) + )) + ); + test_parse_v2!( org_ignored, query_string = "?org=wat&bucket=bananas", @@ -518,14 +525,11 @@ mod tests { ); test_parse_v2!( - encoded_quotation, + single_quotation, query_string = "?bucket=buc'ket", - want = Ok(WriteParams { - namespace, - .. - }) => { - assert_eq!(namespace.as_str(), "buc'ket"); - } + want = Err(Error::SingleTenantError( + SingleTenantExtractError::InvalidNamespace(NamespaceNameError::BadChars { .. }) + )) ); test_parse_v2!( diff --git a/service_grpc_namespace/src/lib.rs b/service_grpc_namespace/src/lib.rs index f624838367..3fd96297bb 100644 --- a/service_grpc_namespace/src/lib.rs +++ b/service_grpc_namespace/src/lib.rs @@ -633,23 +633,26 @@ mod tests { test_create_namespace_name!(ok, name = "bananas", want = Ok("bananas")); - test_create_namespace_name!(multi_byte, name = "🍌", want = Ok("🍌")); + test_create_namespace_name!(multi_byte, name = "🍌", want = Err(e) => { + assert_eq!(e.code(), Code::InvalidArgument); + assert_eq!(e.message(), "namespace name '🍌' contains invalid character, character number 0 is not whitelisted"); + }); test_create_namespace_name!( tab, name = "it\tis\ttabtasitc", want = Err(e) => { assert_eq!(e.code(), Code::InvalidArgument); - assert_eq!(e.message(), "namespace name 'it\tis\ttabtasitc' contains invalid character, character number 2 is a control which is not allowed"); + assert_eq!(e.message(), "namespace name 'it\tis\ttabtasitc' contains invalid character, character number 2 is not whitelisted"); } ); test_create_namespace_name!( null, - name = "bad \0 bananas", + name = "bad\0bananas", want = Err(e) => { assert_eq!(e.code(), Code::InvalidArgument); - assert_eq!(e.message(), "namespace name 'bad \0 bananas' contains invalid character, character number 4 is a control which is not allowed"); + assert_eq!(e.message(), "namespace name 'bad\0bananas' contains invalid character, character number 3 is not whitelisted"); } );