fix(s3): implement S3-compliant CORS and bucket existence checks (#2026)

rustfs/src/server/layer.rs

@@ -27,7 +27,7 @@ use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 use tower::{Layer, Service};
-use tracing::{debug, info};
+use tracing::debug;
 
 /// Redirect layer that redirects browser requests to the console
 #[derive(Clone)]
@@ -363,53 +363,73 @@ where
         let method = req.method().clone();
         let request_headers = req.headers().clone();
         let cors_origins = self.cors_origins.clone();
-        // Handle OPTIONS preflight requests - return response directly without calling handler
-        if method == Method::OPTIONS && request_headers.contains_key(cors::standard::ORIGIN) {
-            info!("OPTIONS preflight request for path: {}", path);
+        let is_s3 = ConditionalCorsLayer::is_s3_path(&path);
+
+        if method == Method::OPTIONS {
+            if is_s3 {
+                let path_trimmed = path.trim_start_matches('/');
+                let bucket = path_trimmed.split('/').next().unwrap_or("").to_string();
+                let has_acrm = request_headers.contains_key(cors::request::ACCESS_CONTROL_REQUEST_METHOD);
+
+                return Box::pin(async move {
+                    if !has_acrm || !request_headers.contains_key(cors::standard::ORIGIN) {
+                        return Ok(Response::builder()
+                            .status(StatusCode::BAD_REQUEST)
+                            .body(ResBody::default())
+                            .unwrap());
+                    }
+
+                    if !bucket.is_empty()
+                        && let Some(cors_headers) = apply_cors_headers(&bucket, &method, &request_headers).await
+                    {
+                        let mut response = Response::builder().status(StatusCode::OK).body(ResBody::default()).unwrap();
+                        for (key, value) in cors_headers.iter() {
+                            response.headers_mut().insert(key, value.clone());
+                        }
+                        return Ok(response);
+                    }
+
+                    Ok(Response::builder()
+                        .status(StatusCode::FORBIDDEN)
+                        .body(ResBody::default())
+                        .unwrap())
+                });
+            }
 
-            let path_trimmed = path.trim_start_matches('/');
-            let bucket = path_trimmed.split('/').next().unwrap_or("").to_string(); // virtual host style?
-            let method_clone = method.clone();
             let request_headers_clone = request_headers.clone();
-
             return Box::pin(async move {
                 let mut response = Response::builder().status(StatusCode::OK).body(ResBody::default()).unwrap();
-
-                if ConditionalCorsLayer::is_s3_path(&path)
-                    && !bucket.is_empty()
-                    && let Some(cors_headers) = apply_cors_headers(&bucket, &method_clone, &request_headers_clone).await
-                {
-                    for (key, value) in cors_headers.iter() {
-                        response.headers_mut().insert(key, value.clone());
-                    }
-                    return Ok(response);
-                }
-
                 let cors_layer = ConditionalCorsLayer {
                     cors_origins: (*cors_origins).clone(),
                 };
                 cors_layer.apply_cors_headers(&request_headers_clone, response.headers_mut());
-
                 Ok(response)
             });
         }
 
-        if method == Method::OPTIONS && ConditionalCorsLayer::is_s3_path(&path) {
-            return Box::pin(async move { Ok(Response::builder().status(StatusCode::OK).body(ResBody::default()).unwrap()) });
-        }
-
         let mut inner = self.inner.clone();
+
         Box::pin(async move {
             let mut response = inner.call(req).await.map_err(Into::into)?;
 
-            // Apply CORS headers only to S3 API requests (non-OPTIONS)
             if request_headers.contains_key(cors::standard::ORIGIN)
                 && !response.headers().contains_key(cors::response::ACCESS_CONTROL_ALLOW_ORIGIN)
             {
-                let cors_layer = ConditionalCorsLayer {
-                    cors_origins: (*cors_origins).clone(),
-                };
-                cors_layer.apply_cors_headers(&request_headers, response.headers_mut());
+                if is_s3 {
+                    let bucket = path.trim_start_matches('/').split('/').next().unwrap_or("");
+                    if !bucket.is_empty()
+                        && let Some(cors_headers) = apply_cors_headers(bucket, &method, &request_headers).await
+                    {
+                        for (key, value) in cors_headers.iter() {
+                            response.headers_mut().insert(key, value.clone());
+                        }
+                    }
+                } else {
+                    let cors_layer = ConditionalCorsLayer {
+                        cors_origins: (*cors_origins).clone(),
+                    };
+                    cors_layer.apply_cors_headers(&request_headers, response.headers_mut());
+                }
             }
 
             Ok(response)
@@ -466,4 +486,50 @@ mod tests {
             ),
         );
     }
+
+    #[test]
+    fn test_is_s3_path_excludes_admin_and_special_paths() {
+        assert!(ConditionalCorsLayer::is_s3_path("/my-bucket/key"));
+        assert!(ConditionalCorsLayer::is_s3_path("/"));
+        assert!(!ConditionalCorsLayer::is_s3_path("/rustfs/admin/v3/info"));
+        assert!(!ConditionalCorsLayer::is_s3_path("/health"));
+        assert!(!ConditionalCorsLayer::is_s3_path("/health/ready"));
+    }
+
+    #[test]
+    fn test_generic_cors_layer_echoes_allowed_origin() {
+        let cors = ConditionalCorsLayer { cors_origins: None };
+        let mut req_headers = HeaderMap::new();
+        req_headers.insert("origin", "https://example.com".parse().unwrap());
+
+        let mut resp_headers = HeaderMap::new();
+        cors.apply_cors_headers(&req_headers, &mut resp_headers);
+
+        assert_eq!(
+            resp_headers.get(cors::response::ACCESS_CONTROL_ALLOW_ORIGIN).unwrap(),
+            "https://example.com"
+        );
+    }
+
+    #[test]
+    fn test_generic_cors_layer_respects_configured_origins() {
+        let cors = ConditionalCorsLayer {
+            cors_origins: Some("https://allowed.com".to_string()),
+        };
+
+        let mut req_headers = HeaderMap::new();
+        req_headers.insert("origin", "https://denied.com".parse().unwrap());
+        let mut resp_headers = HeaderMap::new();
+        cors.apply_cors_headers(&req_headers, &mut resp_headers);
+        assert!(resp_headers.get(cors::response::ACCESS_CONTROL_ALLOW_ORIGIN).is_none());
+
+        let mut req_headers = HeaderMap::new();
+        req_headers.insert("origin", "https://allowed.com".parse().unwrap());
+        let mut resp_headers = HeaderMap::new();
+        cors.apply_cors_headers(&req_headers, &mut resp_headers);
+        assert_eq!(
+            resp_headers.get(cors::response::ACCESS_CONTROL_ALLOW_ORIGIN).unwrap(),
+            "https://allowed.com"
+        );
+    }
 }

rustfs/src/storage/access.rs

@@ -26,9 +26,9 @@ use crate::server::RemoteAddr;
 use metrics::counter;
 use rustfs_ecstore::bucket::metadata_sys;
 use rustfs_ecstore::bucket::policy_sys::PolicySys;
-use rustfs_ecstore::error::StorageError;
+use rustfs_ecstore::error::{StorageError, is_err_bucket_not_found};
 use rustfs_ecstore::new_object_layer_fn;
-use rustfs_ecstore::store_api::ObjectOperations;
+use rustfs_ecstore::store_api::{BucketOperations, ObjectOperations};
 use rustfs_iam::error::Error as IamError;
 use rustfs_policy::policy::action::{Action, S3Action};
 use rustfs_policy::policy::{Args, BucketPolicyArgs};
@@ -771,6 +771,13 @@ impl S3Access for FS {
     ///
     /// This method returns `Ok(())` by default.
     async fn delete_object(&self, req: &mut S3Request<DeleteObjectInput>) -> S3Result<()> {
+        if let Some(store) = new_object_layer_fn()
+            && let Err(err) = store.get_bucket_info(&req.input.bucket, &Default::default()).await
+            && is_err_bucket_not_found(&err)
+        {
+            return Err(s3_error!(NoSuchBucket, "The specified bucket does not exist"));
+        }
+
         let req_info = ext_req_info_mut(&mut req.extensions)?;
         req_info.bucket = Some(req.input.bucket.clone());
         req_info.object = Some(req.input.key.clone());

rustfs/src/storage/ecfs.rs

@@ -17,7 +17,7 @@ use crate::app::multipart_usecase::DefaultMultipartUsecase;
 use crate::app::object_usecase::DefaultObjectUsecase;
 use rustfs_ecstore::{
     bucket::tagging::decode_tags_to_map,
-    error::{is_err_object_not_found, is_err_version_not_found},
+    error::{is_err_bucket_not_found, is_err_object_not_found, is_err_version_not_found},
     new_object_layer_fn,
     store_api::{BucketOperations, BucketOptions, ObjectOperations, ObjectOptions},
 };
@@ -617,6 +617,9 @@ impl FS {
                     );
                     return Ok(std::collections::HashMap::new());
                 }
+                if is_err_bucket_not_found(&e) {
+                    return Err(s3_error!(NoSuchBucket, "The specified bucket does not exist"));
+                }
                 warn!(
                     target: "rustfs::storage::ecfs",
                     bucket = %bucket,

rustfs/src/storage/ecfs_extend.rs

@@ -669,16 +669,13 @@ pub(crate) async fn apply_cors_headers(bucket: &str, method: &http::Method, head
         return None;
     }
 
-    // For OPTIONS (preflight) requests, check Access-Control-Request-Method
+    // Use Access-Control-Request-Method if present (for preflight and non-preflight requests),
+    // otherwise fall back to the actual HTTP method.
     let is_preflight = method == http::Method::OPTIONS;
-    let requested_method = if is_preflight {
-        headers
-            .get(cors::request::ACCESS_CONTROL_REQUEST_METHOD)
-            .and_then(|v| v.to_str().ok())
-            .unwrap_or(method_str)
-    } else {
-        method_str
-    };
+    let requested_method = headers
+        .get(cors::request::ACCESS_CONTROL_REQUEST_METHOD)
+        .and_then(|v| v.to_str().ok())
+        .unwrap_or(method_str);
 
     // Get requested headers from preflight request
     let requested_headers = if is_preflight {

rustfs/src/storage/ecfs_test.rs

@@ -1298,4 +1298,41 @@ mod tests {
             .unwrap();
         assert!(out.is_empty());
     }
+
+    // --- CORS origin pattern matching tests ---
+
+    #[test]
+    fn test_matches_origin_pattern_suffix_wildcard() {
+        assert!(matches_origin_pattern("*suffix", "foo.suffix"));
+        assert!(matches_origin_pattern("*suffix", "suffix"));
+        assert!(!matches_origin_pattern("*suffix", "foo.suffix.get"));
+        assert!(!matches_origin_pattern("*suffix", "foo.bar"));
+    }
+
+    #[test]
+    fn test_matches_origin_pattern_prefix_wildcard() {
+        assert!(matches_origin_pattern("prefix*", "prefix"));
+        assert!(matches_origin_pattern("prefix*", "prefix.suffix"));
+        assert!(!matches_origin_pattern("prefix*", "bla.prefix"));
+    }
+
+    #[test]
+    fn test_matches_origin_pattern_middle_wildcard() {
+        assert!(matches_origin_pattern("start*end", "startend"));
+        assert!(matches_origin_pattern("start*end", "start1end"));
+        assert!(matches_origin_pattern("start*end", "start12end"));
+        assert!(!matches_origin_pattern("start*end", "0start12end"));
+    }
+
+    #[test]
+    fn test_matches_origin_pattern_exact_no_wildcard() {
+        assert!(matches_origin_pattern("example.com", "example.com"));
+        assert!(!matches_origin_pattern("example.com", "other.com"));
+    }
+
+    #[test]
+    fn test_matches_origin_pattern_single_star_wildcard() {
+        assert!(matches_origin_pattern("*", "anything.com"));
+        assert!(matches_origin_pattern("*", ""));
+    }
 }

scripts/s3-tests/implemented_tests.txt

@@ -31,8 +31,11 @@
 # - Atomic read/write: Concurrent read/write consistency
 # - Object Lock: Enable on versioned existing buckets
 # - Checksum: SHA256 and CRC64NVME validation on PutObject
+# - CORS: Bucket-level CORS configuration, preflight, origin matching
+# - HTTP: 100-continue, multipart abort, tagging in POST Object
+# - DeleteObject: Proper NoSuchBucket for deleted buckets
 #
-# Total: 379 tests
+# Total: 393 tests
 
 test_basic_key_count
 test_bucket_create_naming_bad_short_one
@@ -465,3 +468,21 @@ test_object_lock_put_obj_lock_enable_after_create
 # Checksum validation tests
 test_object_checksum_sha256
 test_object_checksum_crc64nvme
+
+# CORS tests
+test_set_cors
+test_cors_origin_response
+test_cors_origin_wildcard
+test_cors_header_option
+test_cors_presigned_get_object
+test_cors_presigned_get_object_tenant
+test_cors_presigned_put_object
+test_cors_presigned_put_object_with_acl
+test_cors_presigned_put_object_tenant
+test_cors_presigned_put_object_tenant_with_acl
+
+# HTTP and DeleteObject tests
+test_100_continue
+test_abort_multipart_upload_not_found
+test_post_object_tags_authenticated_request
+test_object_delete_key_bucket_gone

scripts/s3-tests/non_standard_tests.txt

@@ -8,16 +8,13 @@
 # - fails_on_aws marker: Ceph-specific features
 # - X-RGW-* headers: Ceph proprietary headers
 # - allowUnordered: Ceph-specific query parameter
-# - CORS tests: Not implemented
 # - Bucket Logging: Ceph-specific logging extensions
 # - Object Lock: Ceph-specific lock behavior differences
 # - Lifecycle: Ceph-specific lifecycle expiration behavior
 # - SSE-KMS: Ceph-specific KMS extensions
 # - Error format differences: Minor response format variations
 
-test_100_continue
 test_100_continue_error_retry
-test_abort_multipart_upload_not_found
 test_account_usage
 test_atomic_conditional_write_1mb
 test_atomic_dual_conditional_write_1mb
@@ -146,19 +143,10 @@ test_copy_object_ifmatch_failed
 test_copy_object_ifmatch_good
 test_copy_object_ifnonematch_failed
 test_copy_object_ifnonematch_good
-test_cors_header_option
-test_cors_origin_response
-test_cors_origin_wildcard
-test_cors_presigned_get_object
-test_cors_presigned_get_object_tenant
 test_cors_presigned_get_object_tenant_v2
 test_cors_presigned_get_object_v2
-test_cors_presigned_put_object
-test_cors_presigned_put_object_tenant
 test_cors_presigned_put_object_tenant_v2
-test_cors_presigned_put_object_tenant_with_acl
 test_cors_presigned_put_object_v2
-test_cors_presigned_put_object_with_acl
 test_create_bucket_bucket_owner_enforced
 test_create_bucket_bucket_owner_preferred
 test_create_bucket_object_writer
@@ -257,7 +245,6 @@ test_multipart_use_cksum_helper_sha256
 test_non_multipart_get_part
 test_non_multipart_sse_c_get_part
 test_object_copy_canned_acl
-test_object_delete_key_bucket_gone
 test_object_header_acl_grants
 test_object_lock_changing_mode_from_compliance
 test_object_lock_changing_mode_from_governance_with_bypass
@@ -307,7 +294,6 @@ test_post_object_request_missing_policy_specified_field
 test_post_object_set_key_from_filename
 test_post_object_success_redirect_action
 test_post_object_tags_anonymous_request
-test_post_object_tags_authenticated_request
 test_post_object_wrong_bucket
 test_put_bucket_logging_account_j
 test_put_bucket_logging_account_s
@@ -335,7 +321,6 @@ test_read_through
 test_restore_noncur_obj
 test_restore_object_permanent
 test_restore_object_temporary
-test_set_cors
 test_sse_kms_default_post_object_authenticated_request
 test_sse_kms_default_upload_1b
 test_sse_kms_default_upload_1kb