From b36fe03702e76294d530d405ca61f45a7a382057 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Tue, 26 Nov 2013 20:21:10 -0500 Subject: 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 Reviewed-on: http://review.gluster.org/6364 Reviewed-by: Chetan Risbud --- gluster/swift/common/Glusterfs.py | 14 ++++++++ gluster/swift/common/constraints.py | 39 +++++++++++++++++++++- test/functional/gluster_swift_tests.py | 33 +++++++++++++++++-- test/unit/common/test_constraints.py | 59 ++++++++++++++++++++++++++++++++-- test/unit/proxy/test_server.py | 10 ++++++ 5 files changed, 149 insertions(+), 6 deletions(-) diff --git a/gluster/swift/common/Glusterfs.py b/gluster/swift/common/Glusterfs.py index 55012d3..5d2cab1 100644 --- a/gluster/swift/common/Glusterfs.py +++ b/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' diff --git a/gluster/swift/common/constraints.py b/gluster/swift/common/constraints.py index 523277d..7681c49 100644 --- a/gluster/swift/common/constraints.py +++ b/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 diff --git a/test/functional/gluster_swift_tests.py b/test/functional/gluster_swift_tests.py index d7a833e..0a721b6 100644 --- a/test/functional/gluster_swift_tests.py +++ b/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() - - diff --git a/test/unit/common/test_constraints.py b/test/unit/common/test_constraints.py index 7139094..180721c 100644 --- a/test/unit/common/test_constraints.py +++ b/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')) diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 3745a9f..4467b12 100644 --- a/test/unit/proxy/test_server.py +++ b/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 -- cgit