bugfixes

HJLebbink · HJLebbink · commit 448436e189d6 · 2025-04-23T15:11:57.000+02:00
diff --git a/benches/s3/api_benchmarks.rs b/benches/s3/api_benchmarks.rs
@@ -88,7 +88,7 @@ criterion_group!(
         //
         bench_list_buckets,
         bench_object_copy_internal,
-        //bench_object_append, // TODO: add support to switch on/off s3-express
+        bench_object_append,
         bench_object_put,
         //
         bench_enable_object_legal_hold,
diff --git a/examples/common.rs b/examples/common.rs
@@ -21,6 +21,7 @@ pub fn create_client_on_play() -> Result<Client, Box<dyn std::error::Error + Sen
     Ok(client)
 }
 
+#[allow(dead_code)]
 pub fn create_client_on_localhost() -> Result<Client, Box<dyn std::error::Error + Send + Sync>> {
     let base_url = "http://localhost:9000/".parse::<BaseUrl>()?;
     log::info!("Trying to connect to MinIO at: `{:?}`", base_url);
diff --git a/src/s3/builders/copy_object.rs b/src/s3/builders/copy_object.rs
@@ -98,7 +98,7 @@ impl ToS3Request for UploadPartCopy {
             }
             if !(1..=10000).contains(&self.part_number) {
                 return Err(Error::InvalidPartNumber(
-                    "part number must be between 1 and 1000".into(),
+                    "part number must be between 1 and 10000".into(),
                 ));
             }
         }
diff --git a/src/s3/builders/get_presigned_policy_form_data.rs b/src/s3/builders/get_presigned_policy_form_data.rs
@@ -104,12 +104,12 @@ impl PostPolicy {
     }
 
     fn is_reserved_element(element: &str) -> bool {
-        element == "bucket"
-            || element == "x-amz-algorithm"
-            || element == "x-amz-credential"
-            || element == "x-amz-date"
-            || element == "policy"
-            || element == "x-amz-signature"
+        element.eq_ignore_ascii_case("bucket")
+            || element.eq_ignore_ascii_case("x-amz-algorithm")
+            || element.eq_ignore_ascii_case("x-amz-credential")
+            || element.eq_ignore_ascii_case("x-amz-date")
+            || element.eq_ignore_ascii_case("policy")
+            || element.eq_ignore_ascii_case("x-amz-signature")
     }
 
     fn get_credential_string(access_key: &String, date: &UtcTime, region: &String) -> String {
@@ -142,7 +142,10 @@ impl PostPolicy {
         }
 
         let v = PostPolicy::trim_dollar(element);
-        if v == "success_action_redirect" || v == "redirect" || v == "content-length-range" {
+        if v.eq_ignore_ascii_case("success_action_redirect")
+            || v.eq_ignore_ascii_case("redirect")
+            || v.eq_ignore_ascii_case("content-length-range")
+        {
             return Err(Error::PostPolicyError(format!(
                 "{} is unsupported for equals condition",
                 element
@@ -195,8 +198,8 @@ impl PostPolicy {
         }
 
         let v = PostPolicy::trim_dollar(element);
-        if v == "success_action_status"
-            || v == "content-length-range"
+        if v.eq_ignore_ascii_case("success_action_status")
+            || v.eq_ignore_ascii_case("content-length-range")
             || (v.starts_with("x-amz-") && v.starts_with("x-amz-meta-"))
         {
             return Err(Error::PostPolicyError(format!(
diff --git a/src/s3/builders/put_object.rs b/src/s3/builders/put_object.rs
@@ -417,7 +417,7 @@ impl ToS3Request for UploadPart {
             if let Some(part_number) = self.part_number {
                 if !(1..=10000).contains(&part_number) {
                     return Err(Error::InvalidPartNumber(
-                        "part number must be between 1 and 1000".into(),
+                        "part number must be between 1 and 10000".into(),
                     ));
                 }
             }
diff --git a/src/s3/client.rs b/src/s3/client.rs
@@ -256,19 +256,23 @@ impl Client {
         } else {
             task::block_in_place(|| match tokio::runtime::Runtime::new() {
                 Ok(rt) => {
+                    // create a random bucket name, and check if it exists,
+                    // we are not interested in the result, just the headers
+                    // which will contain the server type
+
                     let bucket_name: String = rand::thread_rng()
                         .sample_iter(&Alphanumeric)
                         .take(20)
                         .map(char::from)
                         .collect::<String>()
                         .to_lowercase();
 
-                    let express = rt.block_on(async {
+                    let express: bool = rt.block_on(async {
                         match BucketExists::new(self.clone(), bucket_name).send().await {
                             Ok(v) => {
                                 if let Some(server) = v.headers.get("server") {
                                     if let Ok(s) = server.to_str() {
-                                        return s == "MinIO Enterprise/S3express";
+                                        return s.eq_ignore_ascii_case("MinIO Enterprise/S3Express");
                                     }
                                 }
                             }
diff --git a/src/s3/error.rs b/src/s3/error.rs
@@ -44,6 +44,7 @@ pub enum ErrorCode {
     AccessDenied,
     NotSupported,
     BucketNotEmpty,
+    BucketAlreadyOwnedByYou,
 
     OtherError(String),
 }
@@ -72,6 +73,8 @@ impl ErrorCode {
             "accessdenied" => ErrorCode::AccessDenied,
             "notsupported" => ErrorCode::NotSupported,
             "bucketnotempty" => ErrorCode::BucketNotEmpty,
+            "bucketalreadyownedbyyou" => ErrorCode::BucketAlreadyOwnedByYou,
+
             v => ErrorCode::OtherError(v.to_owned()),
         }
     }
@@ -176,6 +179,7 @@ pub enum Error {
     TagDecodingError(String, String),
     ContentLengthUnknown,
 
+    //TODO are the following still needed?
     NoSuchTagSet,
     ReplicationConfigurationNotFoundError,
     NoSuchObjectLockConfiguration,
diff --git a/src/s3/http.rs b/src/s3/http.rs
@@ -171,7 +171,7 @@ fn get_aws_info(
     }
 
     let mut tokens: Vec<_> = host.get(matcher.len()..).unwrap().split('.').collect();
-    *dualstack = tokens[0] == "dualstack";
+    *dualstack = tokens[0].eq_ignore_ascii_case("dualstack");
     if *dualstack {
         tokens.remove(0);
     }
@@ -184,10 +184,12 @@ fn get_aws_info(
 
     let domain_suffix = tokens.join(".");
 
-    if host == "s3-external-1.amazonaws.com" {
+    if host.eq_ignore_ascii_case("s3-external-1.amazonaws.com") {
         region_in_host = DEFAULT_REGION.to_string();
     }
-    if host == "s3-us-gov-west-1.amazonaws.com" || host == "s3-fips-us-gov-west-1.amazonaws.com" {
+    if host.eq_ignore_ascii_case("s3-us-gov-west-1.amazonaws.com")
+        || host.eq_ignore_ascii_case("s3-fips-us-gov-west-1.amazonaws.com")
+    {
         region_in_host = "us-gov-west-1".to_string();
     }
 
@@ -336,9 +338,9 @@ impl BaseUrl {
     ) -> Result<(), Error> {
         let mut host = String::from(&self.aws_s3_prefix);
         host.push_str(&self.aws_domain_suffix);
-        if host == "s3-external-1.amazonaws.com"
-            || host == "s3-us-gov-west-1.amazonaws.com"
-            || host == "s3-fips-us-gov-west-1.amazonaws.com"
+        if host.eq_ignore_ascii_case("s3-external-1.amazonaws.com")
+            || host.eq_ignore_ascii_case("s3-us-gov-west-1.amazonaws.com")
+            || host.eq_ignore_ascii_case("s3-fips-us-gov-west-1.amazonaws.com")
         {
             url.host = host;
             return Ok(());
@@ -378,9 +380,9 @@ impl BaseUrl {
 
         let mut host = String::from(&self.aws_s3_prefix);
         host.push_str(&self.aws_domain_suffix);
-        if host == "s3-external-1.amazonaws.com"
-            || host == "s3-us-gov-west-1.amazonaws.com"
-            || host == "s3-fips-us-gov-west-1.amazonaws.com"
+        if host.eq_ignore_ascii_case("s3-external-1.amazonaws.com")
+            || host.eq_ignore_ascii_case("s3-us-gov-west-1.amazonaws.com")
+            || host.eq_ignore_ascii_case("s3-fips-us-gov-west-1.amazonaws.com")
         {
             url.host = host;
             return;
diff --git a/src/s3/response/bucket_exists.rs b/src/s3/response/bucket_exists.rs
@@ -53,10 +53,7 @@ impl FromS3Response for BucketExistsResponse {
                     exists: false,
                 })
             }
-            Err(e) => {
-                println!("Error: {:?}", e);
-                Err(e)
-            }
+            Err(e) => Err(e),
         }
     }
 }
diff --git a/src/s3/types.rs b/src/s3/types.rs
@@ -285,7 +285,7 @@ pub enum RetentionMode {
 
 impl RetentionMode {
     pub fn parse(s: &str) -> Result<RetentionMode, Error> {
-        match s {
+        match s.to_uppercase().as_str() {
             "GOVERNANCE" => Ok(RetentionMode::GOVERNANCE),
             "COMPLIANCE" => Ok(RetentionMode::COMPLIANCE),
             _ => Err(Error::InvalidRetentionMode(s.to_string())),
@@ -407,7 +407,7 @@ pub struct JsonInputSerialization {
 }
 
 #[derive(Clone, Debug, Default)]
-/// Parque input serialization definitions
+/// Parquet input serialization definitions
 pub struct ParquetInputSerialization;
 
 #[derive(Clone, Debug, Default)]
diff --git a/tests/test_bucket_create_delete.rs b/tests/test_bucket_create_delete.rs
@@ -14,28 +14,72 @@
 // limitations under the License.
 
 use minio::s3::client::DEFAULT_REGION;
+use minio::s3::error::{Error, ErrorCode};
 use minio::s3::response::{BucketExistsResponse, MakeBucketResponse, RemoveBucketResponse};
 use minio::s3::types::S3Api;
 use minio_common::test_context::TestContext;
 use minio_common::utils::rand_bucket_name;
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 10)]
-async fn bucket_create_delete() {
+async fn bucket_create() {
     let ctx = TestContext::new_from_env();
     let bucket_name = rand_bucket_name();
 
+    // try to create a bucket that does not exist
     let resp: MakeBucketResponse = ctx.client.make_bucket(&bucket_name).send().await.unwrap();
     assert_eq!(resp.bucket, bucket_name);
+    assert_eq!(resp.region, DEFAULT_REGION);
+
+    // check that the bucket exists
+    let resp: BucketExistsResponse = ctx.client.bucket_exists(&bucket_name).send().await.unwrap();
+    assert!(resp.exists);
+    assert_eq!(resp.bucket, bucket_name);
+    assert_eq!(resp.region, DEFAULT_REGION);
+
+    // try to create a bucket that already exists
+    let resp: Result<MakeBucketResponse, Error> = ctx.client.make_bucket(&bucket_name).send().await;
+    match resp {
+        Ok(_) => panic!("Bucket already exists, but was created again"),
+        Err(Error::S3Error(e)) if e.code == ErrorCode::BucketAlreadyOwnedByYou => {
+            // this is expected, as the bucket already exists
+        }
+        Err(e) => panic!("Unexpected error: {:?}", e),
+    }
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 10)]
+async fn bucket_delete() {
+    let ctx = TestContext::new_from_env();
+    let bucket_name = rand_bucket_name();
+
+    // try to remove a bucket that does not exist
+    let resp: Result<RemoveBucketResponse, Error> =
+        ctx.client.remove_bucket(&bucket_name).send().await;
+    match resp {
+        Ok(_) => panic!("Bucket does not exist, but was removed"),
+        Err(Error::S3Error(e)) if e.code == ErrorCode::NoSuchBucket => {
+            // this is expected, as the bucket does not exist
+        }
+        Err(e) => panic!("Unexpected error: {:?}", e),
+    }
+
+    // create a new bucket
+    let resp: MakeBucketResponse = ctx.client.make_bucket(&bucket_name).send().await.unwrap();
+    assert_eq!(resp.bucket, bucket_name);
+    assert_eq!(resp.region, DEFAULT_REGION);
 
+    // check that the bucket exists
     let resp: BucketExistsResponse = ctx.client.bucket_exists(&bucket_name).send().await.unwrap();
     assert!(resp.exists);
     assert_eq!(resp.bucket, bucket_name);
     assert_eq!(resp.region, DEFAULT_REGION);
 
+    // try to remove a bucket that exists
     let resp: RemoveBucketResponse = ctx.client.remove_bucket(&bucket_name).send().await.unwrap();
     assert_eq!(resp.bucket, bucket_name);
     assert_eq!(resp.region, DEFAULT_REGION);
 
+    // check that the bucket does not exist anymore
     let resp: BucketExistsResponse = ctx.client.bucket_exists(&bucket_name).send().await.unwrap();
     assert!(!resp.exists);
     assert_eq!(resp.bucket, bucket_name);
diff --git a/tests/test_upload_download_object.rs b/tests/test_upload_download_object.rs

Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ pub fn create_client_on_play() -> Result<Client, Box<dyn std::error::Error + Sen`
`21`	`21`	`Ok(client)`
`22`	`22`	`}`
`23`	`23`
	`24`	`+#[allow(dead_code)]`
`24`	`25`	`pub fn create_client_on_localhost() -> Result<Client, Box<dyn std::error::Error + Send + Sync>> {`
`25`	`26`	`let base_url = "http://localhost:9000/".parse::<BaseUrl>()?;`
`26`	`27`	log::info!("Trying to connect to MinIO at: `{:?}`", base_url);
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ impl ToS3Request for UploadPartCopy {`
`98`	`98`	`}`
`99`	`99`	`if !(1..=10000).contains(&self.part_number) {`
`100`	`100`	`return Err(Error::InvalidPartNumber(`
`101`		`- "part number must be between 1 and 1000".into(),`
	`101`	`+ "part number must be between 1 and 10000".into(),`
`102`	`102`	`));`
`103`	`103`	`}`
`104`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -417,7 +417,7 @@ impl ToS3Request for UploadPart {`
`417`	`417`	`if let Some(part_number) = self.part_number {`
`418`	`418`	`if !(1..=10000).contains(&part_number) {`
`419`	`419`	`return Err(Error::InvalidPartNumber(`
`420`		`- "part number must be between 1 and 1000".into(),`
	`420`	`+ "part number must be between 1 and 10000".into(),`
`421`	`421`	`));`
`422`	`422`	`}`
`423`	`423`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,10 +53,7 @@ impl FromS3Response for BucketExistsResponse {`
`53`	`53`	`exists: false,`
`54`	`54`	`})`
`55`	`55`	`}`
`56`		`- Err(e) => {`
`57`		`- println!("Error: {:?}", e);`
`58`		`- Err(e)`
`59`		`- }`
	`56`	`+ Err(e) => Err(e),`
`60`	`57`	`}`
`61`	`58`	`}`
`62`	`59`	`}`