Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
Cloud:OpenStack:Juno
openstack-swift-doc
0001-Fix-metadata-overall-limits-bug.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0001-Fix-metadata-overall-limits-bug.patch of Package openstack-swift-doc
From c020353cc6639beca3dcd2a3dacb85f2b9d83339 Mon Sep 17 00:00:00 2001 From: "Richard (Rick) Hawkins" <richard.hawkins@rackspace.com> Date: Wed, 1 Oct 2014 09:37:47 -0400 Subject: [PATCH] Fix metadata overall limits bug Currently metadata limits are checked on a per request basis. If multiple requests are sent within the per request limits, it is possible to exceed the overall limits. This patch adds an overall metadata check to ensure that multiple requests to add metadata to an account/container will check overall limits before adding the additional metadata. Change-Id: Ib9401a4ee05a9cb737939541bd9b84e8dc239c70 Closes-Bug: 1365350 (cherry picked from commit 5b2c27a5874c2b5b0a333e4955b03544f6a8119f) --- swift/account/server.py | 4 +- swift/common/constraints.py | 5 ++- swift/common/db.py | 34 ++++++++++++++- swift/container/server.py | 4 +- test/functional/test_account.py | 25 +++++++++++ test/functional/test_container.py | 20 +++++++++ test/unit/common/test_db.py | 90 ++++++++++++++++++++++++++++++++++++++- 7 files changed, 175 insertions(+), 7 deletions(-) diff --git a/swift/account/server.py b/swift/account/server.py index 3bdf37f..cf160e9 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -156,7 +156,7 @@ class AccountController(object): for key, value in req.headers.iteritems() if is_sys_or_user_meta('account', key)) if metadata: - broker.update_metadata(metadata) + broker.update_metadata(metadata, validate_metadata=True) if created: return HTTPCreated(request=req) else: @@ -249,7 +249,7 @@ class AccountController(object): for key, value in req.headers.iteritems() if is_sys_or_user_meta('account', key)) if metadata: - broker.update_metadata(metadata) + broker.update_metadata(metadata, validate_metadata=True) return HTTPNoContent(request=req) def __call__(self, env, start_response): diff --git a/swift/common/constraints.py b/swift/common/constraints.py index 24dcbf0..b2af642 100644 --- a/swift/common/constraints.py +++ b/swift/common/constraints.py @@ -99,7 +99,10 @@ FORMAT2CONTENT_TYPE = {'plain': 'text/plain', 'json': 'application/json', def check_metadata(req, target_type): """ - Check metadata sent in the request headers. + Check metadata sent in the request headers. This should only check + that the metadata in the request given is valid. Checks against + account/container overall metadata should be forwarded on to its + respective server to be checked. :param req: request object :param target_type: str: one of: object, container, or account: indicates diff --git a/swift/common/db.py b/swift/common/db.py index c652381..87d7055 100644 --- a/swift/common/db.py +++ b/swift/common/db.py @@ -29,9 +29,11 @@ from tempfile import mkstemp from eventlet import sleep, Timeout import sqlite3 +from swift.common.constraints import MAX_META_COUNT, MAX_META_OVERALL_SIZE from swift.common.utils import json, Timestamp, renamer, \ mkdirs, lock_parent_directory, fallocate from swift.common.exceptions import LockTimeout +from swift.common.swob import HTTPBadRequest #: Whether calls will be made to preallocate disk space for database files. @@ -682,7 +684,35 @@ class DatabaseBroker(object): metadata = {} return metadata - def update_metadata(self, metadata_updates): + @staticmethod + def validate_metadata(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_updates, validate_metadata=False): """ Updates the metadata dict for the database. The metadata dict values are tuples of (value, timestamp) where the timestamp indicates when @@ -715,6 +745,8 @@ class DatabaseBroker(object): value, timestamp = value_timestamp if key not in md or timestamp > md[key][1]: md[key] = value_timestamp + if validate_metadata: + DatabaseBroker.validate_metadata(md) conn.execute('UPDATE %s_stat SET metadata = ?' % self.db_type, (json.dumps(md),)) conn.commit() diff --git a/swift/container/server.py b/swift/container/server.py index 76b7771..6ef7131 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -374,7 +374,7 @@ class ContainerController(object): metadata['X-Container-Sync-To'][0] != \ broker.metadata['X-Container-Sync-To'][0]: broker.set_x_container_sync_points(-1, -1) - broker.update_metadata(metadata) + broker.update_metadata(metadata, validate_metadata=True) resp = self.account_update(req, account, container, broker) if resp: return resp @@ -551,7 +551,7 @@ class ContainerController(object): metadata['X-Container-Sync-To'][0] != \ broker.metadata['X-Container-Sync-To'][0]: broker.set_x_container_sync_points(-1, -1) - broker.update_metadata(metadata) + broker.update_metadata(metadata, validate_metadata=True) return HTTPNoContent(request=req) def __call__(self, env, start_response): diff --git a/test/functional/test_account.py b/test/functional/test_account.py index b6b279d..b276c16 100755 --- a/test/functional/test_account.py +++ b/test/functional/test_account.py @@ -774,6 +774,21 @@ class TestAccount(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_bad_metadata2(self): + if tf.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(self.max_meta_count): headers['X-Account-Meta-%d' % x] = 'v' @@ -787,6 +802,16 @@ class TestAccount(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_bad_metadata3(self): + if tf.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) + headers = {} header_value = 'k' * self.max_meta_value_length size = 0 diff --git a/test/functional/test_container.py b/test/functional/test_container.py index 3a6e1b9..a8ca1ba 100755 --- a/test/functional/test_container.py +++ b/test/functional/test_container.py @@ -401,6 +401,16 @@ class TestContainer(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_POST_bad_metadata2(self): + if tf.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(self.max_meta_count): headers['X-Container-Meta-%d' % x] = 'v' @@ -414,6 +424,16 @@ class TestContainer(unittest.TestCase): resp.read() self.assertEqual(resp.status, 400) + def test_POST_bad_metadata3(self): + if tf.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' * self.max_meta_value_length size = 0 diff --git a/test/unit/common/test_db.py b/test/unit/common/test_db.py index e0c164d..d4f9828 100644 --- a/test/unit/common/test_db.py +++ b/test/unit/common/test_db.py @@ -32,11 +32,14 @@ from mock import patch, MagicMock from eventlet.timeout import Timeout import swift.common.db +from swift.common.constraints import \ + MAX_META_VALUE_LENGTH, MAX_META_COUNT, MAX_META_OVERALL_SIZE from swift.common.db import chexor, dict_factory, get_db_connection, \ DatabaseBroker, DatabaseConnectionError, DatabaseAlreadyExists, \ GreenDBConnection, PICKLE_PROTOCOL from swift.common.utils import normalize_timestamp, mkdirs, json, Timestamp from swift.common.exceptions import LockTimeout +from swift.common.swob import HTTPException from test.unit import with_tempdir @@ -655,7 +658,7 @@ class TestDatabaseBroker(unittest.TestCase): conn.execute('CREATE TABLE test (one TEXT)') conn.execute('CREATE TABLE test_stat (id TEXT)') conn.execute('INSERT INTO test_stat (id) VALUES (?)', - (str(uuid4),)) + (str(uuid4),)) conn.execute('INSERT INTO test (one) VALUES ("1")') conn.commit() stub_called = [False] @@ -1107,6 +1110,91 @@ class TestDatabaseBroker(unittest.TestCase): [first_value, first_timestamp]) self.assert_('Second' not in broker.metadata) + @patch.object(DatabaseBroker, 'validate_metadata') + def test_validate_metadata_is_called_from_update_metadata(self, mock): + broker = self.get_replication_info_tester(metadata=True) + first_timestamp = normalize_timestamp(1) + first_value = '1' + metadata = {'First': [first_value, first_timestamp]} + broker.update_metadata(metadata, validate_metadata=True) + self.assertTrue(mock.called) + + @patch.object(DatabaseBroker, 'validate_metadata') + def test_validate_metadata_is_not_called_from_update_metadata(self, mock): + broker = self.get_replication_info_tester(metadata=True) + first_timestamp = normalize_timestamp(1) + first_value = '1' + metadata = {'First': [first_value, first_timestamp]} + broker.update_metadata(metadata) + self.assertFalse(mock.called) + + def test_metadata_with_max_count(self): + metadata = {} + for c in xrange(MAX_META_COUNT): + key = 'X-Account-Meta-F{0}'.format(c) + metadata[key] = ('B', normalize_timestamp(1)) + key = 'X-Account-Meta-Foo'.format(c) + metadata[key] = ('', normalize_timestamp(1)) + try: + DatabaseBroker.validate_metadata(metadata) + except HTTPException: + self.fail('Unexpected HTTPException') + + def test_metadata_raises_exception_over_max_count(self): + metadata = {} + for c in xrange(MAX_META_COUNT + 1): + key = 'X-Account-Meta-F{0}'.format(c) + metadata[key] = ('B', normalize_timestamp(1)) + message = '' + try: + DatabaseBroker.validate_metadata(metadata) + except HTTPException as e: + message = str(e) + self.assertEqual(message, '400 Bad Request') + + def test_metadata_with_max_overall_size(self): + metadata = {} + metadata_value = 'v' * MAX_META_VALUE_LENGTH + size = 0 + x = 0 + while size < (MAX_META_OVERALL_SIZE - 4 + - MAX_META_VALUE_LENGTH): + size += 4 + MAX_META_VALUE_LENGTH + metadata['X-Account-Meta-%04d' % x] = (metadata_value, + normalize_timestamp(1)) + x += 1 + if MAX_META_OVERALL_SIZE - size > 1: + metadata['X-Account-Meta-k'] = ( + 'v' * (MAX_META_OVERALL_SIZE - size - 1), + normalize_timestamp(1)) + try: + DatabaseBroker.validate_metadata(metadata) + except HTTPException: + self.fail('Unexpected HTTPException') + + def test_metadata_raises_exception_over_max_overall_size(self): + metadata = {} + metadata_value = 'k' * MAX_META_VALUE_LENGTH + size = 0 + x = 0 + while size < (MAX_META_OVERALL_SIZE - 4 + - MAX_META_VALUE_LENGTH): + size += 4 + MAX_META_VALUE_LENGTH + metadata['X-Account-Meta-%04d' % x] = (metadata_value, + normalize_timestamp(1)) + x += 1 + if MAX_META_OVERALL_SIZE - size > 1: + metadata['X-Account-Meta-k'] = ( + 'v' * (MAX_META_OVERALL_SIZE - size - 1), + normalize_timestamp(1)) + metadata['X-Account-Meta-k2'] = ('v', normalize_timestamp(1)) + message = '' + try: + DatabaseBroker.validate_metadata(metadata) + except HTTPException as e: + message = str(e) + self.assertEqual(message, '400 Bad Request') + if __name__ == '__main__': unittest.main() -- 2.6.0
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
