From 650fa986799a9cceaf6d3a036633fe49515eabf5 Mon Sep 17 00:00:00 2001
From: Naoto Nishizono <nishizono.naoto@po.ntts.co.jp>
Date: Mon, 22 Dec 2014 17:42:51 +0900
Subject: [PATCH] Fix response of DELETE Multiple Objects without bucket write
 permission

S3 returns 200 status code and following xml body, even if it does not have
bucket write permission.

<?xml version="1.0" encoding="UTF-8"?>
<DeleteResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
  <Error>
    <Key>sample1</Key>
    <Code>AccessDenied</Code>
    <Message>Access Denied</Message>
  </Error>
  <Error>
    <Key>sample2</Key>
    <Code>AccessDenied</Code>
    <Message>Access Denied</Message>
  </Error>
</DeleteResult>

Change-Id: I75d5543ee35d108c9b3d037c8dfb3a96fc65b634
---
 swift3/controllers/multi_delete.py    | 25 +++++++++++++++++++++----
 swift3/test/unit/test_multi_delete.py | 26 ++++++++++++++++++--------
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/swift3/controllers/multi_delete.py b/swift3/controllers/multi_delete.py
index 647a5887..552f9bde 100644
--- a/swift3/controllers/multi_delete.py
+++ b/swift3/controllers/multi_delete.py
@@ -17,7 +17,7 @@ from swift3.controllers.base import Controller, bucket_operation
 from swift3.etree import Element, SubElement, fromstring, tostring, \
     XMLSyntaxError, DocumentInvalid
 from swift3.response import HTTPOk, S3NotImplemented, NoSuchKey, \
-    ErrorResponse, MalformedXML, UserKeyMustBeSpecified
+    ErrorResponse, MalformedXML, UserKeyMustBeSpecified, AccessDenied
 from swift3.cfg import CONF
 from swift3.utils import LOGGER
 
@@ -29,6 +29,19 @@ class MultiObjectDeleteController(Controller):
     Handles Delete Multiple Objects, which is logged as a MULTI_OBJECT_DELETE
     operation in the S3 server log.
     """
+    def _gen_error_body(self, error, elem, delete_list):
+        for key, version in delete_list:
+            if version is not None:
+                # TODO: delete the specific version of the object
+                raise S3NotImplemented()
+
+            error_elem = SubElement(elem, 'Error')
+            SubElement(error_elem, 'Key').text = key
+            SubElement(error_elem, 'Code').text = error.__class__.__name__
+            SubElement(error_elem, 'Message').text = error._msg
+
+        return tostring(elem)
+
     @bucket_operation
     def POST(self, req):
         """
@@ -45,9 +58,6 @@ class MultiObjectDeleteController(Controller):
 
                 yield key, version
 
-        # check bucket existence
-        req.get_response(self.app, 'HEAD')
-
         try:
             xml = req.xml(MAX_MULTI_DELETE_BODY_SIZE, check_md5=True)
             elem = fromstring(xml, 'Delete')
@@ -71,6 +81,13 @@ class MultiObjectDeleteController(Controller):
 
         elem = Element('DeleteResult')
 
+        # check bucket existence
+        try:
+            req.get_response(self.app, 'HEAD')
+        except AccessDenied as error:
+            body = self._gen_error_body(error, elem, delete_list)
+            return HTTPOk(body=body)
+
         for key, version in delete_list:
             if version is not None:
                 # TODO: delete the specific version of the object
diff --git a/swift3/test/unit/test_multi_delete.py b/swift3/test/unit/test_multi_delete.py
index 2a91ee91..b3113495 100644
--- a/swift3/test/unit/test_multi_delete.py
+++ b/swift3/test/unit/test_multi_delete.py
@@ -173,13 +173,16 @@ class TestSwift3MultiDelete(Swift3TestCase):
         self.assertEquals(self._get_error_code(body), 'MalformedXML')
 
     def _test_object_multi_DELETE(self, account):
-        self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key1',
-                            swob.HTTPNoContent, {}, None)
-        self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key2',
-                            swob.HTTPNotFound, {}, None)
+        self.keys = ['Key1', 'Key2']
+        self.swift.register(
+            'DELETE', '/v1/AUTH_test/bucket/%s' % self.keys[0],
+            swob.HTTPNoContent, {}, None)
+        self.swift.register(
+            'DELETE', '/v1/AUTH_test/bucket/%s' % self.keys[1],
+            swob.HTTPNotFound, {}, None)
 
         elem = Element('Delete')
-        for key in ['Key1', 'Key2']:
+        for key in self.keys:
             obj = SubElement(elem, 'Object')
             SubElement(obj, 'Key').text = key
         body = tostring(elem, use_s3ns=False)
@@ -198,14 +201,21 @@ class TestSwift3MultiDelete(Swift3TestCase):
     @s3acl(s3acl_only=True)
     def test_object_multi_DELETE_without_permission(self):
         status, headers, body = self._test_object_multi_DELETE('test:other')
-        self.assertEquals(self._get_error_code(body), 'AccessDenied')
+        self.assertEquals(status.split()[0], '200')
+        elem = fromstring(body)
+        errors = elem.findall('Error')
+        self.assertEquals(len(errors), len(self.keys))
+        for e in errors:
+            self.assertTrue(e.find('Key').text in self.keys)
+            self.assertEquals(e.find('Code').text, 'AccessDenied')
+            self.assertEquals(e.find('Message').text, 'Access Denied.')
 
     @s3acl(s3acl_only=True)
     def test_object_multi_DELETE_with_write_permission(self):
         status, headers, body = self._test_object_multi_DELETE('test:write')
         self.assertEquals(status.split()[0], '200')
         elem = fromstring(body)
-        self.assertEquals(len(elem.findall('Deleted')), 2)
+        self.assertEquals(len(elem.findall('Deleted')), len(self.keys))
 
     @s3acl(s3acl_only=True)
     def test_object_multi_DELETE_with_fullcontrol_permission(self):
@@ -213,7 +223,7 @@ class TestSwift3MultiDelete(Swift3TestCase):
             self._test_object_multi_DELETE('test:full_control')
         self.assertEquals(status.split()[0], '200')
         elem = fromstring(body)
-        self.assertEquals(len(elem.findall('Deleted')), 2)
+        self.assertEquals(len(elem.findall('Deleted')), len(self.keys))
 
 if __name__ == '__main__':
     unittest.main()