Fix object copy with empty source

When s3 client make a copy request with empty copy source,
current swift3 will return a 500 error. This is caused from
a lack of header validation.

This fixes the behavior to make a 400 Bad Request (InvalidArgument).

Change-Id: I60c7e480582f5b9258433306bbf228b7291e6236

swift3/controllers/obj.py

@@ -58,14 +58,13 @@ class ObjectController(Controller):
         Handle PUT Object and PUT Object (Copy) request
         """
         if CONF.s3_acl:
-            if 'HTTP_X_AMZ_COPY_SOURCE' in req.environ:
+            if 'X-Amz-Copy-Source' in req.headers:
-                src_path = req.environ['HTTP_X_AMZ_COPY_SOURCE']
+                src_path = req.headers['X-Amz-Copy-Source']
-                src_path = src_path if src_path[0] == '/' else ('/' + src_path)
+                src_path = src_path if src_path.startswith('/') else \
+                    ('/' + src_path)
                 src_bucket, src_obj = split_path(src_path, 0, 2, True)
-
                 req.get_response(self.app, 'HEAD', src_bucket, src_obj,
                                  permission='READ')
-
             b_resp = req.get_response(self.app, 'HEAD', obj='')
             # To avoid overwriting the existing object by unauthorized user,
             # we send HEAD request first before writing the object to make
@@ -83,11 +82,11 @@ class ObjectController(Controller):
 
         resp = req.get_response(self.app)
 
-        if 'HTTP_X_COPY_FROM' in req.environ:
+        if 'X-Amz-Copy-Source' in req.headers:
             elem = Element('CopyObjectResult')
             SubElement(elem, 'ETag').text = '"%s"' % resp.etag
             body = tostring(elem, use_s3ns=False)
-            return HTTPOk(body=body)
+            return HTTPOk(body=body, headers=resp.headers)
 
         resp.status = HTTP_OK
 

swift3/request.py

@@ -28,7 +28,6 @@ from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \
     HTTP_PARTIAL_CONTENT, HTTP_NOT_MODIFIED, HTTP_PRECONDITION_FAILED, \
     HTTP_REQUESTED_RANGE_NOT_SATISFIABLE, HTTP_LENGTH_REQUIRED, \
     HTTP_BAD_REQUEST
-
 from swift.common.constraints import check_utf8
 
 from swift3.controllers import ServiceController, BucketController, \
@@ -43,7 +42,7 @@ from swift3.response import AccessDenied, InvalidArgument, InvalidDigest, \
     MissingContentLength, InvalidStorageClass, S3NotImplemented, InvalidURI, \
     MalformedXML, InvalidRequest
 from swift3.exception import NotS3Request, BadSwiftRequest
-from swift3.utils import utf8encode, LOGGER
+from swift3.utils import utf8encode, LOGGER, check_path_header
 from swift3.cfg import CONF
 from swift3.subresource import decode_acl, encode_acl
 from swift3.utils import sysmeta_header
@@ -215,6 +214,16 @@ class Request(swob.Request):
             except Exception:
                 raise InvalidDigest(content_md5=value)
 
+        if 'X-Amz-Copy-Source' in self.headers:
+            try:
+                check_path_header(self, 'X-Amz-Copy-Source', 2, '')
+            except swob.HTTPException:
+                msg = 'Copy Source must mention the source bucket and key: ' \
+                      'sourcebucket/sourcekey'
+                raise InvalidArgument('x-amz-copy-source',
+                                      self.headers['X-Amz-Copy-Source'],
+                                      msg)
+
         if 'x-amz-metadata-directive' in self.headers:
             value = self.headers['x-amz-metadata-directive']
             if value not in ('COPY', 'REPLACE'):

swift3/test/unit/test_obj.py

@@ -16,6 +16,7 @@
 import unittest
 from datetime import datetime
 import hashlib
+from os.path import join
 
 from swift.common import swob
 from swift.common.swob import Request
@@ -205,6 +206,22 @@ class TestSwift3Obj(Swift3TestCase):
         code = self._test_method_error('PUT', '/bucket/object',
                                        swob.HTTPServiceUnavailable)
         self.assertEquals(code, 'InternalError')
+        code = self._test_method_error('PUT', '/bucket/object',
+                                       swob.HTTPCreated,
+                                       {'X-Amz-Copy-Source': ''})
+        self.assertEquals(code, 'InvalidArgument')
+        code = self._test_method_error('PUT', '/bucket/object',
+                                       swob.HTTPCreated,
+                                       {'X-Amz-Copy-Source': '/'})
+        self.assertEquals(code, 'InvalidArgument')
+        code = self._test_method_error('PUT', '/bucket/object',
+                                       swob.HTTPCreated,
+                                       {'X-Amz-Copy-Source': '/bucket'})
+        self.assertEquals(code, 'InvalidArgument')
+        code = self._test_method_error('PUT', '/bucket/object',
+                                       swob.HTTPCreated,
+                                       {'X-Amz-Copy-Source': '/bucket/'})
+        self.assertEquals(code, 'InvalidArgument')
 
     @s3acl
     def test_object_PUT(self):
@@ -357,20 +374,22 @@ class TestSwift3Obj(Swift3TestCase):
             self._test_object_for_s3acl('DELETE', 'test:full_control')
         self.assertEquals(status.split()[0], '204')
 
-    def _test_object_copy_for_s3acl(self, account, src_permission=None):
+    def _test_object_copy_for_s3acl(self, account, src_permission=None,
+                                    src_path='/src_bucket/src_obj'):
         owner = 'test:tester'
         grants = [Grant(User(account), src_permission)] \
             if src_permission else [Grant(User(owner), 'FULL_CONTROL')]
         src_o_headers = \
             encode_acl('object', ACL(Owner(owner, owner), grants))
-        self.swift.register('HEAD', '/v1/AUTH_test/src_bucket/src_obj',
+        self.swift.register(
-                            swob.HTTPOk, src_o_headers, None)
+            'HEAD', join('/v1/AUTH_test', src_path.lstrip('/')),
+            swob.HTTPOk, src_o_headers, None)
 
         req = Request.blank(
             '/bucket/object',
             environ={'REQUEST_METHOD': 'PUT'},
             headers={'Authorization': 'AWS %s:hmac' % account,
-                     'X-Amz-Copy-Source': '/src_bucket/src_obj'})
+                     'X-Amz-Copy-Source': src_path})
 
         return self.call_swift3(req)
 
@@ -417,5 +436,14 @@ class TestSwift3Obj(Swift3TestCase):
             self._test_object_copy_for_s3acl(account, 'READ')
         self.assertEquals(status.split()[0], '403')
 
+    @s3acl(s3acl_only=True)
+    def test_object_PUT_copy_empty_src_path(self):
+        self.swift.register('PUT', '/v1/AUTH_test/bucket/object',
+                            swob.HTTPPreconditionFailed, {}, None)
+        status, headers, body = self._test_object_copy_for_s3acl(
+            'test:write', 'READ', src_path='')
+        self.assertEquals(status.split()[0], '400')
+
+
 if __name__ == '__main__':
     unittest.main()

swift3/test/unit/test_s3_acl.py

@@ -491,6 +491,5 @@ class TestSwift3S3Acl(Swift3TestCase):
         self.assertRaises(TypeError, fake_class.s3acl_s3only_error)
         self.assertEquals(None, fake_class.s3acl_s3only_no_error())
 
-
 if __name__ == '__main__':
     unittest.main()

swift3/utils.py

@@ -20,6 +20,11 @@ import base64
 
 from swift.common.utils import get_logger
 
+# Need for check_path_header
+from swift.common import utils
+from swift.common.swob import HTTPPreconditionFailed
+from urllib import unquote
+
 from swift3.cfg import CONF
 
 LOGGER = get_logger(CONF, log_route='swift3')
@@ -64,3 +69,30 @@ def utf8decode(s):
     if isinstance(s, str):
         s = s.decode('utf8')
     return s
+
+
+def check_path_header(req, name, length, error_msg):
+    # FIXME: replace swift.common.constraints check_path_header
+    #        when swift3 supports swift 2.2 or later
+    """
+    Validate that the value of path-like header is
+    well formatted. We assume the caller ensures that
+    specific header is present in req.headers.
+
+    :param req: HTTP request object
+    :param name: header name
+    :param length: length of path segment check
+    :param error_msg: error message for client
+    :returns: A tuple with path parts according to length
+    :raise: HTTPPreconditionFailed if header value
+            is not well formatted.
+    """
+    src_header = unquote(req.headers.get(name))
+    if not src_header.startswith('/'):
+        src_header = '/' + src_header
+    try:
+        return utils.split_path(src_header, length, length, True)
+    except ValueError:
+        raise HTTPPreconditionFailed(
+            request=req,
+            body=error_msg)