Allow multipart uploads to have an empty final part

Prior to Swift commit 7f636a557296ecc6ae4727700cfcf9f82573bd16, SLO
would allow large objects to include zero-byte segments if they were the
last segment in the manifest. This was consistent with AWS's behavior
during multipart uploads.

Now, however, SLO requires that every segment be at least one byte. This
commit once more allows zero-length final parts by removing such parts
from the manifest and cleaning up the already-uploaded segment.

Additionally, if the multipart upload consisted of a single zero-length
part (which is apparently allowed via S3), an ordinary object will be
created instead of a SLO.

Change-Id: I06750b2ffcb2a90bee44fa366afda4520705f09c

swift3/controllers/multi_upload.py

@@ -551,11 +551,31 @@ class UploadController(Controller):
             LOGGER.error(e)
             raise exc_type, exc_value, exc_traceback
 
+        # Following swift commit 7f636a5, zero-byte segments aren't allowed,
+        # even as the final segment
+        if int(info['size_bytes']) == 0:
+            manifest.pop()
+
+            # Ordinarily, we just let SLO check segment sizes. However, we
+            # just popped off a zero-byte segment; if there was a second
+            # zero-byte segment and it was at the end, it would succeed on
+            # Swift < 2.6.0 and fail on newer Swift. It seems reasonable that
+            # it should always fail.
+            if manifest and int(manifest[-1]['size_bytes']) == 0:
+                raise EntityTooSmall()
+
         try:
             # TODO: add support for versioning
-            resp = req.get_response(self.app, 'PUT', body=json.dumps(manifest),
-                                    query={'multipart-manifest': 'put'},
-                                    headers=headers)
+            if manifest:
+                resp = req.get_response(self.app, 'PUT',
+                                        body=json.dumps(manifest),
+                                        query={'multipart-manifest': 'put'},
+                                        headers=headers)
+            else:
+                # the upload must have consisted of a single zero-length part
+                # just write it directly
+                resp = req.get_response(self.app, 'PUT', body='',
+                                        headers=headers)
         except BadSwiftRequest as e:
             msg = str(e)
             if msg.startswith('Each segment, except the last, '
@@ -567,6 +587,13 @@ class UploadController(Controller):
             else:
                 raise
 
+        if int(info['size_bytes']) == 0:
+            # clean up the zero-byte segment
+            empty_seg_cont, empty_seg_name = info['path'].split('/', 2)[1:]
+            req.get_response(self.app, 'DELETE',
+                             container=empty_seg_cont, obj=empty_seg_name)
+
+        # clean up the multipart-upload record
         obj = '%s/%s' % (req.object_name, upload_id)
         req.get_response(self.app, 'DELETE', container, obj)
 

swift3/test/functional/test_multi_upload.py

@@ -494,17 +494,26 @@ class TestSwift3MultiUpload(Swift3FunctionalTestCase):
             etags.append(headers['etag'])
         xml = self._gen_comp_xml(etags)
 
+        # part 1 too small
         query = 'uploadId=%s' % upload_id
+        status, headers, body = \
+            self.conn.make_request('POST', bucket, keys[0], body=xml,
+                                   query=query)
+        self.assertEquals(get_error_code(body), 'EntityTooSmall')
+
+        # invalid credentials
         auth_error_conn = Connection(aws_secret_key='invalid')
         status, headers, body = \
             auth_error_conn.make_request('POST', bucket, keys[0], body=xml,
                                          query=query)
         self.assertEquals(get_error_code(body), 'SignatureDoesNotMatch')
 
+        # wrong/missing bucket
         status, headers, body = \
             self.conn.make_request('POST', 'nothing', keys[0], query=query)
         self.assertEquals(get_error_code(body), 'NoSuchBucket')
 
+        # wrong upload ID
         query = 'uploadId=%s' % 'nothing'
         status, headers, body = \
             self.conn.make_request('POST', bucket, keys[0], body=xml,

swift3/test/unit/test_multi_upload.py

@@ -77,8 +77,7 @@ class TestSwift3MultiUpload(Swift3TestCase):
                       objects_template)
         object_list = json.dumps(objects)
 
-        self.swift.register('PUT',
-                            '/v1/AUTH_test/bucket+segments',
+        self.swift.register('PUT', segment_bucket,
                             swob.HTTPAccepted, {}, None)
         self.swift.register('GET', segment_bucket, swob.HTTPOk, {},
                             object_list)
@@ -635,6 +634,111 @@ class TestSwift3MultiUpload(Swift3TestCase):
         self.assertEquals(headers.get('X-Object-Meta-Foo'), 'bar')
         self.assertEquals(headers.get('Content-Type'), 'baz/quux')
 
+    def test_object_multipart_upload_complete_single_zero_length_segment(self):
+        segment_bucket = '/v1/AUTH_test/empty-bucket+segments'
+        put_headers = {'etag': self.etag, 'last-modified': self.last_modified}
+
+        object_list = [{
+            'name': 'object/X/1',
+            'last_modified': self.last_modified,
+            'hash': 'd41d8cd98f00b204e9800998ecf8427e',
+            'bytes': '0',
+        }]
+
+        self.swift.register('GET', segment_bucket, swob.HTTPOk, {},
+                            json.dumps(object_list))
+        self.swift.register('HEAD', '/v1/AUTH_test/empty-bucket',
+                            swob.HTTPNoContent, {}, None)
+        self.swift.register('HEAD', segment_bucket + '/object/X',
+                            swob.HTTPOk, {'x-object-meta-foo': 'bar',
+                                          'content-type': 'baz/quux'}, None)
+        self.swift.register('PUT', '/v1/AUTH_test/empty-bucket/object',
+                            swob.HTTPCreated, {}, None)
+        self.swift.register('DELETE', segment_bucket + '/object/X/1',
+                            swob.HTTPOk, {}, None)
+        self.swift.register('DELETE', segment_bucket + '/object/X',
+                            swob.HTTPOk, {}, None)
+
+        xml = '<CompleteMultipartUpload>' \
+            '<Part>' \
+            '<PartNumber>1</PartNumber>' \
+            '<ETag>d41d8cd98f00b204e9800998ecf8427e</ETag>' \
+            '</Part>' \
+            '</CompleteMultipartUpload>'
+
+        req = Request.blank('/empty-bucket/object?uploadId=X',
+                            environ={'REQUEST_METHOD': 'POST'},
+                            headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Date': self.get_date_header(), },
+                            body=xml)
+        status, headers, body = self.call_swift3(req)
+        fromstring(body, 'CompleteMultipartUploadResult')
+        self.assertEquals(status.split()[0], '200')
+
+        self.assertEqual(self.swift.calls, [
+            ('HEAD', '/v1/AUTH_test/empty-bucket'),
+            ('HEAD', '/v1/AUTH_test/empty-bucket+segments/object/X'),
+            ('GET', '/v1/AUTH_test/empty-bucket+segments?delimiter=/&'
+                    'format=json&prefix=object/X/'),
+            # note the lack of multipart-manifest=put below
+            ('PUT', '/v1/AUTH_test/empty-bucket/object'),
+            ('DELETE', '/v1/AUTH_test/empty-bucket+segments/object/X/1'),
+            ('DELETE', '/v1/AUTH_test/empty-bucket+segments/object/X'),
+        ])
+        _, _, put_headers = self.swift.calls_with_headers[-3]
+        self.assertEquals(put_headers.get('X-Object-Meta-Foo'), 'bar')
+        self.assertEquals(put_headers.get('Content-Type'), 'baz/quux')
+
+    def test_object_multipart_upload_complete_double_zero_length_segment(self):
+        segment_bucket = '/v1/AUTH_test/empty-bucket+segments'
+
+        object_list = [{
+            'name': 'object/X/1',
+            'last_modified': self.last_modified,
+            'hash': 'd41d8cd98f00b204e9800998ecf8427e',
+            'bytes': '0',
+        }, {
+            'name': 'object/X/2',
+            'last_modified': self.last_modified,
+            'hash': 'd41d8cd98f00b204e9800998ecf8427e',
+            'bytes': '0',
+        }]
+
+        self.swift.register('GET', segment_bucket, swob.HTTPOk, {},
+                            json.dumps(object_list))
+        self.swift.register('HEAD', '/v1/AUTH_test/empty-bucket',
+                            swob.HTTPNoContent, {}, None)
+        self.swift.register('HEAD', segment_bucket + '/object/X',
+                            swob.HTTPOk, {'x-object-meta-foo': 'bar',
+                                          'content-type': 'baz/quux'}, None)
+
+        xml = '<CompleteMultipartUpload>' \
+            '<Part>' \
+            '<PartNumber>1</PartNumber>' \
+            '<ETag>d41d8cd98f00b204e9800998ecf8427e</ETag>' \
+            '</Part>' \
+            '<Part>' \
+            '<PartNumber>2</PartNumber>' \
+            '<ETag>d41d8cd98f00b204e9800998ecf8427e</ETag>' \
+            '</Part>' \
+            '</CompleteMultipartUpload>'
+
+        req = Request.blank('/empty-bucket/object?uploadId=X',
+                            environ={'REQUEST_METHOD': 'POST'},
+                            headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Date': self.get_date_header(), },
+                            body=xml)
+        status, headers, body = self.call_swift3(req)
+        self.assertEquals(self._get_error_code(body), 'EntityTooSmall')
+        self.assertEquals(status.split()[0], '400')
+
+        self.assertEqual(self.swift.calls, [
+            ('HEAD', '/v1/AUTH_test/empty-bucket'),
+            ('HEAD', '/v1/AUTH_test/empty-bucket+segments/object/X'),
+            ('GET', '/v1/AUTH_test/empty-bucket+segments?delimiter=/&'
+                    'format=json&prefix=object/X/'),
+        ])
+
     @s3acl(s3acl_only=True)
     def test_object_multipart_upload_complete_s3acl(self):
         acl_headers = encode_acl('object', ACLPublicRead(Owner('test:tester',