From 82f9771af91cac6d0699d26f997e14328532a9ec Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Sat, 11 Nov 2023 00:30:53 +0100 Subject: [PATCH] fix S3 parser with empty integer query string parameters --- localstack/aws/protocol/parser.py | 8 +++ tests/aws/services/s3/test_s3.py | 44 +++++++++++++- tests/aws/services/s3/test_s3.snapshot.json | 65 +++++++++++++++++++++ 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/localstack/aws/protocol/parser.py b/localstack/aws/protocol/parser.py index 07b62538fc5ae..c0237d81193c1 100644 --- a/localstack/aws/protocol/parser.py +++ b/localstack/aws/protocol/parser.py @@ -1076,6 +1076,14 @@ def _parse_shape( uri_params["Key"] = uri_params["Key"] + "/" return super()._parse_shape(request, shape, node, uri_params) + @_text_content + def _parse_integer(self, _, shape, node: str, ___) -> int | None: + # S3 accepts empty query string parameters that should be integer + # to not break other cases, validate that the shape is in the querystring + if node == "" and shape.serialization.get("location") == "querystring": + return None + return int(node) + class SQSQueryRequestParser(QueryRequestParser): def _get_serialized_name(self, shape: Shape, default_name: str, node: dict) -> str: diff --git a/tests/aws/services/s3/test_s3.py b/tests/aws/services/s3/test_s3.py index 9e85e77320bb9..9f39c360cbf50 100644 --- a/tests/aws/services/s3/test_s3.py +++ b/tests/aws/services/s3/test_s3.py @@ -837,7 +837,7 @@ def test_list_objects_next_marker(self, s3_bucket, snapshot, aws_client): snapshot.match("list-objects-marker-empty", resp) @markers.aws.validated - @pytest.mark.xfail(condition=is_v2_provider, reason="not implemented in moto") + @pytest.mark.xfail(condition=is_v2_provider(), reason="not implemented in moto") def test_list_multiparts_next_marker(self, s3_bucket, snapshot, aws_client): snapshot.add_transformer(snapshot.transform.s3_api()) snapshot.add_transformers_list( @@ -943,7 +943,7 @@ def test_list_multiparts_next_marker(self, s3_bucket, snapshot, aws_client): snapshot.match("list-multiparts-next-key-empty", response) @markers.aws.validated - @pytest.mark.xfail(condition=is_v2_provider, reason="not implemented in moto") + @pytest.mark.xfail(condition=is_v2_provider(), reason="not implemented in moto") def test_list_multiparts_with_prefix_and_delimiter( self, s3_bucket, snapshot, aws_client, aws_http_client_factory ): @@ -989,7 +989,7 @@ def test_list_multiparts_with_prefix_and_delimiter( resp_dict["ListMultipartUploadsResult"].pop("@xmlns", None) snapshot.match("list-multiparts-no-encoding", resp_dict) - @pytest.mark.xfail(condition=is_v2_provider, reason="not implemented in moto") + @pytest.mark.xfail(condition=is_v2_provider(), reason="not implemented in moto") @markers.aws.validated def test_list_parts_pagination(self, s3_bucket, snapshot, aws_client): snapshot.add_transformer( @@ -1045,6 +1045,44 @@ def test_list_parts_pagination(self, s3_bucket, snapshot, aws_client): ) snapshot.match("list-parts-wrong-part", response) + @pytest.mark.xfail( + condition=is_v2_provider(), reason="moto does not handle empty query string parameters" + ) + @markers.aws.validated + def test_list_parts_empty_part_number_marker(self, s3_bucket, snapshot, aws_client_factory): + # we need to disable validation for this test + s3_client = aws_client_factory(config=Config(parameter_validation=False)).s3 + snapshot.add_transformer( + [ + snapshot.transform.key_value("Bucket", reference_replacement=False), + snapshot.transform.key_value("Location"), + snapshot.transform.key_value("UploadId"), + snapshot.transform.key_value("DisplayName", reference_replacement=False), + snapshot.transform.key_value("ID", reference_replacement=False), + ] + ) + object_key = "test-list-part-empty-marker" + response = s3_client.create_multipart_upload(Bucket=s3_bucket, Key=object_key) + upload_id = response["UploadId"] + + s3_client.upload_part( + Bucket=s3_bucket, + Key=object_key, + Body=BytesIO(b"data"), + PartNumber=1, + UploadId=upload_id, + ) + # it seems S3 does not care about empty string for integer query string parameters + response = s3_client.list_parts( + Bucket=s3_bucket, UploadId=upload_id, Key=object_key, PartNumberMarker="" + ) + snapshot.match("list-parts-empty-marker", response) + + response = s3_client.list_parts( + Bucket=s3_bucket, UploadId=upload_id, Key=object_key, MaxParts="" + ) + snapshot.match("list-parts-empty-max-parts", response) + @markers.aws.validated def test_get_object_no_such_bucket(self, snapshot, aws_client): snapshot.add_transformer(snapshot.transform.key_value("BucketName")) diff --git a/tests/aws/services/s3/test_s3.snapshot.json b/tests/aws/services/s3/test_s3.snapshot.json index c385d4089427a..4d7fdcca0b842 100644 --- a/tests/aws/services/s3/test_s3.snapshot.json +++ b/tests/aws/services/s3/test_s3.snapshot.json @@ -12236,5 +12236,70 @@ } } } + }, + "tests/aws/services/s3/test_s3.py::TestS3::test_list_parts_empty_part_number_marker": { + "recorded-date": "11-11-2023, 00:20:09", + "recorded-content": { + "list-parts-empty-marker": { + "Bucket": "bucket", + "Initiator": { + "DisplayName": "display-name", + "ID": "i-d" + }, + "IsTruncated": false, + "Key": "test-list-part-empty-marker", + "MaxParts": 1000, + "NextPartNumberMarker": 1, + "Owner": { + "DisplayName": "display-name", + "ID": "i-d" + }, + "PartNumberMarker": 0, + "Parts": [ + { + "ETag": "\"8d777f385d3dfec8815d20f7496026dc\"", + "LastModified": "datetime", + "PartNumber": 1, + "Size": 4 + } + ], + "StorageClass": "STANDARD", + "UploadId": "", + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 200 + } + }, + "list-parts-empty-max-parts": { + "Bucket": "bucket", + "Initiator": { + "DisplayName": "display-name", + "ID": "i-d" + }, + "IsTruncated": false, + "Key": "test-list-part-empty-marker", + "MaxParts": 1000, + "NextPartNumberMarker": 1, + "Owner": { + "DisplayName": "display-name", + "ID": "i-d" + }, + "PartNumberMarker": 0, + "Parts": [ + { + "ETag": "\"8d777f385d3dfec8815d20f7496026dc\"", + "LastModified": "datetime", + "PartNumber": 1, + "Size": 4 + } + ], + "StorageClass": "STANDARD", + "UploadId": "", + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 200 + } + } + } } }