Skip to content

fix ceph CORS tests failure #8968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,4 @@ s3tests_boto3/functional/test_sts.py::test_assume_role_with_web_identity_resourc
s3tests_boto3/functional/test_sts.py::test_assume_role_with_web_identity_wrong_resource_tag_deny
s3tests_boto3/functional/test_sts.py::test_assume_role_with_web_identity_resource_tag_princ_tag
s3tests_boto3/functional/test_sts.py::test_assume_role_with_web_identity_resource_tag_copy_obj
s3tests_boto3/functional/test_sts.py::test_assume_role_with_web_identity_role_resource_tag
s3tests_boto3/functional/test_s3.py::test_cors_presigned_put_object_with_acl
s3tests_boto3/functional/test_s3.py::test_cors_presigned_put_object_tenant_with_acl
s3tests_boto3/functional/test_sts.py::test_assume_role_with_web_identity_role_resource_tag
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ s3tests/functional/test_headers.py::test_object_create_bad_date_none_aws2
s3tests/functional/test_headers.py::test_bucket_create_bad_authorization_invalid_aws2
s3tests/functional/test_headers.py::test_bucket_create_bad_date_none_aws2
s3tests_boto3/functional/test_s3.py::test_post_object_wrong_bucket
s3tests_boto3/functional/test_s3.py::test_cors_presigned_put_object_tenant
s3tests_boto3/functional/test_s3.py::test_object_presigned_put_object_with_acl_tenant
s3tests_boto3/functional/test_s3.py::test_lifecycle_expiration_newer_noncurrent
s3tests_boto3/functional/test_s3.py::test_lifecycle_expiration_size_gt
Expand Down
4 changes: 2 additions & 2 deletions src/util/http_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -719,8 +719,8 @@ function set_cors_headers_s3(req, res, cors_rules) {
// https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html
dbg.log0('set_cors_headers_s3: found matching CORS rule:', matched_rule);
set_cors_headers(req, res, {
allow_origin: matched_rule.allowed_origins.includes('*') ? '*' : req.headers.origin,
allow_methods: matched_rule.allowed_methods.join(','),
allow_origin: match_origin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not correct. in case you have * in the allowed_origin list - you need to return * and not the sent origin - see here:

curl -X GET "${ENDPOINT}" \
  -H "Host: ${BUCKET_NAME}.s3.${AWS_REGION}.amazonaws.com" \
  -H "Date: ${DATE_NOW}" \
  -H "Authorization: AWS ${ACCESS_KEY}:${SIGNATURE}" \
  -H "Origin: https://allowed-origin.com" \
  -D - \
  --silent
HTTP/1.1 404 Not Found
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, POST, PUT, DELETE
Access-Control-Expose-Headers: ETag, X-Amz-Version-Id
Vary: Origin, Access-Control-Request-Headers, Access-Control-Request-Method
x-amz-request-id: RVP2Z31CTC8AH42M
x-amz-id-2: YmZDrdYXE2wG/tcQ6qgCqGdF26qu2hDpTtVKd4vJF2vTUoPWZGHho5Cy061FVvuK67sRHq2nChc=
Content-Type: application/xml
Transfer-Encoding: chunked
Date: Thu, 10 Apr 2025 12:06:44 GMT
Server: AmazonS3```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is tested with AWS. let me know if you need the script for running request with AWS bucket

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also see your change cause another s3_test to fail about CORS:
test_cors_origin_wildcard
please check in advance all of the tests are passing.

allow_methods: match_method,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also not correct - take a look at the above example
Allow-Methods in the rule are: GET, POST, PUT, DELETE
, and match_method is GET

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this was not the correct solution to the problem.
Will revert back. I have found more issues and will discuss it with you.

allow_headers: matched_rule.allowed_headers?.join(','),
expose_headers: matched_rule.expose_headers?.join(','),
allow_credentials: 'true',
Expand Down
Loading