Skip to content

Commit 49d3db4

Browse files
bentskubentsku
committed
fix nits
1 parent 5183436 commit 49d3db4

File tree

5 files changed

+36
-24
lines changed

5 files changed

+36
-24
lines changed

‎localstack/services/s3/cors.py‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,25 @@ def __init__(self):
3838
def __call__(self, chain: HandlerChain, context: RequestContext, response: Response):
3939
self.handle_cors(chain, context, response)
4040

41-
def pre_parse_s3_request(self, context: RequestContext) -> Tuple[bool, Optional[str]]:
41+
def pre_parse_s3_request(self, request: Request) -> Tuple[bool, Optional[str]]:
42+
"""
43+
Parse the request to try to determine if it's directed towards S3. It tries to match on host, then check
44+
if the targeted bucket exists. If we could not determine it was a s3 request from the host, but the first
45+
element in the path contains an existing bucket, we can think it's S3.
46+
:param request: Request from the context
47+
:return: is_s3, whether we're certain it's a s3 request, and bucket_name if the bucket exists
48+
"""
4249
is_s3: bool
4350
bucket_name: str
4451

45-
path = context.request.path
46-
host = context.request.host
52+
path = request.path
53+
host = request.host
4754

4855
# first, try to figure out best-effort whether the request is an s3 request
4956
if host.startswith(S3_VIRTUAL_HOSTNAME):
5057
is_s3 = True
5158
bucket_name = path.split("/")[1]
52-
# try to extract the bucket from the hostname (the "in" check is a minor optimization
59+
# try to extract the bucket from the hostname (the "in" check is a minor optimization)
5360
elif ".s3" in host and (match := _s3_virtual_host_regex.match(host)):
5461
is_s3 = True
5562
bucket_name = match.group(3)
@@ -76,21 +83,23 @@ def handle_cors(self, chain: HandlerChain, context: RequestContext, response: Re
7683
if config.LEGACY_S3_PROVIDER or config.DISABLE_CUSTOM_CORS_S3:
7784
return
7885

79-
is_s3, bucket_name = self.pre_parse_s3_request(context)
86+
request = context.request
87+
is_s3, bucket_name = self.pre_parse_s3_request(context.request)
8088

81-
if not is_s3 and not bucket_name:
89+
if not is_s3:
8290
# continue the chain, let the default CORS handler take care of the request
8391
return
8492

8593
# set the service so that the regular CORS enforcer knows it needs to ignore this request
8694
context.service = self._service
8795

88-
request = context.request
8996
is_options_request = request.method == "OPTIONS"
9097

9198
def stop_options_chain():
92-
# FIXME: need to add it there as not handled by the serializer, we stop the chain to avoid the request
93-
# being parsed
99+
"""
100+
Stops the chain to avoid the OPTIONS request being parsed. The request is ready to be returned to the
101+
client. We also need to add specific headers normally added by the serializer for regular requests.
102+
"""
94103
request_id = gen_amzn_requestid_long()
95104
response.headers["x-amz-request-id"] = request_id
96105
response.headers[
@@ -134,11 +143,8 @@ def stop_options_chain():
134143
message = "CORSResponse: Bucket not found"
135144
else:
136145
message = "CORSResponse: CORS is not enabled for this bucket."
137-
try:
138-
context.operation = self._get_op_from_request(request)
139-
except Exception:
140-
context.operation = context.service.operation_model("GetObject")
141146

147+
context.operation = self._get_op_from_request(request)
142148
raise AccessForbidden(
143149
message, HostId=FAKE_HOST_ID, Method="OPTIONS", ResourceType="BUCKET"
144150
)
@@ -147,11 +153,6 @@ def stop_options_chain():
147153
return
148154

149155
rules = self.bucket_cors_index.cors[bucket_name]["CORSRules"]
150-
# check this but should not happen? maybe?
151-
if not rules:
152-
if is_options_request:
153-
stop_options_chain()
154-
return
155156

156157
if not (rule := self.match_rules(request, rules)):
157158
if is_options_request:

‎localstack/services/s3/provider.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -609,8 +609,6 @@ def put_bucket_cors(
609609
expected_bucket_owner: AccountId = None,
610610
) -> None:
611611
response = call_moto(context)
612-
# max 100 rules
613-
# validate CORS? see moto
614612
self.get_store().bucket_cors[bucket] = cors_configuration
615613
self._cors_handler.invalidate_cache()
616614
return response

‎localstack/services/s3/s3_listener.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -638,9 +638,7 @@ def append_cors_headers(
638638

639639
for allowed in allowed_origins:
640640
allowed = allowed or ""
641-
if origin in allowed or re.match(
642-
allowed.replace("*", ".*"), origin
643-
): # no wildcard in origin?
641+
if origin in allowed or re.match(allowed.replace("*", ".*"), origin):
644642

645643
response.headers["Access-Control-Allow-Origin"] = origin
646644
if "AllowedMethod" in rule:

‎tests/integration/s3/test_s3_cors.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,11 @@ def test_put_cors_invalid_rules(self, s3_client, s3_bucket, snapshot):
588588

589589
snapshot.match("put-cors-exc", e.value.response)
590590

591+
with pytest.raises(ClientError) as e:
592+
s3_client.put_bucket_cors(Bucket=s3_bucket, CORSConfiguration={"CORSRules": []})
593+
594+
snapshot.match("put-cors-exc-empty", e.value.response)
595+
591596
@pytest.mark.aws_validated
592597
def test_put_cors_empty_origin(self, s3_client, s3_bucket, snapshot):
593598
# derived from TestAccS3Bucket_Security_corsEmptyOrigin TF test

‎tests/integration/s3/test_s3_cors.snapshot.json‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@
525525
}
526526
},
527527
"tests/integration/s3/test_s3_cors.py::TestS3Cors::test_put_cors_invalid_rules": {
528-
"recorded-date": "24-10-2022, 14:39:40",
528+
"recorded-date": "28-10-2022, 15:57:45",
529529
"recorded-content": {
530530
"put-cors-exc": {
531531
"Error": {
@@ -536,6 +536,16 @@
536536
"HTTPHeaders": {},
537537
"HTTPStatusCode": 400
538538
}
539+
},
540+
"put-cors-exc-empty": {
541+
"Error": {
542+
"Code": "MalformedXML",
543+
"Message": "The XML you provided was not well-formed or did not validate against our published schema"
544+
},
545+
"ResponseMetadata": {
546+
"HTTPHeaders": {},
547+
"HTTPStatusCode": 400
548+
}
539549
}
540550
}
541551
},

0 commit comments

Comments
 (0)