diff --git a/rustfs/src/server/layer.rs b/rustfs/src/server/layer.rs index 0cc734cec..338fb91c3 100644 --- a/rustfs/src/server/layer.rs +++ b/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" + ); + } } diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index e307c07b8..92889513e 100644 --- a/rustfs/src/storage/access.rs +++ b/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) -> 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()); diff --git a/rustfs/src/storage/ecfs.rs b/rustfs/src/storage/ecfs.rs index 9733dcc07..bbc22bd39 100644 --- a/rustfs/src/storage/ecfs.rs +++ b/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, diff --git a/rustfs/src/storage/ecfs_extend.rs b/rustfs/src/storage/ecfs_extend.rs index c01e41d71..e261cecc1 100644 --- a/rustfs/src/storage/ecfs_extend.rs +++ b/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 { diff --git a/rustfs/src/storage/ecfs_test.rs b/rustfs/src/storage/ecfs_test.rs index 6d4e82ef0..4314e9491 100644 --- a/rustfs/src/storage/ecfs_test.rs +++ b/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("*", "")); + } } diff --git a/scripts/s3-tests/implemented_tests.txt b/scripts/s3-tests/implemented_tests.txt index 0cb4a2830..2737546dd 100644 --- a/scripts/s3-tests/implemented_tests.txt +++ b/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 diff --git a/scripts/s3-tests/non_standard_tests.txt b/scripts/s3-tests/non_standard_tests.txt index c2c9ba1a5..c17dfb9b9 100644 --- a/scripts/s3-tests/non_standard_tests.txt +++ b/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