diff --git a/gluster/swift/common/DiskDir.py b/gluster/swift/common/DiskDir.py index 3983db2..6112709 100644 --- a/gluster/swift/common/DiskDir.py +++ b/gluster/swift/common/DiskDir.py @@ -28,6 +28,8 @@ from gluster.swift.common.utils import validate_account, validate_container, \ from gluster.swift.common import Glusterfs from gluster.swift.common.exceptions import FileOrDirNotFoundError, \ GlusterFileSystemIOError +from swift.common.constraints import MAX_META_COUNT, MAX_META_OVERALL_SIZE +from swift.common.swob import HTTPBadRequest DATADIR = 'containers' @@ -201,12 +203,41 @@ class DiskCommon(object): except FileOrDirNotFoundError: return True + def validate_metadata(self, metadata): + """ + Validates that metadata falls within acceptable limits. + + :param metadata: to be validated + :raises: HTTPBadRequest if MAX_META_COUNT or MAX_META_OVERALL_SIZE + is exceeded + """ + meta_count = 0 + meta_size = 0 + for key, (value, timestamp) in metadata.iteritems(): + key = key.lower() + if value != '' and (key.startswith('x-account-meta') or + key.startswith('x-container-meta')): + prefix = 'x-account-meta-' + if key.startswith('x-container-meta-'): + prefix = 'x-container-meta-' + key = key[len(prefix):] + meta_count = meta_count + 1 + meta_size = meta_size + len(key) + len(value) + if meta_count > MAX_META_COUNT: + raise HTTPBadRequest('Too many metadata items; max %d' + % MAX_META_COUNT) + if meta_size > MAX_META_OVERALL_SIZE: + raise HTTPBadRequest('Total metadata too large; max %d' + % MAX_META_OVERALL_SIZE) + def update_metadata(self, metadata, validate_metadata=False): assert self.metadata, "Valid container/account metadata should have " \ "been created by now" if metadata: new_metadata = self.metadata.copy() new_metadata.update(metadata) + if validate_metadata: + self.validate_metadata(new_metadata) if new_metadata != self.metadata: write_metadata(self.datadir, new_metadata) self.metadata = new_metadata @@ -464,7 +495,7 @@ class DiskDir(DiskCommon): # If we create it, ensure we own it. do_chown(self.datadir, self.uid, self.gid) metadata = get_container_metadata(self.datadir) - metadata[X_TIMESTAMP] = timestamp + metadata[X_TIMESTAMP] = (timestamp, 0) write_metadata(self.datadir, metadata) self.metadata = metadata self._dir_exists = True @@ -576,7 +607,7 @@ class DiskAccount(DiskCommon): :param metadata: Metadata to write. """ metadata = get_account_metadata(self.datadir) - metadata[X_TIMESTAMP] = timestamp + metadata[X_TIMESTAMP] = (timestamp, 0) write_metadata(self.datadir, metadata) self.metadata = metadata diff --git a/test/functional/test_account.py b/test/functional/test_account.py index 30cef31..1cc61bc 100755 --- a/test/functional/test_account.py +++ b/test/functional/test_account.py @@ -32,6 +32,42 @@ from test.functional.tests import load_constraint class TestAccount(unittest.TestCase): + def setUp(self): + self.max_meta_count = load_constraint('max_meta_count') + self.max_meta_name_length = load_constraint('max_meta_name_length') + self.max_meta_overall_size = load_constraint('max_meta_overall_size') + self.max_meta_value_length = load_constraint('max_meta_value_length') + + def head(url, token, parsed, conn): + conn.request('HEAD', parsed.path, '', {'X-Auth-Token': token}) + return check_response(conn) + resp = retry(head) + self.existing_metadata = set([ + k for k, v in resp.getheaders() if + k.lower().startswith('x-account-meta')]) + + def tearDown(self): + def head(url, token, parsed, conn): + conn.request('HEAD', parsed.path, '', {'X-Auth-Token': token}) + return check_response(conn) + resp = retry(head) + resp.read() + new_metadata = set( + [k for k, v in resp.getheaders() if + k.lower().startswith('x-account-meta')]) + + def clear_meta(url, token, parsed, conn, remove_metadata_keys): + headers = {'X-Auth-Token': token} + headers.update((k, '') for k in remove_metadata_keys) + conn.request('POST', parsed.path, '', headers) + return check_response(conn) + extra_metadata = list(self.existing_metadata ^ new_metadata) + for i in range(0, len(extra_metadata), 90): + batch = extra_metadata[i:i + 90] + resp = retry(clear_meta, batch) + resp.read() + self.assertEqual(resp.status // 100, 2) + def test_metadata(self): if skip: raise SkipTest @@ -733,6 +769,21 @@ class TestAccount(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_bad_metadata2(self): + if skip: + raise SkipTest + + def post(url, token, parsed, conn, extra_headers): + headers = {'X-Auth-Token': token} + headers.update(extra_headers) + conn.request('POST', parsed.path, '', headers) + return check_response(conn) + + # TODO: Find the test that adds these and remove them. + headers = {'x-remove-account-meta-temp-url-key': 'remove', + 'x-remove-account-meta-temp-url-key-2': 'remove'} + resp = retry(post, headers) + headers = {} for x in xrange(MAX_META_COUNT): headers['X-Account-Meta-%d' % x] = 'v' @@ -746,6 +797,21 @@ class TestAccount(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_bad_metadata3(self): + if skip: + raise SkipTest + + def post(url, token, parsed, conn, extra_headers): + headers = {'X-Auth-Token': token} + headers.update(extra_headers) + conn.request('POST', parsed.path, '', headers) + return check_response(conn) + + # TODO: Find the test that adds these and remove them. + headers = {'x-remove-account-meta-temp-url-key': 'remove', + 'x-remove-account-meta-temp-url-key-2': 'remove'} + resp = retry(post, headers) + headers = {} header_value = 'k' * MAX_META_VALUE_LENGTH size = 0 diff --git a/test/functional/test_container.py b/test/functional/test_container.py index 7c0fd3e..91702e9 100755 --- a/test/functional/test_container.py +++ b/test/functional/test_container.py @@ -382,6 +382,16 @@ class TestContainer(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_POST_bad_metadata2(self): + if skip: + raise SkipTest + + def post(url, token, parsed, conn, extra_headers): + headers = {'X-Auth-Token': token} + headers.update(extra_headers) + conn.request('POST', parsed.path + '/' + self.name, '', headers) + return check_response(conn) + headers = {} for x in xrange(MAX_META_COUNT): headers['X-Container-Meta-%d' % x] = 'v' @@ -395,6 +405,16 @@ class TestContainer(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_POST_bad_metadata3(self): + if skip: + raise SkipTest + + def post(url, token, parsed, conn, extra_headers): + headers = {'X-Auth-Token': token} + headers.update(extra_headers) + conn.request('POST', parsed.path + '/' + self.name, '', headers) + return check_response(conn) + headers = {} header_value = 'k' * MAX_META_VALUE_LENGTH size = 0 diff --git a/test/unit/common/test_diskdir.py b/test/unit/common/test_diskdir.py index f32c3ad..ebc554a 100644 --- a/test/unit/common/test_diskdir.py +++ b/test/unit/common/test_diskdir.py @@ -408,7 +408,7 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.container)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) def test_creation_existing(self): @@ -419,7 +419,7 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.container)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) def test_creation_existing_bad_metadata(self): @@ -432,7 +432,7 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.container)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) def test_empty(self): @@ -954,7 +954,7 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.container)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) broker.delete_db(normalize_timestamp(time())) self.assertTrue(broker.is_deleted()) @@ -1005,7 +1005,7 @@ class TestAccountBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.drive_fullpath)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) def test_creation_bad_metadata(self): @@ -1016,7 +1016,7 @@ class TestAccountBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.drive_fullpath)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) def test_empty(self): @@ -1219,7 +1219,7 @@ class TestAccountBroker(unittest.TestCase): self.assertEqual(os.path.basename(broker.db_file), 'db_file.db') broker.initialize(self.initial_ts) self.assertTrue(os.path.isdir(self.drive_fullpath)) - self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP]) + self.assertEquals(self.initial_ts, broker.metadata[utils.X_TIMESTAMP][0]) self.assertFalse(broker.is_deleted()) broker.delete_db(normalize_timestamp(time())) # Deleting the "db" should be a NOOP