fix(s3): implement S3-compliant CORS and bucket existence checks#2026
Merged
fix(s3): implement S3-compliant CORS and bucket existence checks#2026
Conversation
- Refactor CORS middleware to use bucket-level CORS configuration for S3 paths instead of generic hardcoded methods. OPTIONS preflight returns 400 for missing headers, 403 for non-matching rules. - Support Access-Control-Request-Method header on all requests (not just preflight) for CORS rule matching, matching AWS S3 behavior. - Return NoSuchBucket (404) instead of AccessDenied (403) for DeleteObject on non-existent buckets by checking bucket existence before authorization. - Handle bucket-not-found in get_object_tag_conditions_for_policy to return NoSuchBucket instead of AccessDenied. - Add unit tests for CORS origin pattern matching, S3 path detection, and generic CORS layer behavior. - Move 14 newly passing tests from non_standard to implemented list (379 → 393 total). Made-with: Cursor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves S3 compatibility by aligning CORS behavior with S3 bucket-level CORS configuration and correcting error codes for bucket-not-found scenarios (notably for DeleteObject and policy tag condition evaluation). It also updates the s3-tests allowlist/non-standard lists to reflect newly supported tests and adds unit coverage for CORS matching/path detection.
Changes:
- Refactor server CORS handling to apply bucket-level CORS rules for S3 paths (including OPTIONS preflight) while keeping generic CORS for admin/console paths.
- Adjust S3 error mapping to return
NoSuchBucketwhen appropriate (DeleteObject pre-check, tag condition evaluation). - Promote previously non-standard s3-tests to implemented and add unit tests for CORS origin wildcard matching and S3-path detection.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/s3-tests/non_standard_tests.txt | Removes now-supported CORS/HTTP/DeleteObject tests from the non-standard list. |
| scripts/s3-tests/implemented_tests.txt | Adds newly supported tests to the implemented list and updates summary comments. |
| rustfs/src/storage/ecfs_test.rs | Adds unit tests for matches_origin_pattern wildcard behaviors. |
| rustfs/src/storage/ecfs_extend.rs | Updates CORS method matching to prefer Access-Control-Request-Method when present. |
| rustfs/src/storage/ecfs.rs | Maps bucket_not_found during tag-condition fetching to NoSuchBucket. |
| rustfs/src/storage/access.rs | Adds a bucket existence check before authorization in delete_object. |
| rustfs/src/server/layer.rs | Refactors conditional CORS service behavior + adds tests for S3 path detection and generic CORS behavior. |
- Only return NoSuchBucket for actual bucket_not_found errors (not transient storage errors) using is_err_bucket_not_found check. - Remove eager path_trimmed allocation; compute bucket segment lazily inside the CORS branch to reduce per-request overhead. Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Access-Control-Request-Methodheader for CORS matching.NoSuchBucket(404) instead ofAccessDenied(403) when deleting from a non-existent bucket, by checking bucket existence before authorization.bucket_not_foundinget_object_tag_conditions_for_policyto returnNoSuchBucketinstead ofAccessDenied.Changes
rustfs/src/server/layer.rsConditionalCorsServiceto use bucket-level CORS for S3 paths; generic CORS only for admin/consolerustfs/src/storage/ecfs_extend.rsAccess-Control-Request-Methodfor all requests, not just preflightrustfs/src/storage/access.rsdelete_objectrustfs/src/storage/ecfs.rsbucket_not_foundin tag conditionsrustfs/src/storage/ecfs_test.rsrustfs/src/server/layer.rs(tests)Test plan
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_acltest_100_continue,test_abort_multipart_upload_not_found,test_post_object_tags_authenticated_requesttest_object_delete_key_bucket_gonecargo fmt --all --checkpassescargo clippy --all-targets --all-features -- -D warningspassescargo test --workspace --exclude e2e_testpassesMade with Cursor