Return BadRequest on X-Delete-At/After headers

Gluster-swift does not support X-Delete-After or X-Delete-At
headers.  The code needs to inform the caller with a 400-BadRequest
if they use these headers.

Change-Id: Ic9d3a1646c0d26bb0204245efce4501f7479fee6
Signed-off-by: Luis Pabon <lpabon@redhat.com>
Reviewed-on: http://review.gluster.org/6364
Reviewed-by: Chetan Risbud <crisbud@redhat.com>

gluster/swift/common/Glusterfs.py

@@ -37,6 +37,7 @@ _allow_mount_per_server = False
 _implicit_dir_objects = False
 _container_update_object_count = False
 _account_update_container_count = False
+_ignore_unsupported_headers = False
 
 if _fs_conf.read(os.path.join(SWIFT_DIR, 'fs.conf')):
     try:
@@ -97,6 +98,19 @@ if _fs_conf.read(os.path.join(SWIFT_DIR, 'fs.conf')):
     except (NoSectionError, NoOptionError):
         pass
 
+    # -- Hidden configuration option --
+    # Ignore unsupported headers and allow them in a request without
+    # returning a 400-BadRequest.  This setting can be set to
+    # allow unsupported headers such as X-Delete-At and
+    # X-Delete-After even though they will not be used.
+    try:
+        _ignore_unsupported_headers = \
+            _fs_conf.get('DEFAULT',
+                         'ignore_unsupported_headers',
+                         "no") in TRUE_VALUES
+    except (NoSectionError, NoOptionError):
+        pass
+
 NAME = 'glusterfs'
 
 

gluster/swift/common/constraints.py

@@ -23,6 +23,7 @@ import swift.common.ring as _ring
 from gluster.swift.common import Glusterfs, ring
 
 MAX_OBJECT_NAME_COMPONENT_LENGTH = 255
+UNSUPPORTED_HEADERS = ['x-delete-at', 'x-delete-after']
 
 
 def set_object_name_component_length(len=None):
@@ -54,8 +55,41 @@ def validate_obj_name_component(obj):
         return 'cannot be . or ..'
     return ''
 
+
+def validate_headers(req):
+    """
+    Validate client header requests
+    :param req: Http request
+    """
+    if not Glusterfs._ignore_unsupported_headers:
+        for unsupported_header in UNSUPPORTED_HEADERS:
+            if unsupported_header in req.headers:
+                return '%s headers are not supported' \
+                       % ','.join(UNSUPPORTED_HEADERS)
+    return ''
+
 # Save the original check object creation
 __check_object_creation = swift.common.constraints.check_object_creation
+__check_metadata = swift.common.constraints.check_metadata
+
+
+def gluster_check_metadata(req, target_type, POST=True):
+    """
+    :param req: HTTP request object
+    :param target_type: Value from POST passed to __check_metadata
+    :param POST: Only call __check_metadata on POST since Swift only
+                 calls check_metadata on POSTs.
+    """
+    ret = None
+    if POST:
+        ret = __check_metadata(req, target_type)
+    if ret is None:
+        bdy = validate_headers(req)
+        if bdy:
+            ret = HTTPBadRequest(body=bdy,
+                                 request=req,
+                                 content_type='text/plain')
+    return ret
 
 
 # Define our new one which invokes the original
@@ -85,11 +119,14 @@ def gluster_check_object_creation(req, object_name):
                 ret = HTTPBadRequest(body=bdy,
                                      request=req,
                                      content_type='text/plain')
+    if ret is None:
+        ret = gluster_check_metadata(req, 'object', POST=False)
 
     return ret
 
-# Replace the original check object creation with ours
+# Replace the original checks with ours
 swift.common.constraints.check_object_creation = gluster_check_object_creation
+swift.common.constraints.check_metadata = gluster_check_metadata
 
 # Replace the original check mount with ours
 swift.common.constraints.check_mount = Glusterfs.mount

test/functional/gluster_swift_tests.py

@@ -58,6 +58,37 @@ class TestFile(Base):
         data_read = file.read()
         self.assertEquals(data,data_read)
 
+    def testInvalidHeadersPUT(self):
+        file = self.env.container.file(Utils.create_name())
+        self.assertRaises(ResponseError,
+                          file.write_random,
+                          self.env.file_size,
+                          hdrs={'X-Delete-At': '9876545321'})
+        self.assert_status(400)
+        self.assertRaises(ResponseError,
+                          file.write_random,
+                          self.env.file_size,
+                          hdrs={'X-Delete-After': '60'})
+        self.assert_status(400)
+
+    def testInvalidHeadersPOST(self):
+        file = self.env.container.file(Utils.create_name())
+        file.write_random(self.env.file_size)
+        headers = file.make_headers(cfg={})
+        headers.update({ 'X-Delete-At' : '987654321'})
+        # Need to call conn.make_request instead of file.sync_metadata
+        # because sync_metadata calls make_headers.  make_headers()
+        # overwrites any headers in file.metadata as 'user' metadata
+        # by appending 'X-Object-Meta-' to any of the headers
+        # in file.metadata.
+        file.conn.make_request('POST', file.path, hdrs=headers, cfg={})
+        self.assertEqual(400, file.conn.response.status)
+
+        headers = file.make_headers(cfg={})
+        headers.update({ 'X-Delete-After' : '60'})
+        file.conn.make_request('POST', file.path, hdrs=headers, cfg={})
+        self.assertEqual(400, file.conn.response.status)
+
 
 class TestFileUTF8(Base2, TestFile):
     set_up = False
@@ -336,5 +367,3 @@ class TestMultiProtocolAccess(Base):
         md5_returned = hashlib.md5(data_read_from_mountP).hexdigest()
         self.assertEquals(md5_returned,file_info['etag'])
         fhOnMountPoint.close()
-
-

test/unit/common/test_constraints.py

@@ -16,7 +16,7 @@
 import unittest
 import swift.common.constraints
 from nose import SkipTest
-from mock import patch
+from mock import Mock, patch
 from gluster.swift.common import constraints as cnt
 
 
@@ -75,12 +75,65 @@ class TestConstraints(unittest.TestCase):
         self.assertTrue(cnt.validate_obj_name_component('..'))
         self.assertTrue(cnt.validate_obj_name_component(''))
 
+    def test_validate_headers(self):
+        req = Mock()
+        req.headers = []
+        self.assertEqual(cnt.validate_headers(req), '')
+        req.headers = ['x-some-header']
+        self.assertEqual(cnt.validate_headers(req), '')
+        req.headers = ['x-delete-at', 'x-some-header']
+        self.assertNotEqual(cnt.validate_headers(req), '')
+        req.headers = ['x-delete-after', 'x-some-header']
+        self.assertNotEqual(cnt.validate_headers(req), '')
+        req.headers = ['x-delete-at', 'x-delete-after', 'x-some-header']
+        self.assertNotEqual(cnt.validate_headers(req), '')
+
+    def test_validate_headers_ignoring_config_set(self):
+        with patch('gluster.swift.common.constraints.'
+                   'Glusterfs._ignore_unsupported_headers', True):
+            req = Mock()
+            req.headers = []
+            self.assertEqual(cnt.validate_headers(req), '')
+            req.headers = ['x-some-header']
+            self.assertEqual(cnt.validate_headers(req), '')
+            req.headers = ['x-delete-at', 'x-some-header']
+            self.assertEqual(cnt.validate_headers(req), '')
+            req.headers = ['x-delete-after', 'x-some-header']
+            self.assertEqual(cnt.validate_headers(req), '')
+            req.headers = ['x-delete-at', 'x-delete-after', 'x-some-header']
+            self.assertEqual(cnt.validate_headers(req), '')
+
+    def test_gluster_check_metadata(self):
+        mock_check_metadata = Mock()
+        with patch('gluster.swift.common.constraints.__check_metadata',
+                   mock_check_metadata):
+            req = Mock()
+            req.headers = []
+            cnt.gluster_check_metadata(req, 'object')
+            self.assertTrue(1, mock_check_metadata.call_count)
+            cnt.gluster_check_metadata(req, 'object', POST=False)
+            self.assertTrue(1, mock_check_metadata.call_count)
+            req.headers = ['x-some-header']
+            self.assertEqual(cnt.gluster_check_metadata(req, 'object', POST=False), None)
+            req.headers = ['x-delete-at', 'x-some-header']
+            self.assertNotEqual(cnt.gluster_check_metadata(req, 'object', POST=False), None)
+            req.headers = ['x-delete-after', 'x-some-header']
+            self.assertNotEqual(cnt.gluster_check_metadata(req, 'object', POST=False), None)
+            req.headers = ['x-delete-at', 'x-delete-after', 'x-some-header']
+            self.assertNotEqual(cnt.gluster_check_metadata(req, 'object', POST=False), None)
+
     def test_gluster_check_object_creation(self):
         with patch('gluster.swift.common.constraints.__check_object_creation',
                    mock_check_object_creation):
-            self.assertFalse(cnt.gluster_check_object_creation(None, 'dir/z'))
+            req = Mock()
+            req.headers = []
+            self.assertFalse(cnt.gluster_check_object_creation(req, 'dir/z'))
 
     def test_gluster_check_object_creation_err(self):
         with patch('gluster.swift.common.constraints.__check_object_creation',
                    mock_check_object_creation):
-            self.assertTrue(cnt.gluster_check_object_creation(None, 'dir/.'))
+            req = Mock()
+            req.headers = []
+            self.assertTrue(cnt.gluster_check_object_creation(req, 'dir/.'))
+            req.headers = ['x-delete-at']
+            self.assertTrue(cnt.gluster_check_object_creation(req, 'dir/z'))

test/unit/proxy/test_server.py

@@ -4358,6 +4358,7 @@ class TestObjectController(unittest.TestCase):
         self.assert_(called[0])
 
     def test_POST_converts_delete_after_to_delete_at(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',
                                                        'container', 'object')
@@ -4394,6 +4395,7 @@ class TestObjectController(unittest.TestCase):
                 time.time = orig_time
 
     def test_POST_non_int_delete_after(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',
                                                        'container', 'object')
@@ -4408,6 +4410,7 @@ class TestObjectController(unittest.TestCase):
             self.assertTrue('Non-integer X-Delete-After' in res.body)
 
     def test_POST_negative_delete_after(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',
                                                        'container', 'object')
@@ -4422,6 +4425,7 @@ class TestObjectController(unittest.TestCase):
             self.assertTrue('X-Delete-At in past' in res.body)
 
     def test_POST_delete_at(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             given_headers = {}
 
@@ -4466,6 +4470,7 @@ class TestObjectController(unittest.TestCase):
             self.assertTrue('X-Delete-At in past' in resp.body)
 
     def test_PUT_converts_delete_after_to_delete_at(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',
                                                        'container', 'object')
@@ -4488,6 +4493,7 @@ class TestObjectController(unittest.TestCase):
                 time.time = orig_time
 
     def test_PUT_non_int_delete_after(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',
                                                        'container', 'object')
@@ -4503,6 +4509,7 @@ class TestObjectController(unittest.TestCase):
             self.assertTrue('Non-integer X-Delete-After' in res.body)
 
     def test_PUT_negative_delete_after(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',
                                                        'container', 'object')
@@ -4518,6 +4525,7 @@ class TestObjectController(unittest.TestCase):
             self.assertTrue('X-Delete-At in past' in res.body)
 
     def test_PUT_delete_at(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         with save_globals():
             given_headers = {}
 
@@ -4894,6 +4902,7 @@ class TestObjectController(unittest.TestCase):
 
     @mock.patch('time.time', new=lambda: STATIC_TIME)
     def test_PUT_x_delete_at_with_fewer_container_replicas(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         self.app.container_ring.set_replicas(2)
 
         delete_at_timestamp = int(time.time()) + 100000
@@ -4929,6 +4938,7 @@ class TestObjectController(unittest.TestCase):
 
     @mock.patch('time.time', new=lambda: STATIC_TIME)
     def test_PUT_x_delete_at_with_more_container_replicas(self):
+        raise SkipTest("X-Delete-At and X-Delete-After are not supported")
         self.app.container_ring.set_replicas(4)
         self.app.expiring_objects_account = 'expires'
         self.app.expiring_objects_container_divisor = 60