From bba9202f310b257ed52d1a82052f05532495c62e Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Mon, 19 May 2025 20:36:03 +0200 Subject: [PATCH] add test for punycode --- src/api/admin/bucket.rs | 4 +- src/api/s3/bucket.rs | 2 +- src/garage/admin/bucket.rs | 2 +- src/garage/tests/common/garage.rs | 2 + src/garage/tests/s3/website.rs | 73 +++++++++++++++++++++++++++++++ src/model/bucket_alias_table.rs | 8 ++-- src/model/helper/locked.rs | 4 +- src/util/config.rs | 4 +- 8 files changed, 87 insertions(+), 12 deletions(-) diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs index 6cc21938..f8bd1eb5 100644 --- a/src/api/admin/bucket.rs +++ b/src/api/admin/bucket.rs @@ -277,7 +277,7 @@ pub async fn handle_create_bucket( let helper = garage.locked_helper().await; if let Some(ga) = &req.global_alias { - if !is_valid_bucket_name(ga, garage.config.allow_punnycode) { + if !is_valid_bucket_name(ga, garage.config.allow_punycode) { return Err(Error::bad_request(format!( "{}: {}", ga, INVALID_BUCKET_NAME_MESSAGE @@ -292,7 +292,7 @@ pub async fn handle_create_bucket( } if let Some(la) = &req.local_alias { - if !is_valid_bucket_name(&la.alias, garage.config.allow_punnycode) { + if !is_valid_bucket_name(&la.alias, garage.config.allow_punycode) { return Err(Error::bad_request(format!( "{}: {}", la.alias, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/api/s3/bucket.rs b/src/api/s3/bucket.rs index d2a36c18..23cceb84 100644 --- a/src/api/s3/bucket.rs +++ b/src/api/s3/bucket.rs @@ -172,7 +172,7 @@ pub async fn handle_create_bucket( } // Create the bucket! - if !is_valid_bucket_name(&bucket_name, garage.config.allow_punnycode) { + if !is_valid_bucket_name(&bucket_name, garage.config.allow_punycode) { return Err(Error::bad_request(format!( "{}: {}", bucket_name, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/garage/admin/bucket.rs b/src/garage/admin/bucket.rs index 1ed0ebd8..073329c1 100644 --- a/src/garage/admin/bucket.rs +++ b/src/garage/admin/bucket.rs @@ -126,7 +126,7 @@ impl AdminRpcHandler { #[allow(clippy::ptr_arg)] async fn handle_create_bucket(&self, name: &String) -> Result { - if !is_valid_bucket_name(name, self.garage.config.allow_punnycode) { + if !is_valid_bucket_name(name, self.garage.config.allow_punycode) { return Err(Error::BadRequest(format!( "{}: {}", name, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/garage/tests/common/garage.rs b/src/garage/tests/common/garage.rs index 8d71504f..2b0a381c 100644 --- a/src/garage/tests/common/garage.rs +++ b/src/garage/tests/common/garage.rs @@ -63,6 +63,8 @@ rpc_bind_addr = "127.0.0.1:{rpc_port}" rpc_public_addr = "127.0.0.1:{rpc_port}" rpc_secret = "{secret}" +allow_punycode = true + [s3_api] s3_region = "{region}" api_bind_addr = "127.0.0.1:{s3_port}" diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 9a9e29f2..6d37eee8 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -533,3 +533,76 @@ async fn test_website_check_domain() { }) ); } + +#[tokio::test] +async fn test_website_puny() { + const BCKT_NAME: &str = "xn--pda.eu"; + let ctx = common::context(); + let bucket = ctx.create_bucket(BCKT_NAME); + + let data = ByteStream::from_static(BODY); + + ctx.client + .put_object() + .bucket(&bucket) + .key("index.html") + .body(data) + .send() + .await + .unwrap(); + + let client = Client::builder(TokioExecutor::new()).build_http(); + + let req = |suffix| { + Request::builder() + .method("GET") + .uri(format!("http://127.0.0.1:{}/", ctx.garage.web_port)) + .header("Host", format!("{}{}", BCKT_NAME, suffix)) + .body(Body::new(Bytes::new())) + .unwrap() + }; + + ctx.garage + .command() + .args(["bucket", "website", "--allow", BCKT_NAME]) + .quiet() + .expect_success_status("Could not allow website on bucket"); + + let mut resp = client.request(req("")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!( + resp.into_body().collect().await.unwrap().to_bytes(), + BODY.as_ref() + ); + + resp = client.request(req(".web.garage")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!( + resp.into_body().collect().await.unwrap().to_bytes(), + BODY.as_ref() + ); + + for bname in [ + BCKT_NAME.to_string(), + format!("{BCKT_NAME}.web.garage"), + format!("{BCKT_NAME}.s3.garage"), + ] { + let admin_req = || { + Request::builder() + .method("GET") + .uri(format!( + "http://127.0.0.1:{0}/check?domain={1}", + ctx.garage.admin_port, bname + )) + .body(Body::new(Bytes::new())) + .unwrap() + }; + + let admin_resp = client.request(admin_req()).await.unwrap(); + assert_eq!(admin_resp.status(), StatusCode::OK); + assert_eq!( + admin_resp.into_body().collect().await.unwrap().to_bytes(), + format!("Domain '{bname}' is managed by Garage").as_bytes() + ); + } +} diff --git a/src/model/bucket_alias_table.rs b/src/model/bucket_alias_table.rs index 04d808e8..276d0d1c 100644 --- a/src/model/bucket_alias_table.rs +++ b/src/model/bucket_alias_table.rs @@ -76,7 +76,7 @@ impl TableSchema for BucketAliasTable { /// In the case of Garage, bucket names must not be hex-encoded /// 32 byte string, which is excluded thanks to the /// maximum length of 63 bytes given in the spec. -pub fn is_valid_bucket_name(n: &str, punny: bool) -> bool { +pub fn is_valid_bucket_name(n: &str, puny: bool) -> bool { // Bucket names must be between 3 and 63 characters n.len() >= 3 && n.len() <= 63 // Bucket names must be composed of lowercase letters, numbers, @@ -88,9 +88,9 @@ pub fn is_valid_bucket_name(n: &str, punny: bool) -> bool { // Bucket names must not be formatted as an IP address && n.parse::().is_err() // Bucket names must not start with "xn--" - && (!n.starts_with("xn--") || punny) - // We are a bit stricter, to properly restrict punnycode in all labels - && (!n.contains(".xn--") || punny) + && (!n.starts_with("xn--") || puny) + // We are a bit stricter, to properly restrict punycode in all labels + && (!n.contains(".xn--") || puny) // Bucket names must not end with "-s3alias" && !n.ends_with("-s3alias") } diff --git a/src/model/helper/locked.rs b/src/model/helper/locked.rs index 16b0bafc..a5821f77 100644 --- a/src/model/helper/locked.rs +++ b/src/model/helper/locked.rs @@ -57,7 +57,7 @@ impl<'a> LockedHelper<'a> { bucket_id: Uuid, alias_name: &String, ) -> Result<(), Error> { - if !is_valid_bucket_name(alias_name, self.0.config.allow_punnycode) { + if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } @@ -217,7 +217,7 @@ impl<'a> LockedHelper<'a> { ) -> Result<(), Error> { let key_helper = KeyHelper(self.0); - if !is_valid_bucket_name(alias_name, self.0.config.allow_punnycode) { + if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } diff --git a/src/util/config.rs b/src/util/config.rs index f128177b..c74029e7 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -136,9 +136,9 @@ pub struct Config { #[serde(default = "Default::default")] pub admin: AdminConfig, - /// Allow punnycode in bucket names + /// Allow punycode in bucket names #[serde(default)] - pub allow_punnycode: bool, + pub allow_punycode: bool, } /// Value for data_dir: either a single directory or a list of dirs with attributes