From 8f97d691e1ff195308200e5b4c09a10450ff3f7b Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Wed, 20 Sep 2023 18:17:41 -0400 Subject: [PATCH] aws: s3sink: Fix handling of special characters in key Properly URL-encode the string if needed, and add some tests for a couple of cases. Fixes: https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/issues/431 Part-of: --- net/aws/src/s3sink/imp.rs | 13 ++-- net/aws/tests/s3.rs | 128 ++++++++++++++++++++++---------------- 2 files changed, 80 insertions(+), 61 deletions(-) diff --git a/net/aws/src/s3sink/imp.rs b/net/aws/src/s3sink/imp.rs index e6aa633d..652c6a24 100644 --- a/net/aws/src/s3sink/imp.rs +++ b/net/aws/src/s3sink/imp.rs @@ -115,12 +115,13 @@ struct Settings { impl Settings { fn to_uri(&self) -> String { - format!( - "s3://{}/{}/{}", - self.region, - self.bucket.as_ref().unwrap(), - self.key.as_ref().unwrap() - ) + GstS3Url { + region: self.region.clone(), + bucket: self.bucket.clone().unwrap(), + object: self.key.clone().unwrap(), + version: None, + } + .to_string() } fn to_metadata(&self, imp: &S3Sink) -> Option> { diff --git a/net/aws/tests/s3.rs b/net/aws/tests/s3.rs index a7d0071d..fafd4a24 100644 --- a/net/aws/tests/s3.rs +++ b/net/aws/tests/s3.rs @@ -8,72 +8,90 @@ // SPDX-License-Identifier: MPL-2.0 // -use gst::prelude::*; - -const DEFAULT_S3_REGION: &str = "us-west-2"; - -fn init() { - use std::sync::Once; - static INIT: Once = Once::new(); - - INIT.call_once(|| { - gst::init().unwrap(); - gstaws::plugin_register_static().unwrap(); - }); -} - // The test times out on Windows for some reason, skip until we figure out why #[cfg(not(target_os = "windows"))] #[test_with::env(AWS_ACCESS_KEY_ID)] #[test_with::env(AWS_SECRET_ACCESS_KEY)] -#[tokio::test] -async fn test_s3() { - init(); - // Makes it easier to get AWS SDK logs if needed - env_logger::init(); +#[cfg(test)] +mod tests { + use gst::prelude::*; - let region = std::env::var("AWS_REGION").unwrap_or_else(|_| DEFAULT_S3_REGION.to_string()); - let bucket = - std::env::var("AWS_S3_BUCKET").unwrap_or_else(|_| "gst-plugins-rs-tests".to_string()); - let key = format!("s3-test-{:?}.txt", chrono::Utc::now()); - let uri = format!("s3://{}/{}/{}", region, bucket, key); - let content = "Hello, world!\n".as_bytes(); + const DEFAULT_S3_REGION: &str = "us-west-2"; - // Manually add the element so we can configure it before it goes to PLAYING - let mut h1 = gst_check::Harness::new_empty(); - // Need to add_parse() because the Harness API / Rust bindings aren't conducive to creating and - // adding an element manually - h1.add_parse(format!("awss3sink uri={}", uri).as_str()); + fn init() { + use std::sync::Once; + static INIT: Once = Once::new(); - h1.set_src_caps(gst::Caps::builder("text/plain").build()); - h1.play(); + INIT.call_once(|| { + gst::init().unwrap(); + gstaws::plugin_register_static().unwrap(); + // Makes it easier to get AWS SDK logs if needed + env_logger::init(); + }); + } - h1.push(gst::Buffer::from_slice(content)).unwrap(); - h1.push_event(gst::event::Eos::new()); + // Common helper + async fn do_s3_test(key_prefix: &str) { + init(); - let mut h2 = gst_check::Harness::new("awss3src"); - h2.element().unwrap().set_property("uri", uri.clone()); - h2.play(); + let region = std::env::var("AWS_REGION").unwrap_or_else(|_| DEFAULT_S3_REGION.to_string()); + let bucket = + std::env::var("AWS_S3_BUCKET").unwrap_or_else(|_| "gst-plugins-rs-tests".to_string()); + let key = format!("{key_prefix}-{:?}.txt", chrono::Utc::now()); + let uri = format!("s3://{region}/{bucket}/{key}"); + let content = "Hello, world!\n".as_bytes(); - let buf = h2.pull_until_eos().unwrap().unwrap(); - assert_eq!( - content, - buf.into_mapped_buffer_readable().unwrap().as_slice() - ); + // Manually add the element so we can configure it before it goes to PLAYING + let mut h1 = gst_check::Harness::new_empty(); + // Need to add_parse() because the Harness API / Rust bindings aren't conducive to creating and + // adding an element manually + h1.add_parse(format!("awss3sink uri=\"{uri}\"").as_str()); - let region_provider = aws_config::meta::region::RegionProviderChain::first_try( - aws_sdk_s3::Region::new(region.clone()), - ) - .or_default_provider(); + h1.set_src_caps(gst::Caps::builder("text/plain").build()); + h1.play(); - let config = aws_config::from_env().region(region_provider).load().await; - let client = aws_sdk_s3::Client::new(&config); + h1.push(gst::Buffer::from_slice(content)).unwrap(); + h1.push_event(gst::event::Eos::new()); - client - .delete_object() - .bucket(bucket) - .key(key) - .send() - .await - .unwrap(); + let mut h2 = gst_check::Harness::new("awss3src"); + h2.element().unwrap().set_property("uri", uri.clone()); + h2.play(); + + let buf = h2.pull_until_eos().unwrap().unwrap(); + assert_eq!( + content, + buf.into_mapped_buffer_readable().unwrap().as_slice() + ); + + let region_provider = aws_config::meta::region::RegionProviderChain::first_try( + aws_sdk_s3::config::Region::new(region.clone()), + ) + .or_default_provider(); + + let config = aws_config::from_env().region(region_provider).load().await; + let client = aws_sdk_s3::Client::new(&config); + + client + .delete_object() + .bucket(bucket) + .key(key) + .send() + .await + .unwrap(); + } + + #[tokio::test] + async fn test_s3_simple() { + do_s3_test("s3-test").await; + } + + #[tokio::test] + async fn test_s3_whitespace() { + do_s3_test("s3 test").await; + } + + #[tokio::test] + async fn test_s3_unicode() { + do_s3_test("s3 🧪 😱").await; + } }