fix:improve S3 error response and parsing compatibility (s3s-project#462)

* fix:improve S3 error response and parsing compatibility

* fix:cr

* fix:add unit test

* fix:fmt

* fix:cr

Author: LeonWang0735

crates/s3s/src/dto/copy_source.rs

@@ -54,31 +54,30 @@ impl CopySource {
     /// # Errors
     /// Returns an error if the header is invalid
     pub fn parse(header: &str) -> Result<Self, ParseCopySourceError> {
-        let header = urlencoding::decode(header).map_err(|_| ParseCopySourceError::InvalidEncoding)?;
+        let (path_part, version_id) = if let Some(idx) = header.find("?versionId=") {
+            let (path, version_part) = header.split_at(idx);
+            let version_id_raw = version_part.strip_prefix("?versionId=");
+            let version_id = version_id_raw
+                .map(urlencoding::decode)
+                .transpose()
+                .map_err(|_| ParseCopySourceError::InvalidEncoding)?;
+            (path, version_id)
+        } else {
+            (header, None)
+        };
+        let header = urlencoding::decode(path_part).map_err(|_| ParseCopySourceError::InvalidEncoding)?;
         let header = header.strip_prefix('/').unwrap_or(&header);
 
         // FIXME: support access point
         match header.split_once('/') {
             None => Err(ParseCopySourceError::PatternMismatch),
-            Some((bucket, remaining)) => {
-                let (key, version_id) = match remaining.split_once('?') {
-                    Some((key, remaining)) => {
-                        let version_id = remaining
-                            .split_once('=')
-                            .and_then(|(name, val)| (name == "versionId").then_some(val));
-                        (key, version_id)
-                    }
-                    None => (remaining, None),
-                };
-
+            Some((bucket, key)) => {
                 if !path::check_bucket_name(bucket) {
                     return Err(ParseCopySourceError::InvalidBucketName);
                 }
-
                 if !path::check_key(key) {
                     return Err(ParseCopySourceError::InvalidKey);
                 }
-
                 Ok(Self::Bucket {
                     bucket: bucket.into(),
                     key: key.into(),
@@ -119,6 +118,20 @@ impl http::TryFromHeaderValue for CopySource {
 mod tests {
     use super::*;
 
+    #[test]
+    fn leading_slash_and_percent_decoding() {
+        let header = "/awsexamplebucket/reports/file%3Fversion.txt?versionId=abc";
+        let val = CopySource::parse(header).unwrap();
+        match val {
+            CopySource::Bucket { bucket, key, version_id } => {
+                assert_eq!(&*bucket, "awsexamplebucket");
+                assert_eq!(&*key, "reports/file?version.txt");
+                assert_eq!(version_id.as_deref().unwrap(), "abc");
+            }
+            CopySource::AccessPoint { .. } => panic!(),
+        }
+    }
+
     #[test]
     fn path_style() {
         {

crates/s3s/src/ops/signature.rs

@@ -84,6 +84,7 @@ pub struct SignatureContext<'a> {
     pub trailing_headers: Option<TrailingHeaders>,
 }
 
+#[derive(Debug)]
 pub struct CredentialsExt {
     pub access_key: String,
     pub secret_key: SecretKey,

crates/s3s/src/ops/tests.rs

@@ -128,3 +128,53 @@ fn extract_host_from_uri() {
     let host = extract_host(&req).unwrap();
     assert_eq!(host, None);
 }
+
+#[tokio::test]
+async fn presigned_url_expires_0_should_be_expired() {
+    use crate::S3ErrorCode;
+    use crate::config::{S3ConfigProvider, StaticConfigProvider};
+    use crate::http::{Body, OrderedHeaders, OrderedQs};
+    use crate::ops::signature::SignatureContext;
+    use hyper::{Method, Uri};
+    use std::sync::Arc;
+
+    let qs = OrderedQs::parse(concat!(
+        "X-Amz-Algorithm=AWS4-HMAC-SHA256",
+        "&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20130524%2Fus-east-1%2Fs3%2Faws4_request",
+        "&X-Amz-Date=20130524T000000Z",
+        "&X-Amz-Expires=0",
+        "&X-Amz-SignedHeaders=host",
+        "&X-Amz-Signature=aeeed9bbccd4d02ee5c0109b86d86835f995330da4c265957d157751f604d404"
+    ))
+    .unwrap();
+
+    let config: Arc<dyn S3ConfigProvider> = Arc::new(StaticConfigProvider::default());
+
+    let method = Method::GET;
+    let uri = Uri::from_static("https://s3.amazonaws.com/test.txt");
+    let mut body = Body::empty();
+
+    let mut cx = SignatureContext {
+        auth: None,
+        config: &config,
+        req_version: ::http::Version::HTTP_11,
+        req_method: &method,
+        req_uri: &uri,
+        req_body: &mut body,
+        qs: Some(&qs),
+        hs: OrderedHeaders::from_slice_unchecked(&[]),
+        decoded_uri_path: "/test.txt".to_owned(),
+        vh_bucket: None,
+        content_length: None,
+        mime: None,
+        decoded_content_length: None,
+        transformed_body: None,
+        multipart: None,
+        trailing_headers: None,
+    };
+
+    let result = cx.v4_check_presigned_url().await;
+    assert!(result.is_err());
+    let err = result.unwrap_err();
+    assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
+}

crates/s3s/src/sig_v4/presigned_url_v4.rs

@@ -103,6 +103,6 @@ impl<'a> PresignedUrlV4<'a> {
 }
 
 fn parse_expires(s: &str) -> Option<time::Duration> {
-    let x = s.parse::<u32>().ok().filter(|&x| x > 0)?;
+    let x = s.parse::<u32>().ok()?;
     Some(time::Duration::new(i64::from(x), 0))
 }