x/swiftonfile
Code Issues Proposed changes

Backport: Fix metadata overall limits bug

Browse Source 
This is a manual backport of the following upstream fix:
https://review.openstack.org/126645

Change-Id: Ib807a3271f34fa5a58ce3305566a32362055ee07
Signed-off-by: Prashanth Pai <ppai@redhat.com>
This commit is contained in:
Prashanth Pai 2015-08-21 12:20:45 +05:30
parent ad0bb79827
commit 79ea52a745
4 changed files with 126 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
gluster/swift/common/DiskDir.py
View File

@ -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

66
test/functional/test_account.py
View File

@ -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

20
test/functional/test_container.py
View File

@ -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

14
test/unit/common/test_diskdir.py
View File

@ -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