From 647b2dae33cd2b841e6f9514f4d4dc1313707fa7 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Tue, 3 Sep 2024 16:23:05 +0200 Subject: [PATCH 1/5] feat(api|web): persist `x-amz-website-redirect-location` directives This offers to a user the possibility to PUT Object (and their copy variants as long as the metadata directive is to REPLACE and not COPY) with a redirection as per https://docs.aws.amazon.com/AmazonS3/latest/userguide/how-to-page-redirect.html. Signed-off-by: Raito Bezarius --- src/api/s3/put.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 1e3b1b44..956db2e0 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -618,9 +618,11 @@ pub(crate) fn get_headers(headers: &HeaderMap) -> Result Date: Tue, 3 Sep 2024 15:43:08 +0200 Subject: [PATCH 2/5] feat(api|web): follow through metadata-driven redirects This is an implementation of the GET handler for the `x-amz-website-redirect-location`. This implements the semantics described in https://docs.aws.amazon.com/AmazonS3/latest/userguide/how-to-page-redirect.html for the S3 API and the web API. Signed-off-by: Raito Bezarius --- Cargo.lock | 12 ++++++++++++ Cargo.nix | 15 ++++++++++++++- src/api/Cargo.toml | 1 + src/api/s3/api_server.rs | 5 ++++- src/api/s3/get.rs | 33 ++++++++++++++++++++++++++++++--- src/garage/tests/s3/website.rs | 33 +++++++++++++++++++++++++++++++++ src/web/web_server.rs | 5 +++++ 7 files changed, 99 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b23da311..51f75afd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -197,6 +197,17 @@ dependencies = [ "zstd-safe", ] +[[package]] +name = "async-recursion" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.48", +] + [[package]] name = "async-stream" version = "0.3.5" @@ -1365,6 +1376,7 @@ dependencies = [ "aes-gcm", "argon2", "async-compression", + "async-recursion", "async-trait", "base64 0.21.7", "bytes", diff --git a/Cargo.nix b/Cargo.nix index 124d3196..e63b92a2 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -34,7 +34,7 @@ args@{ ignoreLockHash, }: let - nixifiedLockHash = "c0aa85d369b22875a652356862a5810c22838970be9fbec558dd108d5232881d"; + nixifiedLockHash = "fe0e77c1af963cc04accebb4ddd1607df5bbbaa48ca0b7a325f5e4497e21790e"; workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc; currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock); lockHashIgnored = if ignoreLockHash @@ -349,6 +349,18 @@ in }; }); + "registry+https://github.com/rust-lang/crates.io-index".async-recursion."1.1.1" = overridableMkRustCrate (profileName: rec { + name = "async-recursion"; + version = "1.1.1"; + registry = "registry+https://github.com/rust-lang/crates.io-index"; + src = fetchCratesIo { inherit name version; sha256 = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11"; }; + dependencies = { + proc_macro2 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".proc-macro2."1.0.78" { inherit profileName; }).out; + quote = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".quote."1.0.35" { inherit profileName; }).out; + syn = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".syn."2.0.48" { inherit profileName; }).out; + }; + }); + "registry+https://github.com/rust-lang/crates.io-index".async-stream."0.3.5" = overridableMkRustCrate (profileName: rec { name = "async-stream"; version = "0.3.5"; @@ -2003,6 +2015,7 @@ in aes_gcm = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".aes-gcm."0.10.3" { inherit profileName; }).out; argon2 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".argon2."0.5.3" { inherit profileName; }).out; async_compression = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".async-compression."0.4.6" { inherit profileName; }).out; + async_recursion = (buildRustPackages."registry+https://github.com/rust-lang/crates.io-index".async-recursion."1.1.1" { profileName = "__noProfile"; }).out; async_trait = (buildRustPackages."registry+https://github.com/rust-lang/crates.io-index".async-trait."0.1.77" { profileName = "__noProfile"; }).out; base64 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".base64."0.21.7" { inherit profileName; }).out; bytes = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".bytes."1.5.0" { inherit profileName; }).out; diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml index a5645c26..cccb722f 100644 --- a/src/api/Cargo.toml +++ b/src/api/Cargo.toml @@ -68,6 +68,7 @@ quick-xml.workspace = true opentelemetry.workspace = true opentelemetry-prometheus = { workspace = true, optional = true } prometheus = { workspace = true, optional = true } +async-recursion = "1.1.1" [features] k2v = [ "garage_util/k2v", "garage_model/k2v" ] diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs index 1737af33..e89d9a5e 100644 --- a/src/api/s3/api_server.rs +++ b/src/api/s3/api_server.rs @@ -201,7 +201,10 @@ impl ApiHandler for S3ApiServer { response_content_type, response_expires, }; - handle_get(ctx, &req, &key, part_number, overrides).await + // Redirects are flattened over the S3 API as per: + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/how-to-page-redirect.html + // > REST endpoint – Amazon S3 doesn't redirect the page request. It returns the requested object. + handle_get(ctx, &req, &key, part_number, overrides, true).await } Endpoint::UploadPart { key, diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index f5d3cf11..0d8a2dd6 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -280,22 +280,25 @@ pub async fn handle_head_without_ctx( /// Handle GET request pub async fn handle_get( ctx: ReqCtx, - req: &Request, + req: &Request, key: &str, part_number: Option, overrides: GetObjectOverrides, + flatten_redirect: bool ) -> Result, Error> { - handle_get_without_ctx(ctx.garage, req, ctx.bucket_id, key, part_number, overrides).await + handle_get_without_ctx(ctx.garage, req, ctx.bucket_id, key, part_number, overrides, flatten_redirect).await } /// Handle GET request +#[async_recursion::async_recursion] pub async fn handle_get_without_ctx( garage: Arc, - req: &Request, + req: &Request, bucket_id: Uuid, key: &str, part_number: Option, overrides: GetObjectOverrides, + flatten_redirect: bool ) -> Result, Error> { let object = garage .object_table @@ -329,6 +332,30 @@ pub async fn handle_get_without_ctx( let checksum_mode = checksum_mode(&req); + if let Some(redirect_hdr) = headers + .headers + .iter() + .find(|(k, _)| k == "x-amz-website-redirect-location") + .map(|(_, v)| v) + { + if flatten_redirect { + return handle_get_without_ctx(garage, + req, + bucket_id, + redirect_hdr, + part_number, + overrides, + flatten_redirect + ).await; + } else { + return Ok(Response::builder() + .status(StatusCode::FOUND) + .header("Location", redirect_hdr) + .body(empty_body()) + .unwrap()); + } + } + match (part_number, parse_range_header(req, last_v_meta.size)?) { (Some(_), Some(_)) => Err(Error::bad_request( "Cannot specify both partNumber and Range header", diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 0cadc388..5bb2fb6d 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -181,6 +181,16 @@ async fn test_website_s3_api() { .await .unwrap(); + ctx.client + .put_object() + .bucket(&bucket) + .key("site/home-redirected.html") + .website_redirect_location("/site/home.html") + .body(ByteStream::from_static(&[])) + .send() + .await + .unwrap(); + let conf = WebsiteConfiguration::builder() .index_document( IndexDocument::builder() @@ -274,6 +284,29 @@ async fn test_website_s3_api() { ); } + // Test redirection with CORS + { + let req = Request::builder() + .method("GET") + .uri(format!("http://127.0.0.1:{}/site/home-redirected.html", ctx.garage.web_port)) + .header("Host", format!("{}.web.garage", BCKT_NAME)) + .header("Origin", "https://example.com") + .body(Body::new(Bytes::new())) + .unwrap(); + + let resp = client.request(req).await.unwrap(); + + assert!(resp.status().is_redirection()); + assert_eq!( + resp.headers().get("access-control-allow-origin").unwrap(), + "*" + ); + assert_eq!( + resp.headers().get(http::header::LOCATION).unwrap(), + "/site/home.html" + ); + } + // Test ErrorDocument on 404 { let req = Request::builder() diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 69939f65..592f0be3 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -249,6 +249,9 @@ impl WebServer { handle_head_without_ctx(self.garage.clone(), req, bucket_id, &key, None).await } Method::GET => { + // Redirects are not flattened in the web API, as per: + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/how-to-page-redirect.html + // > Region-specific website endpoint – Amazon S3 redirects the page request according to the value of the x-amz-website-redirect-location property. handle_get_without_ctx( self.garage.clone(), req, @@ -256,6 +259,7 @@ impl WebServer { &key, None, Default::default(), + false ) .await } @@ -309,6 +313,7 @@ impl WebServer { &error_document, None, Default::default(), + false ) .await { -- 2.45.2 From 6db21f1bd531719c61349c04bdce20df2ef54139 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Tue, 3 Sep 2024 16:58:12 +0200 Subject: [PATCH 3/5] tests(api): test redirections for the S3 API This implements a 2-long chain of redirections. --- src/garage/tests/s3/objects.rs | 116 +++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/src/garage/tests/s3/objects.rs b/src/garage/tests/s3/objects.rs index 77eca2b1..ab21a9e8 100644 --- a/src/garage/tests/s3/objects.rs +++ b/src/garage/tests/s3/objects.rs @@ -3,6 +3,8 @@ use aws_sdk_s3::primitives::ByteStream; use aws_sdk_s3::types::{Delete, ObjectIdentifier}; const STD_KEY: &str = "hello world"; +const REDIRECT_KEY: &str = "redirected"; +const FURTHER_REDIRECT_KEY: &str = "redirected 2"; const CTRL_KEY: &str = "\x00\x01\x02\x00"; const UTF8_KEY: &str = "\u{211D}\u{1F923}\u{1F44B}"; const BODY: &[u8; 62] = b"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; @@ -404,3 +406,117 @@ async fn test_deleteobject() { .await .unwrap(); } + +#[tokio::test] +async fn test_redirections() { + let ctx = common::context(); + let bucket = ctx.create_bucket("redirections"); + + { + let etag = "\"49f68a5c8493ec2c0bf489821c21fc3b\""; + let empty_etag = "\"d41d8cd98f00b204e9800998ecf8427e\""; + let content_type = "text/csv"; + let data = ByteStream::from_static(b"hi"); + + + { + // Send an empty object (can serve as a directory marker) + // with a content type + let r = ctx + .client + .put_object() + .bucket(&bucket) + .key(STD_KEY) + .body(data) + .content_type(content_type) + .send() + .await + .unwrap(); + + assert_eq!(r.e_tag.unwrap().as_str(), etag); + // We return a version ID here + // We should check if Amazon is returning one when versioning is not enabled + assert!(r.version_id.is_some()); + } + + { + // Send an empty redirected object. + let r = ctx + .client + .put_object() + .bucket(&bucket) + .key(REDIRECT_KEY) + .website_redirect_location(STD_KEY) + .send() + .await + .unwrap(); + + // A redirected object remains an object in the end. + assert_eq!(r.e_tag.unwrap().as_str(), empty_etag); + // We return a version ID here + // We should check if Amazon is returning one when versioning is not enabled + assert!(r.version_id.is_some()); + + + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(REDIRECT_KEY) + .send() + .await + .unwrap(); + + assert_bytes_eq!(o.body, b"hi"); + assert_eq!(o.e_tag.unwrap(), etag); + // We do not return version ID + // We should check if Amazon is returning one when versioning is not enabled + // assert_eq!(o.version_id.unwrap(), _version); + assert_eq!(o.content_type.unwrap(), content_type); + assert!(o.last_modified.is_some()); + assert_eq!(o.content_length.unwrap(), 2); + assert_eq!(o.parts_count, None); + assert_eq!(o.tag_count, None); + } + + { + // Send an empty redirected object. + let r = ctx + .client + .put_object() + .bucket(&bucket) + .key(FURTHER_REDIRECT_KEY) + .website_redirect_location(REDIRECT_KEY) + .send() + .await + .unwrap(); + + // A redirected object remains an object in the end. + assert_eq!(r.e_tag.unwrap().as_str(), empty_etag); + // We return a version ID here + // We should check if Amazon is returning one when versioning is not enabled + assert!(r.version_id.is_some()); + + + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(FURTHER_REDIRECT_KEY) + .send() + .await + .unwrap(); + + assert_bytes_eq!(o.body, b"hi"); + assert_eq!(o.e_tag.unwrap(), etag); + // We do not return version ID + // We should check if Amazon is returning one when versioning is not enabled + // assert_eq!(o.version_id.unwrap(), _version); + assert_eq!(o.content_type.unwrap(), content_type); + assert!(o.last_modified.is_some()); + assert_eq!(o.content_length.unwrap(), 2); + assert_eq!(o.parts_count, None); + assert_eq!(o.tag_count, None); + } + } +} -- 2.45.2 From b05e20901ea68d7040467c6a51d426372fea53b8 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Tue, 3 Sep 2024 16:47:21 +0200 Subject: [PATCH 4/5] chore(devshell): add cargo2nix to it Signed-off-by: Raito Bezarius --- flake.nix | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flake.nix b/flake.nix index e690aef1..962418a1 100644 --- a/flake.nix +++ b/flake.nix @@ -71,6 +71,7 @@ rustfmt clang mold + cargo2nix.packages.${system}.default ]); # import the full shell using `nix develop .#full` @@ -79,6 +80,7 @@ rust-analyzer clang mold + cargo2nix.packages.${system}.default # ---- extra packages for dev tasks ---- cargo-audit cargo-outdated -- 2.45.2 From 5275ae60b0140f5bf326ded4e0bcbcf7121977b5 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Tue, 3 Sep 2024 16:47:48 +0200 Subject: [PATCH 5/5] chore(flake-inputs): use a cargo2nix reference without a deprecated Nix feature Otherwise, this make it hard for people with strict deprecation policies to use the devshell. Signed-off-by: Raito Bezarius --- flake.lock | 14 +++++++------- flake.nix | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/flake.lock b/flake.lock index a8ebe3c2..0b835517 100644 --- a/flake.lock +++ b/flake.lock @@ -12,17 +12,17 @@ "rust-overlay": "rust-overlay" }, "locked": { - "lastModified": 1666087781, - "narHash": "sha256-trKVdjMZ8mNkGfLcY5LsJJGtdV3xJDZnMVrkFjErlcs=", + "lastModified": 1725369207, + "narHash": "sha256-nIMmEOHOSSkyEZzIwXdKRmilV2FboLIa4ceipJc3oNo=", "owner": "Alexis211", "repo": "cargo2nix", - "rev": "a7a61179b66054904ef6a195d8da736eaaa06c36", + "rev": "0f857db8af0f9e2d9223b1605e5318c1cf8b7235", "type": "github" }, "original": { "owner": "Alexis211", "repo": "cargo2nix", - "rev": "a7a61179b66054904ef6a195d8da736eaaa06c36", + "rev": "0f857db8af0f9e2d9223b1605e5318c1cf8b7235", "type": "github" } }, @@ -58,11 +58,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1724395761, - "narHash": "sha256-zRkDV/nbrnp3Y8oCADf5ETl1sDrdmAW6/bBVJ8EbIdQ=", + "lastModified": 1725194671, + "narHash": "sha256-tLGCFEFTB5TaOKkpfw3iYT9dnk4awTP/q4w+ROpMfuw=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "ae815cee91b417be55d43781eb4b73ae1ecc396c", + "rev": "b833ff01a0d694b910daca6e2ff4a3f26dee478c", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index 962418a1..c313135d 100644 --- a/flake.nix +++ b/flake.nix @@ -10,7 +10,8 @@ inputs.cargo2nix = { # As of 2022-10-18: two small patches over unstable branch, one for clippy and one to fix feature detection - url = "github:Alexis211/cargo2nix/a7a61179b66054904ef6a195d8da736eaaa06c36"; + # with non deprecated URL literals. + url = "github:Alexis211/cargo2nix/0f857db8af0f9e2d9223b1605e5318c1cf8b7235"; # As of 2023-04-25: # - my two patches were merged into unstable (one for clippy and one to "fix" feature detection) -- 2.45.2