From 004bb5b4f1b2086914376265425fd46df5059db3 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Sun, 29 Jan 2023 01:16:04 +0000 Subject: [PATCH 1/2] api_server.rs: Adapted to use query string per Caddy upstream change. --- src/api/admin/api_server.rs | 32 +++++++++-------- src/garage/tests/s3/website.rs | 65 ++++++++++++++++++++++++++-------- 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs index a115d732..dae42059 100644 --- a/src/api/admin/api_server.rs +++ b/src/api/admin/api_server.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::net::SocketAddr; use std::sync::Arc; @@ -81,29 +82,32 @@ impl AdminApiServer { &self, req: Request, ) -> Result, Error> { - let has_domain_header = req.headers().contains_key("domain"); + let query_params: HashMap = req + .uri() + .query() + .map(|v| { + url::form_urlencoded::parse(v.as_bytes()) + .into_owned() + .collect() + }) + .unwrap_or_else(HashMap::new); - if !has_domain_header { - return Err(Error::bad_request("No domain header found")); + let has_domain_key = query_params.contains_key("domain"); + + if !has_domain_key { + return Err(Error::bad_request("No domain query string found")); } - let domain = &req - .headers() + let domain = query_params .get("domain") - .ok_or_internal_error("Could not parse domain header")?; - - let domain_string = String::from( - domain - .to_str() - .ok_or_bad_request("Invalid characters found in domain header")?, - ); + .ok_or_internal_error("Could not parse domain query string")?; let bucket_id = self .garage .bucket_helper() - .resolve_global_bucket_name(&domain_string) + .resolve_global_bucket_name(&domain) .await? - .ok_or(HelperError::NoSuchBucket(domain_string))?; + .ok_or(HelperError::NoSuchBucket(domain.to_string()))?; let bucket = self .garage diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 4e136e1b..7579058d 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -56,8 +56,11 @@ async fn test_website() { let admin_req = || { Request::builder() .method("GET") - .uri(format!("http://127.0.0.1:{}/check", ctx.garage.admin_port)) - .header("domain", BCKT_NAME.to_string()) + .uri(format!( + "http://127.0.0.1:{0}/check?domain={1}", + ctx.garage.admin_port, + BCKT_NAME.to_string() + )) .body(Body::empty()) .unwrap() }; @@ -91,8 +94,11 @@ async fn test_website() { let admin_req = || { Request::builder() .method("GET") - .uri(format!("http://127.0.0.1:{}/check", ctx.garage.admin_port)) - .header("domain", BCKT_NAME.to_string()) + .uri(format!( + "http://127.0.0.1:{0}/check?domain={1}", + ctx.garage.admin_port, + BCKT_NAME.to_string() + )) .body(Body::empty()) .unwrap() }; @@ -120,8 +126,11 @@ async fn test_website() { let admin_req = || { Request::builder() .method("GET") - .uri(format!("http://127.0.0.1:{}/check", ctx.garage.admin_port)) - .header("domain", BCKT_NAME.to_string()) + .uri(format!( + "http://127.0.0.1:{0}/check?domain={1}", + ctx.garage.admin_port, + BCKT_NAME.to_string() + )) .body(Body::empty()) .unwrap() }; @@ -408,7 +417,7 @@ async fn test_website_check_website_enabled() { res_body, json!({ "code": "InvalidRequest", - "message": "Bad request: No domain header found", + "message": "Bad request: No domain query string found", "region": "garage-integ-test", "path": "/check", }) @@ -417,8 +426,34 @@ async fn test_website_check_website_enabled() { let admin_req = || { Request::builder() .method("GET") - .uri(format!("http://127.0.0.1:{}/check", ctx.garage.admin_port)) - .header("domain", "foobar") + .uri(format!( + "http://127.0.0.1:{}/check?domain=", + ctx.garage.admin_port + )) + .body(Body::empty()) + .unwrap() + }; + + let admin_resp = client.request(admin_req()).await.unwrap(); + assert_eq!(admin_resp.status(), StatusCode::NOT_FOUND); + let res_body = json_body(admin_resp).await; + assert_json_eq!( + res_body, + json!({ + "code": "NoSuchBucket", + "message": "Bucket not found: ", + "region": "garage-integ-test", + "path": "/check", + }) + ); + + let admin_req = || { + Request::builder() + .method("GET") + .uri(format!( + "http://127.0.0.1:{}/check?domain=foobar", + ctx.garage.admin_port + )) .body(Body::empty()) .unwrap() }; @@ -439,20 +474,22 @@ async fn test_website_check_website_enabled() { let admin_req = || { Request::builder() .method("GET") - .uri(format!("http://127.0.0.1:{}/check", ctx.garage.admin_port)) - .header("domain", "☹") + .uri(format!( + "http://127.0.0.1:{}/check?domain=%E2%98%B9", + ctx.garage.admin_port + )) .body(Body::empty()) .unwrap() }; let admin_resp = client.request(admin_req()).await.unwrap(); - assert_eq!(admin_resp.status(), StatusCode::BAD_REQUEST); + assert_eq!(admin_resp.status(), StatusCode::NOT_FOUND); let res_body = json_body(admin_resp).await; assert_json_eq!( res_body, json!({ - "code": "InvalidRequest", - "message": "Bad request: Invalid characters found in domain header: failed to convert header to a str", + "code": "NoSuchBucket", + "message": "Bucket not found: ☹", "region": "garage-integ-test", "path": "/check", }) From 9c354f0a8ff258872aa3a4b7c116e1d66815afd1 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Sun, 29 Jan 2023 20:27:15 +0000 Subject: [PATCH 2/2] Improved bucket authorization response strings. --- src/api/admin/api_server.rs | 16 ++++++++++------ src/garage/tests/s3/website.rs | 6 +++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs index dae42059..58dd38d8 100644 --- a/src/api/admin/api_server.rs +++ b/src/api/admin/api_server.rs @@ -119,12 +119,16 @@ impl AdminApiServer { let bucket_website_config = bucket_state.website_config.get(); match bucket_website_config { - Some(_v) => Ok(Response::builder() - .status(StatusCode::OK) - .body(Body::from("Bucket authorized for website hosting"))?), - None => Err(Error::bad_request( - "Bucket is not authorized for website hosting", - )), + Some(_v) => { + Ok(Response::builder() + .status(StatusCode::OK) + .body(Body::from(format!( + "Bucket '{domain}' is authorized for website hosting" + )))?) + } + None => Err(Error::bad_request(format!( + "Bucket '{domain}' is not authorized for website hosting" + ))), } } diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 7579058d..f61838e4 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -72,7 +72,7 @@ async fn test_website() { res_body, json!({ "code": "InvalidRequest", - "message": "Bad request: Bucket is not authorized for website hosting", + "message": "Bad request: Bucket 'my-website' is not authorized for website hosting", "region": "garage-integ-test", "path": "/check", }) @@ -107,7 +107,7 @@ async fn test_website() { assert_eq!(admin_resp.status(), StatusCode::OK); assert_eq!( to_bytes(admin_resp.body_mut()).await.unwrap().as_ref(), - b"Bucket authorized for website hosting" + format!("Bucket '{BCKT_NAME}' is authorized for website hosting").as_bytes() ); ctx.garage @@ -142,7 +142,7 @@ async fn test_website() { res_body, json!({ "code": "InvalidRequest", - "message": "Bad request: Bucket is not authorized for website hosting", + "message": "Bad request: Bucket 'my-website' is not authorized for website hosting", "region": "garage-integ-test", "path": "/check", })