From 9ed6e52d0ab86a0bd7b00caeb4af2c2cfb54bf42 Mon Sep 17 00:00:00 2001 From: Antoine Wendlinger Date: Wed, 22 Apr 2020 19:31:43 +0200 Subject: [PATCH 1/2] Handle VersionId in S3:delete_objects VersionId is not read in delete_objects requests, and the behavior differs from its singular counterpart delete_object. This fixes the issue. --- moto/s3/responses.py | 26 +++++++++++++++++--------- tests/test_s3/test_s3.py | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 442489a8a..ec6015f7a 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -840,26 +840,33 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): def _bucket_response_delete_keys(self, request, body, bucket_name): template = self.response_template(S3_DELETE_KEYS_RESPONSE) - keys = minidom.parseString(body).getElementsByTagName("Key") - deleted_names = [] + objects = minidom.parseString(body).getElementsByTagName("Object") + + deleted_objects = [] error_names = [] - if len(keys) == 0: + if len(objects) == 0: raise MalformedXML() - for k in keys: - key_name = k.firstChild.nodeValue + for object_ in objects: + key_name = object_.getElementsByTagName("Key")[0].firstChild.nodeValue + version_id_node = object_.getElementsByTagName("VersionId") + if version_id_node: + version_id = version_id_node[0].firstChild.nodeValue + else: + version_id = None + success = self.backend.delete_key( - bucket_name, undo_clean_key_name(key_name) + bucket_name, undo_clean_key_name(key_name), version_id=version_id ) if success: - deleted_names.append(key_name) + deleted_objects.append((key_name, version_id)) else: error_names.append(key_name) return ( 200, {}, - template.render(deleted=deleted_names, delete_errors=error_names), + template.render(deleted=deleted_objects, delete_errors=error_names), ) def _handle_range_header(self, request, headers, response_content): @@ -1861,9 +1868,10 @@ S3_BUCKET_GET_VERSIONS = """ S3_DELETE_KEYS_RESPONSE = """ -{% for k in deleted %} +{% for k, v in deleted %} {{k}} +{% if v %}{{v}}{% endif %} {% endfor %} {% for k in delete_errors %} diff --git a/tests/test_s3/test_s3.py b/tests/test_s3/test_s3.py index ffbd73966..4a94c9c38 100644 --- a/tests/test_s3/test_s3.py +++ b/tests/test_s3/test_s3.py @@ -2218,6 +2218,29 @@ def test_boto3_deleted_versionings_list(): assert len(listed["Contents"]) == 1 +@mock_s3 +def test_boto3_delete_objects_for_specific_version_id(): + client = boto3.client("s3", region_name=DEFAULT_REGION_NAME) + client.create_bucket(Bucket="blah") + client.put_bucket_versioning( + Bucket="blah", VersioningConfiguration={"Status": "Enabled"} + ) + + client.put_object(Bucket="blah", Key="test1", Body=b"test1a") + client.put_object(Bucket="blah", Key="test1", Body=b"test1b") + + response = client.list_object_versions(Bucket="blah", Prefix="test1") + id_to_delete = [v["VersionId"] for v in response["Versions"] if v["IsLatest"]][0] + + response = client.delete_objects( + Bucket="blah", Delete={"Objects": [{"Key": "test1", "VersionId": id_to_delete}]} + ) + assert response["Deleted"] == [{"Key": "test1", "VersionId": id_to_delete}] + + listed = client.list_objects_v2(Bucket="blah") + assert len(listed["Contents"]) == 1 + + @mock_s3 def test_boto3_delete_versioned_bucket(): client = boto3.client("s3", region_name=DEFAULT_REGION_NAME) From 41abd4344bdcb2dfd5dd4fcebf9bc6be325c052e Mon Sep 17 00:00:00 2001 From: Antoine Wendlinger Date: Mon, 27 Apr 2020 11:42:27 +0200 Subject: [PATCH 2/2] Use xmltodict for parsing --- moto/s3/responses.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index ec6015f7a..fa6f8e568 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -839,21 +839,22 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): def _bucket_response_delete_keys(self, request, body, bucket_name): template = self.response_template(S3_DELETE_KEYS_RESPONSE) + body_dict = xmltodict.parse(body) - objects = minidom.parseString(body).getElementsByTagName("Object") - - deleted_objects = [] - error_names = [] + objects = body_dict["Delete"].get("Object", []) + if not isinstance(objects, list): + # We expect a list of objects, but when there is a single node xmltodict does not + # return a list. + objects = [objects] if len(objects) == 0: raise MalformedXML() + deleted_objects = [] + error_names = [] + for object_ in objects: - key_name = object_.getElementsByTagName("Key")[0].firstChild.nodeValue - version_id_node = object_.getElementsByTagName("VersionId") - if version_id_node: - version_id = version_id_node[0].firstChild.nodeValue - else: - version_id = None + key_name = object_["Key"] + version_id = object_.get("VersionId", None) success = self.backend.delete_key( bucket_name, undo_clean_key_name(key_name), version_id=version_id