From 7f98a8a09ac615fd00ae0fd20b700481afe93f3f Mon Sep 17 00:00:00 2001 From: Vadym Dytyniak Date: Fri, 5 May 2023 12:37:25 +0300 Subject: [PATCH 1/2] Do not unquote the key two times on multipart upload complete --- moto/s3/models.py | 8 ++++-- moto/s3/responses.py | 7 ++++- tests/test_s3/test_s3_multipart.py | 41 ++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/moto/s3/models.py b/moto/s3/models.py index e6365dabf46f..930d7625697e 100644 --- a/moto/s3/models.py +++ b/moto/s3/models.py @@ -1888,9 +1888,13 @@ def put_object( return new_key def put_object_acl( - self, bucket_name: str, key_name: str, acl: Optional[FakeAcl] + self, + bucket_name: str, + key_name: str, + acl: Optional[FakeAcl], + key_is_clean: bool = False, ) -> None: - key = self.get_object(bucket_name, key_name) + key = self.get_object(bucket_name, key_name, key_is_clean=key_is_clean) # TODO: Support the XML-based ACL format if key is not None: key.set_acl(acl) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 10270de8ffa2..da42bac69880 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -2173,7 +2173,12 @@ def _key_response_post( ) key.set_metadata(multipart.metadata) self.backend.set_key_tags(key, multipart.tags) - self.backend.put_object_acl(bucket_name, key.name, multipart.acl) + self.backend.put_object_acl( + bucket_name=bucket_name, + key_name=key.name, + acl=multipart.acl, + key_is_clean=True, + ) template = self.response_template(S3_MULTIPART_COMPLETE_RESPONSE) headers: Dict[str, Any] = {} diff --git a/tests/test_s3/test_s3_multipart.py b/tests/test_s3/test_s3_multipart.py index b152c88bad0c..551ca9cd1894 100644 --- a/tests/test_s3/test_s3_multipart.py +++ b/tests/test_s3/test_s3_multipart.py @@ -148,6 +148,47 @@ def test_multipart_upload(): response["Body"].read().should.equal(part1 + part2) +@mock_s3 +@reduced_min_part_size +def test_multipart_upload_with_encoded_char(): + s3 = boto3.resource("s3", region_name=DEFAULT_REGION_NAME) + client = boto3.client("s3", region_name=DEFAULT_REGION_NAME) + s3.create_bucket(Bucket="foobar") + + part1 = b"0" * REDUCED_PART_SIZE + part2 = b"1" + mp = client.create_multipart_upload(Bucket="foobar", Key="the%20key") + up1 = client.upload_part( + Body=BytesIO(part1), + PartNumber=1, + Bucket="foobar", + Key="the%20key", + UploadId=mp["UploadId"], + ) + up2 = client.upload_part( + Body=BytesIO(part2), + PartNumber=2, + Bucket="foobar", + Key="the%20key", + UploadId=mp["UploadId"], + ) + + client.complete_multipart_upload( + Bucket="foobar", + Key="the%20key", + MultipartUpload={ + "Parts": [ + {"ETag": up1["ETag"], "PartNumber": 1}, + {"ETag": up2["ETag"], "PartNumber": 2}, + ] + }, + UploadId=mp["UploadId"], + ) + # we should get both parts as the key contents + response = client.get_object(Bucket="foobar", Key="the%20key") + response["Body"].read().should.equal(part1 + part2) + + @mock_s3 @reduced_min_part_size def test_multipart_upload_out_of_order(): From 9e4298a99c1e44e139a2082d45a860b90ab59321 Mon Sep 17 00:00:00 2001 From: Vadym Dytyniak Date: Fri, 5 May 2023 18:15:20 +0300 Subject: [PATCH 2/2] Parametrize test --- tests/test_s3/test_s3_multipart.py | 54 ++++-------------------------- 1 file changed, 7 insertions(+), 47 deletions(-) diff --git a/tests/test_s3/test_s3_multipart.py b/tests/test_s3/test_s3_multipart.py index 551ca9cd1894..fb511e76afa5 100644 --- a/tests/test_s3/test_s3_multipart.py +++ b/tests/test_s3/test_s3_multipart.py @@ -107,75 +107,35 @@ def test_multipart_upload_too_small(): ) +@pytest.mark.parametrize("key", ["the-key", "the%20key"]) @mock_s3 @reduced_min_part_size -def test_multipart_upload(): +def test_multipart_upload(key: str): s3 = boto3.resource("s3", region_name=DEFAULT_REGION_NAME) client = boto3.client("s3", region_name=DEFAULT_REGION_NAME) s3.create_bucket(Bucket="foobar") part1 = b"0" * REDUCED_PART_SIZE part2 = b"1" - mp = client.create_multipart_upload(Bucket="foobar", Key="the-key") + mp = client.create_multipart_upload(Bucket="foobar", Key=key) up1 = client.upload_part( Body=BytesIO(part1), PartNumber=1, Bucket="foobar", - Key="the-key", - UploadId=mp["UploadId"], - ) - up2 = client.upload_part( - Body=BytesIO(part2), - PartNumber=2, - Bucket="foobar", - Key="the-key", - UploadId=mp["UploadId"], - ) - - client.complete_multipart_upload( - Bucket="foobar", - Key="the-key", - MultipartUpload={ - "Parts": [ - {"ETag": up1["ETag"], "PartNumber": 1}, - {"ETag": up2["ETag"], "PartNumber": 2}, - ] - }, - UploadId=mp["UploadId"], - ) - # we should get both parts as the key contents - response = client.get_object(Bucket="foobar", Key="the-key") - response["Body"].read().should.equal(part1 + part2) - - -@mock_s3 -@reduced_min_part_size -def test_multipart_upload_with_encoded_char(): - s3 = boto3.resource("s3", region_name=DEFAULT_REGION_NAME) - client = boto3.client("s3", region_name=DEFAULT_REGION_NAME) - s3.create_bucket(Bucket="foobar") - - part1 = b"0" * REDUCED_PART_SIZE - part2 = b"1" - mp = client.create_multipart_upload(Bucket="foobar", Key="the%20key") - up1 = client.upload_part( - Body=BytesIO(part1), - PartNumber=1, - Bucket="foobar", - Key="the%20key", + Key=key, UploadId=mp["UploadId"], ) up2 = client.upload_part( Body=BytesIO(part2), PartNumber=2, Bucket="foobar", - Key="the%20key", + Key=key, UploadId=mp["UploadId"], ) client.complete_multipart_upload( Bucket="foobar", - Key="the%20key", + Key=key, MultipartUpload={ "Parts": [ {"ETag": up1["ETag"], "PartNumber": 1}, @@ -185,7 +145,7 @@ def test_multipart_upload_with_encoded_char(): UploadId=mp["UploadId"], ) # we should get both parts as the key contents - response = client.get_object(Bucket="foobar", Key="the%20key") + response = client.get_object(Bucket="foobar", Key=key) response["Body"].read().should.equal(part1 + part2)