Fix container quota MW for handling a bad source path

The copy source must be container/object.
This patch avoids the server to return
an internal server error when user provides
a path without a container.

Fixes: bug #1255049
Change-Id: I1a85c98d9b3a78bad40b8ceba9088cf323042412

swift/common/constraints.py

@@ -15,11 +15,12 @@
 
 import os
 import urllib
+from urllib import unquote
 from ConfigParser import ConfigParser, NoSectionError, NoOptionError
 
-from swift.common.utils import ismount
+from swift.common.utils import ismount, split_path
 from swift.common.swob import HTTPBadRequest, HTTPLengthRequired, \
-    HTTPRequestEntityTooLarge
+    HTTPRequestEntityTooLarge, HTTPPreconditionFailed
 
 constraints_conf = ConfigParser()
 constraints_conf.read('/etc/swift/swift.conf')
@@ -211,3 +212,26 @@ def check_utf8(string):
     # So, we should catch both UnicodeDecodeError & UnicodeEncodeError
     except UnicodeError:
         return False
+
+
+def check_copy_from_header(req):
+    """
+    Validate that the value from x-copy-from header is
+    well formatted. We assume the caller ensures that
+    x-copy-from header is present in req.headers.
+
+    :param req: HTTP request object
+    :returns: A tuple with container name and object name
+    :raise: HTTPPreconditionFailed if x-copy-from value
+            is not well formatted.
+    """
+    src_header = unquote(req.headers.get('X-Copy-From'))
+    if not src_header.startswith('/'):
+        src_header = '/' + src_header
+    try:
+        return split_path(src_header, 2, 2, True)
+    except ValueError:
+        raise HTTPPreconditionFailed(
+            request=req,
+            body='X-Copy-From header must be of the form'
+                 '<container name>/<object name>')

swift/common/middleware/container_quotas.py

@@ -41,7 +41,7 @@
 |                                             | container.                    |
 +---------------------------------------------+-------------------------------+
 """
-
+from swift.common.constraints import check_copy_from_header
 from swift.common.http import is_success
 from swift.common.swob import Response, HTTPBadRequest, wsgify
 from swift.common.utils import register_swift_info
@@ -90,10 +90,10 @@ def __call__(self, req):
                     'bytes' in container_info and \
                     container_info['meta']['quota-bytes'].isdigit():
                 content_length = (req.content_length or 0)
-                copy_from = req.headers.get('X-Copy-From')
-                if copy_from:
-                    path = '/%s/%s/%s' % (version, account,
-                                          copy_from.lstrip('/'))
+                if 'x-copy-from' in req.headers:
+                    src_cont, src_obj = check_copy_from_header(req)
+                    path = '/%s/%s/%s/%s' % (version, account,
+                                             src_cont, src_obj)
                     object_info = get_object_info(req.environ, self.app, path)
                     if not object_info or not object_info['length']:
                         content_length = 0

swift/proxy/controllers/obj.py

@@ -45,7 +45,7 @@
     get_valid_utf8_str, GreenAsyncPile
 from swift.common.bufferedhttp import http_connect
 from swift.common.constraints import check_metadata, check_object_creation, \
-    CONTAINER_LISTING_LIMIT, MAX_FILE_SIZE
+    CONTAINER_LISTING_LIMIT, MAX_FILE_SIZE, check_copy_from_header
 from swift.common.exceptions import ChunkReadTimeout, \
     ChunkWriteTimeout, ConnectionTimeout, ListingIterNotFound, \
     ListingIterNotAuthorized, ListingIterError, SegmentError
@@ -992,21 +992,12 @@ def PUT(self, req):
             if req.environ.get('swift.orig_req_method', req.method) != 'POST':
                 req.environ.setdefault('swift.log_info', []).append(
                     'x-copy-from:%s' % source_header)
-            source_header = unquote(source_header)
-            acct = req.swift_entity_path.split('/', 2)[1]
+            src_container_name, src_obj_name = check_copy_from_header(req)
+            ver, acct, _rest = req.split_path(2, 3, True)
             if isinstance(acct, unicode):
                 acct = acct.encode('utf-8')
-            if not source_header.startswith('/'):
-                source_header = '/' + source_header
-            source_header = '/v1/' + acct + source_header
-            try:
-                src_container_name, src_obj_name = \
-                    source_header.split('/', 4)[3:]
-            except ValueError:
-                return HTTPPreconditionFailed(
-                    request=req,
-                    body='X-Copy-From header must be of the form'
-                         '<container name>/<object name>')
+            source_header = '/%s/%s/%s/%s' % (ver, acct,
+                                              src_container_name, src_obj_name)
             source_req = req.copy_get()
             source_req.path_info = source_header
             source_req.headers['X-Newest'] = 'true'

test/unit/common/middleware/test_quotas.py

@@ -138,6 +138,16 @@ def test_bytes_quota_copy_from_no_src(self):
         res = req.get_response(app)
         self.assertEquals(res.status_int, 200)
 
+    def test_bytes_quota_copy_from_bad_src(self):
+        app = container_quotas.ContainerQuotaMiddleware(FakeApp(), {})
+        cache = FakeCache({'bytes': 0, 'meta': {'quota-bytes': '100'}})
+        req = Request.blank('/v1/a/c/o',
+                            environ={'REQUEST_METHOD': 'PUT',
+                            'swift.cache': cache},
+                            headers={'x-copy-from': 'bad_path'})
+        res = req.get_response(app)
+        self.assertEquals(res.status_int, 412)
+
     def test_exceed_counts_quota(self):
         app = container_quotas.ContainerQuotaMiddleware(FakeApp(), {})
         cache = FakeCache({'object_count': 1, 'meta': {'quota-count': '1'}})

test/unit/common/test_constraints.py

@@ -19,7 +19,7 @@
 from test import safe_repr
 from test.unit import MockTrue
 
-from swift.common.swob import HTTPBadRequest, Request
+from swift.common.swob import HTTPBadRequest, Request, HTTPException
 from swift.common.http import HTTP_REQUEST_ENTITY_TOO_LARGE, \
     HTTP_BAD_REQUEST, HTTP_LENGTH_REQUIRED
 from swift.common import constraints
@@ -257,6 +257,33 @@ def test_validate_constraints(self):
         self.assertTrue(c.MAX_HEADER_SIZE > c.MAX_META_NAME_LENGTH)
         self.assertTrue(c.MAX_HEADER_SIZE > c.MAX_META_VALUE_LENGTH)
 
+    def test_validate_copy_from(self):
+        req = Request.blank(
+            '/v/a/c/o',
+            headers={'x-copy-from': 'c/o2'})
+        src_cont, src_obj = constraints.check_copy_from_header(req)
+        self.assertEqual(src_cont, 'c')
+        self.assertEqual(src_obj, 'o2')
+        req = Request.blank(
+            '/v/a/c/o',
+            headers={'x-copy-from': 'c/subdir/o2'})
+        src_cont, src_obj = constraints.check_copy_from_header(req)
+        self.assertEqual(src_cont, 'c')
+        self.assertEqual(src_obj, 'subdir/o2')
+        req = Request.blank(
+            '/v/a/c/o',
+            headers={'x-copy-from': '/c/o2'})
+        src_cont, src_obj = constraints.check_copy_from_header(req)
+        self.assertEqual(src_cont, 'c')
+        self.assertEqual(src_obj, 'o2')
+
+    def test_validate_bad_copy_from(self):
+        req = Request.blank(
+            '/v/a/c/o',
+            headers={'x-copy-from': 'bad_object'})
+        self.assertRaises(HTTPException,
+                          constraints.check_copy_from_header, req)
+
 
 if __name__ == '__main__':
     unittest.main()

test/unit/proxy/controllers/test_obj.py

@@ -21,6 +21,7 @@
 
 import swift
 from swift.proxy import server as proxy_server
+from swift.common.swob import HTTPException
 from test.unit import FakeRing, FakeMemcache, fake_http_connect
 
 
@@ -107,14 +108,20 @@ def test_PUT_log_info(self):
             # and now test that we add the header to log_info
             req = swift.common.swob.Request.blank('/v1/a/c/o')
             req.headers['x-copy-from'] = 'somewhere'
-            controller.PUT(req)
+            try:
+                controller.PUT(req)
+            except HTTPException:
+                pass
             self.assertEquals(
                 req.environ.get('swift.log_info'), ['x-copy-from:somewhere'])
             # and then check that we don't do that for originating POSTs
             req = swift.common.swob.Request.blank('/v1/a/c/o')
             req.method = 'POST'
             req.headers['x-copy-from'] = 'elsewhere'
-            controller.PUT(req)
+            try:
+                controller.PUT(req)
+            except HTTPException:
+                pass
             self.assertEquals(req.environ.get('swift.log_info'), None)