Merge "moving object validation checks to top of PUT method"

swift/common/constraints.py

@@ -15,12 +15,14 @@
 
 import os
 import urllib
+import time
 from urllib import unquote
 from ConfigParser import ConfigParser, NoSectionError, NoOptionError
 
 from swift.common import utils, exceptions
 from swift.common.swob import HTTPBadRequest, HTTPLengthRequired, \
-    HTTPRequestEntityTooLarge, HTTPPreconditionFailed
+    HTTPRequestEntityTooLarge, HTTPPreconditionFailed, HTTPNotImplemented, \
+    HTTPException
 
 MAX_FILE_SIZE = 5368709122
 MAX_META_NAME_LENGTH = 128
@@ -154,24 +156,45 @@ def check_object_creation(req, object_name):
                                  a chunked request
     :returns HTTPBadRequest: missing or bad content-type header, or
                              bad metadata
+    :returns HTTPNotImplemented: unsupported transfer-encoding header value
     """
-    if req.content_length and req.content_length > MAX_FILE_SIZE:
+    try:
+        ml = req.message_length()
+    except ValueError as e:
+        return HTTPBadRequest(request=req, content_type='text/plain',
+                              body=str(e))
+    except AttributeError as e:
+        return HTTPNotImplemented(request=req, content_type='text/plain',
+                                  body=str(e))
+    if ml is not None and ml > MAX_FILE_SIZE:
         return HTTPRequestEntityTooLarge(body='Your request is too large.',
                                          request=req,
                                          content_type='text/plain')
     if req.content_length is None and \
             req.headers.get('transfer-encoding') != 'chunked':
-        return HTTPLengthRequired(request=req)
+        return HTTPLengthRequired(body='Missing Content-Length header.',
+                                  request=req,
+                                  content_type='text/plain')
+
     if 'X-Copy-From' in req.headers and req.content_length:
         return HTTPBadRequest(body='Copy requests require a zero byte body',
                               request=req, content_type='text/plain')
+
     if len(object_name) > MAX_OBJECT_NAME_LENGTH:
         return HTTPBadRequest(body='Object name length of %d longer than %d' %
                               (len(object_name), MAX_OBJECT_NAME_LENGTH),
                               request=req, content_type='text/plain')
+
     if 'Content-Type' not in req.headers:
         return HTTPBadRequest(request=req, content_type='text/plain',
                               body='No content type')
+
+    try:
+        req = check_delete_headers(req)
+    except HTTPException as e:
+        return HTTPBadRequest(request=req, body=e.body,
+                              content_type='text/plain')
+
     if not check_utf8(req.headers['Content-Type']):
         return HTTPBadRequest(request=req, body='Invalid Content-Type',
                               content_type='text/plain')
@@ -225,6 +248,46 @@ def valid_timestamp(request):
                              content_type='text/plain')
 
 
+def check_delete_headers(request):
+    """
+    Validate if 'x-delete' headers are have correct values
+    values should be positive integers and correspond to
+    a time in the future.
+
+    :param request: the swob request object
+
+    :returns: HTTPBadRequest in case of invalid values
+              or None if values are ok
+    """
+    if 'x-delete-after' in request.headers:
+        try:
+            x_delete_after = int(request.headers['x-delete-after'])
+        except ValueError:
+            raise HTTPBadRequest(request=request,
+                                 content_type='text/plain',
+                                 body='Non-integer X-Delete-After')
+        actual_del_time = time.time() + x_delete_after
+        if actual_del_time < time.time():
+            raise HTTPBadRequest(request=request,
+                                 content_type='text/plain',
+                                 body='X-Delete-After in past')
+        request.headers['x-delete-at'] = utils.normalize_delete_at_timestamp(
+            actual_del_time)
+
+    if 'x-delete-at' in request.headers:
+        try:
+            x_delete_at = int(utils.normalize_delete_at_timestamp(
+                int(request.headers['x-delete-at'])))
+        except ValueError:
+            raise HTTPBadRequest(request=request, content_type='text/plain',
+                                 body='Non-integer X-Delete-At')
+
+        if x_delete_at < time.time():
+            raise HTTPBadRequest(request=request, content_type='text/plain',
+                                 body='X-Delete-At in past')
+    return request
+
+
 def check_utf8(string):
     """
     Validate if a string is valid UTF-8 str or unicode and that it

swift/proxy/controllers/obj.py

@@ -56,7 +56,7 @@ from swift.proxy.controllers.base import Controller, delay_denial, \
 from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \
     HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \
     HTTPServerError, HTTPServiceUnavailable, Request, \
-    HTTPClientDisconnect, HTTPNotImplemented
+    HTTPClientDisconnect
 from swift.common.request_helpers import is_sys_or_user_meta, is_sys_meta, \
     remove_items, copy_header_subset
 
@@ -437,27 +437,11 @@ class ObjectController(Controller):
         delete_at_part = None
         delete_at_nodes = None
 
-        if 'x-delete-after' in req.headers:
+        req = constraints.check_delete_headers(req)
-            try:
-                x_delete_after = int(req.headers['x-delete-after'])
-            except ValueError:
-                raise HTTPBadRequest(request=req, content_type='text/plain',
-                                     body='Non-integer X-Delete-After')
-
-            req.headers['x-delete-at'] = normalize_delete_at_timestamp(
-                time.time() + x_delete_after)
 
         if 'x-delete-at' in req.headers:
-            try:
+            x_delete_at = int(normalize_delete_at_timestamp(
-                x_delete_at = int(normalize_delete_at_timestamp(
+                int(req.headers['x-delete-at'])))
-                    int(req.headers['x-delete-at'])))
-            except ValueError:
-                raise HTTPBadRequest(request=req, content_type='text/plain',
-                                     body='Non-integer X-Delete-At')
-
-            if x_delete_at < time.time():
-                raise HTTPBadRequest(request=req, content_type='text/plain',
-                                     body='X-Delete-At in past')
 
             req.environ.setdefault('swift.log_info', []).append(
                 'x-delete-at:%s' % x_delete_at)
@@ -502,16 +486,23 @@ class ObjectController(Controller):
         if not containers:
             return HTTPNotFound(request=req)
 
-        try:
+        # Sometimes the 'content-type' header exists, but is set to None.
-            ml = req.message_length()
+        content_type_manually_set = True
-        except ValueError as e:
+        detect_content_type = \
-            return HTTPBadRequest(request=req, content_type='text/plain',
+            config_true_value(req.headers.get('x-detect-content-type'))
-                                  body=str(e))
+        if detect_content_type or not req.headers.get('content-type'):
-        except AttributeError as e:
+            guessed_type, _junk = mimetypes.guess_type(req.path_info)
-            return HTTPNotImplemented(request=req, content_type='text/plain',
+            req.headers['Content-Type'] = guessed_type or \
-                                      body=str(e))
+                'application/octet-stream'
-        if ml is not None and ml > constraints.MAX_FILE_SIZE:
+            if detect_content_type:
-            return HTTPRequestEntityTooLarge(request=req)
+                req.headers.pop('x-detect-content-type')
+            else:
+                content_type_manually_set = False
+
+        error_response = check_object_creation(req, self.object_name) or \
+            check_content_type(req)
+        if error_response:
+            return error_response
 
         partition, nodes = obj_ring.get_nodes(
             self.account_name, self.container_name, self.object_name)
@@ -545,23 +536,6 @@ class ObjectController(Controller):
         else:
             req.headers['X-Timestamp'] = Timestamp(time.time()).internal
 
-        # Sometimes the 'content-type' header exists, but is set to None.
-        content_type_manually_set = True
-        detect_content_type = \
-            config_true_value(req.headers.get('x-detect-content-type'))
-        if detect_content_type or not req.headers.get('content-type'):
-            guessed_type, _junk = mimetypes.guess_type(req.path_info)
-            req.headers['Content-Type'] = guessed_type or \
-                'application/octet-stream'
-            if detect_content_type:
-                req.headers.pop('x-detect-content-type')
-            else:
-                content_type_manually_set = False
-
-        error_response = check_object_creation(req, self.object_name) or \
-            check_content_type(req)
-        if error_response:
-            return error_response
         if object_versions and not req.environ.get('swift_versioned_copy'):
             if hresp.status_int != HTTP_NOT_FOUND:
                 # This is a version manifest and needs to be handled

test/functional/tests.py

@@ -1422,6 +1422,16 @@ class TestFile(Base):
                           cfg={'no_content_length': True})
         self.assert_status(400)
 
+        # no content-length
+        self.assertRaises(ResponseError, file_item.write_random, file_length,
+                          cfg={'no_content_length': True})
+        self.assert_status(411)
+
+        self.assertRaises(ResponseError, file_item.write_random, file_length,
+                          hdrs={'transfer-encoding': 'gzip,chunked'},
+                          cfg={'no_content_length': True})
+        self.assert_status(501)
+
         # bad request types
         #for req in ('LICK', 'GETorHEAD_base', 'container_info',
         #            'best_response'):

test/unit/common/test_constraints.py

@@ -23,7 +23,7 @@ from test.unit import MockTrue
 
 from swift.common.swob import HTTPBadRequest, Request, HTTPException
 from swift.common.http import HTTP_REQUEST_ENTITY_TOO_LARGE, \
-    HTTP_BAD_REQUEST, HTTP_LENGTH_REQUIRED
+    HTTP_BAD_REQUEST, HTTP_LENGTH_REQUIRED, HTTP_NOT_IMPLEMENTED
 from swift.common import constraints, utils
 
 
@@ -125,20 +125,68 @@ class TestConstraints(unittest.TestCase):
                    'Content-Type': 'text/plain'}
         self.assertEquals(constraints.check_object_creation(Request.blank(
             '/', headers=headers), 'object_name'), None)
+
         headers = {'Content-Length': str(constraints.MAX_FILE_SIZE + 1),
                    'Content-Type': 'text/plain'}
         self.assertEquals(constraints.check_object_creation(
             Request.blank('/', headers=headers), 'object_name').status_int,
             HTTP_REQUEST_ENTITY_TOO_LARGE)
+
         headers = {'Transfer-Encoding': 'chunked',
                    'Content-Type': 'text/plain'}
         self.assertEquals(constraints.check_object_creation(Request.blank(
             '/', headers=headers), 'object_name'), None)
+
+        headers = {'Transfer-Encoding': 'gzip',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name').status_int,
+            HTTP_BAD_REQUEST)
+
         headers = {'Content-Type': 'text/plain'}
         self.assertEquals(constraints.check_object_creation(
             Request.blank('/', headers=headers), 'object_name').status_int,
             HTTP_LENGTH_REQUIRED)
 
+        headers = {'Content-Length': 'abc',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name').status_int,
+            HTTP_BAD_REQUEST)
+
+        headers = {'Transfer-Encoding': 'gzip,chunked',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name').status_int,
+            HTTP_NOT_IMPLEMENTED)
+
+    def test_check_object_creation_copy(self):
+        headers = {'Content-Length': '0',
+                   'X-Copy-From': 'c/o2',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name'), None)
+
+        headers = {'Content-Length': '1',
+                   'X-Copy-From': 'c/o2',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name').status_int,
+            HTTP_BAD_REQUEST)
+
+        headers = {'Transfer-Encoding': 'chunked',
+                   'X-Copy-From': 'c/o2',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name'), None)
+
+        # a content-length header is always required
+        headers = {'X-Copy-From': 'c/o2',
+                   'Content-Type': 'text/plain'}
+        self.assertEquals(constraints.check_object_creation(Request.blank(
+            '/', headers=headers), 'object_name').status_int,
+            HTTP_LENGTH_REQUIRED)
+
     def test_check_object_creation_name_length(self):
         headers = {'Transfer-Encoding': 'chunked',
                    'Content-Type': 'text/plain'}
@@ -168,6 +216,115 @@ class TestConstraints(unittest.TestCase):
         self.assertEquals(resp.status_int, HTTP_BAD_REQUEST)
         self.assert_('Content-Type' in resp.body)
 
+    def test_check_object_creation_bad_delete_headers(self):
+        headers = {'Transfer-Encoding': 'chunked',
+                   'Content-Type': 'text/plain',
+                   'X-Delete-After': 'abc'}
+        resp = constraints.check_object_creation(
+            Request.blank('/', headers=headers), 'object_name')
+        self.assertEquals(resp.status_int, HTTP_BAD_REQUEST)
+        self.assert_('Non-integer X-Delete-After' in resp.body)
+
+        t = str(int(time.time() - 60))
+        headers = {'Transfer-Encoding': 'chunked',
+                   'Content-Type': 'text/plain',
+                   'X-Delete-At': t}
+        resp = constraints.check_object_creation(
+            Request.blank('/', headers=headers), 'object_name')
+        self.assertEquals(resp.status_int, HTTP_BAD_REQUEST)
+        self.assert_('X-Delete-At in past' in resp.body)
+
+    def test_check_delete_headers(self):
+
+        # X-Delete-After
+        headers = {'X-Delete-After': '60'}
+        resp = constraints.check_delete_headers(
+            Request.blank('/', headers=headers))
+        self.assertTrue(isinstance(resp, Request))
+        self.assertTrue('x-delete-at' in resp.headers)
+
+        headers = {'X-Delete-After': 'abc'}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('Non-integer X-Delete-After' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
+        headers = {'X-Delete-After': '60.1'}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('Non-integer X-Delete-After' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
+        headers = {'X-Delete-After': '-1'}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('X-Delete-After in past' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
+        # X-Delete-At
+        t = str(int(time.time() + 100))
+        headers = {'X-Delete-At': t}
+        resp = constraints.check_delete_headers(
+            Request.blank('/', headers=headers))
+        self.assertTrue(isinstance(resp, Request))
+        self.assertTrue('x-delete-at' in resp.headers)
+        self.assertEquals(resp.headers.get('X-Delete-At'), t)
+
+        headers = {'X-Delete-At': 'abc'}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('Non-integer X-Delete-At' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
+        t = str(int(time.time() + 100)) + '.1'
+        headers = {'X-Delete-At': t}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('Non-integer X-Delete-At' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
+        t = str(int(time.time()))
+        headers = {'X-Delete-At': t}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('X-Delete-At in past' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
+        t = str(int(time.time() - 1))
+        headers = {'X-Delete-At': t}
+        try:
+            resp = constraints.check_delete_headers(
+                Request.blank('/', headers=headers))
+        except HTTPException as e:
+            self.assertEquals(e.status_int, HTTP_BAD_REQUEST)
+            self.assertTrue('X-Delete-At in past' in e.body)
+        else:
+            self.fail("Should have failed with HTTPBadRequest")
+
     def test_check_mount(self):
         self.assertFalse(constraints.check_mount('', ''))
         with mock.patch("swift.common.utils.ismount", MockTrue()):

test/unit/proxy/controllers/test_obj.py

@@ -279,10 +279,7 @@ class TestObjController(unittest.TestCase):
         req = swob.Request.blank('/v1/a/c/o', method='POST',
                                  headers={'Content-Type': 'foo/bar',
                                           'X-Delete-After': t})
-        x_newest_responses = [200] * self.obj_ring.replicas + \
+        resp = req.get_response(self.app)
-            [404] * self.obj_ring.max_more_nodes
-        with set_http_connect(*x_newest_responses):
-            resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 400)
         self.assertEqual('Non-integer X-Delete-After', resp.body)
 
@@ -290,22 +287,16 @@ class TestObjController(unittest.TestCase):
         req = swob.Request.blank('/v1/a/c/o', method='POST',
                                  headers={'Content-Type': 'foo/bar',
                                           'X-Delete-After': '-60'})
-        x_newest_responses = [200] * self.obj_ring.replicas + \
+        resp = req.get_response(self.app)
-            [404] * self.obj_ring.max_more_nodes
-        with set_http_connect(*x_newest_responses):
-            resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 400)
-        self.assertEqual('X-Delete-At in past', resp.body)
+        self.assertEqual('X-Delete-After in past', resp.body)
 
     def test_POST_delete_at_non_integer(self):
         t = str(int(time.time() + 100)) + '.1'
         req = swob.Request.blank('/v1/a/c/o', method='POST',
                                  headers={'Content-Type': 'foo/bar',
                                           'X-Delete-At': t})
-        x_newest_responses = [200] * self.obj_ring.replicas + \
+        resp = req.get_response(self.app)
-            [404] * self.obj_ring.max_more_nodes
-        with set_http_connect(*x_newest_responses):
-            resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 400)
         self.assertEqual('Non-integer X-Delete-At', resp.body)
 
@@ -314,10 +305,7 @@ class TestObjController(unittest.TestCase):
         req = swob.Request.blank('/v1/a/c/o', method='POST',
                                  headers={'Content-Type': 'foo/bar',
                                           'X-Delete-At': t})
-        x_newest_responses = [200] * self.obj_ring.replicas + \
+        resp = req.get_response(self.app)
-            [404] * self.obj_ring.max_more_nodes
-        with set_http_connect(*x_newest_responses):
-            resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 400)
         self.assertEqual('X-Delete-At in past', resp.body)
 
@@ -363,7 +351,7 @@ class TestObjController(unittest.TestCase):
         with set_http_connect():
             resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 400)
-        self.assertEqual('X-Delete-At in past', resp.body)
+        self.assertEqual('X-Delete-After in past', resp.body)
 
     def test_PUT_delete_at(self):
         t = str(int(time.time() + 100))