diff --git a/swift/account/server.py b/swift/account/server.py index fb316a9cc1..592606a21a 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -27,7 +27,7 @@ from swift.account.utils import account_listing_response, get_response_headers from swift.common.db import DatabaseConnectionError, DatabaseAlreadyExists from swift.common.request_helpers import get_param, \ split_and_validate_path, validate_internal_account, \ - validate_internal_container + validate_internal_container, constrain_req_limit from swift.common.utils import get_logger, hash_path, public, \ Timestamp, storage_directory, config_true_value, \ timing_stats, replication, get_log_line, \ @@ -247,16 +247,8 @@ class AccountController(BaseStorageServer): drive, part, account = get_account_name_and_placement(req) prefix = get_param(req, 'prefix') delimiter = get_param(req, 'delimiter') - limit = constraints.ACCOUNT_LISTING_LIMIT - given_limit = get_param(req, 'limit') reverse = config_true_value(get_param(req, 'reverse')) - if given_limit and given_limit.isdigit(): - limit = int(given_limit) - if limit > constraints.ACCOUNT_LISTING_LIMIT: - return HTTPPreconditionFailed( - request=req, - body='Maximum limit is %d' % - constraints.ACCOUNT_LISTING_LIMIT) + limit = constrain_req_limit(req, constraints.ACCOUNT_LISTING_LIMIT) marker = get_param(req, 'marker', '') end_marker = get_param(req, 'end_marker') out_content_type = listing_formats.get_listing_content_type(req) diff --git a/swift/common/middleware/versioned_writes/object_versioning.py b/swift/common/middleware/versioned_writes/object_versioning.py index c2c0bfff1f..46b336d9d7 100644 --- a/swift/common/middleware/versioned_writes/object_versioning.py +++ b/swift/common/middleware/versioned_writes/object_versioning.py @@ -151,11 +151,13 @@ import time from cgi import parse_header from six.moves.urllib.parse import unquote -from swift.common.constraints import MAX_FILE_SIZE, valid_api_version +from swift.common.constraints import MAX_FILE_SIZE, valid_api_version, \ + ACCOUNT_LISTING_LIMIT from swift.common.http import is_success, is_client_error, HTTP_NOT_FOUND, \ HTTP_CONFLICT from swift.common.request_helpers import get_sys_meta_prefix, \ - copy_header_subset, get_reserved_name, split_reserved_name + copy_header_subset, get_reserved_name, split_reserved_name, \ + constrain_req_limit from swift.common.middleware.symlink import TGT_OBJ_SYMLINK_HDR, \ TGT_ETAG_SYSMETA_SYMLINK_HDR, SYMLOOP_EXTEND, ALLOW_RESERVED_NAMES, \ TGT_BYTES_SYSMETA_SYMLINK_HDR, TGT_ACCT_SYMLINK_HDR @@ -183,6 +185,7 @@ SYSMETA_VERSIONS_SYMLINK = get_sys_meta_prefix('object') + 'versions-symlink' def build_listing(*to_splice, **kwargs): reverse = kwargs.pop('reverse') + limit = kwargs.pop('limit') if kwargs: raise TypeError('Invalid keyword arguments received: %r' % kwargs) @@ -195,7 +198,7 @@ def build_listing(*to_splice, **kwargs): itertools.chain(*to_splice), key=merge_key, reverse=reverse, - )).encode('ascii') + )[:limit]).encode('ascii') def non_expiry_header(header): @@ -1197,9 +1200,11 @@ class ContainerContext(ObjectVersioningContext): 'hash': item['hash'], 'last_modified': item['last_modified'], }) + limit = constrain_req_limit(req, ACCOUNT_LISTING_LIMIT) body = build_listing( null_listing, subdir_listing, broken_listing, - reverse=config_true_value(params.get('reverse', 'no'))) + reverse=config_true_value(params.get('reverse', 'no')), + limit=limit) self.update_content_length(len(body)) app_resp = [body] close_if_possible(versions_resp.app_iter) @@ -1260,10 +1265,13 @@ class ContainerContext(ObjectVersioningContext): 'last_modified': item['last_modified'], }) + limit = constrain_req_limit(req, ACCOUNT_LISTING_LIMIT) body = build_listing( null_listing, versions_listing, subdir_listing, broken_listing, - reverse=config_true_value(params.get('reverse', 'no'))) + reverse=config_true_value(params.get('reverse', 'no')), + limit=limit, + ) self.update_content_length(len(body)) app_resp = [body] else: @@ -1333,9 +1341,8 @@ class AccountContext(ObjectVersioningContext): # look-up by name. Ignore 'subdir' items for item in [item for item in versions_listing if 'name' in item]: - name = self._split_versions_container_name( + container_name = self._split_versions_container_name( item['name']) - container_name = bytes_to_wsgi(name.encode('utf-8')) versions_dict[container_name] = item # update bytes from original listing with bytes from @@ -1343,10 +1350,8 @@ class AccountContext(ObjectVersioningContext): if len(versions_dict) > 0: # ignore 'subdir' items for item in [item for item in listing if 'name' in item]: - container_name = bytes_to_wsgi( - item['name'].encode('utf-8')) - if container_name in versions_dict: - v_info = versions_dict.pop(container_name) + if item['name'] in versions_dict: + v_info = versions_dict.pop(item['name']) item['bytes'] = item['bytes'] + v_info['bytes'] # if there are items left in versions_dict, it indicates an @@ -1360,9 +1365,12 @@ class AccountContext(ObjectVersioningContext): item['count'] = 0 # None of these are current listing.append(item) + limit = constrain_req_limit(req, ACCOUNT_LISTING_LIMIT) body = build_listing( listing, - reverse=config_true_value(params.get('reverse', 'no'))) + reverse=config_true_value(params.get('reverse', 'no')), + limit=limit, + ) self.update_content_length(len(body)) app_resp = [body] diff --git a/swift/common/request_helpers.py b/swift/common/request_helpers.py index f7d6ffd888..c120ac2778 100644 --- a/swift/common/request_helpers.py +++ b/swift/common/request_helpers.py @@ -94,6 +94,17 @@ def get_param(req, name, default=None): return value +def constrain_req_limit(req, constrained_limit): + given_limit = get_param(req, 'limit') + limit = constrained_limit + if given_limit and given_limit.isdigit(): + limit = int(given_limit) + if limit > constrained_limit: + raise HTTPPreconditionFailed( + request=req, body='Maximum limit is %d' % constrained_limit) + return limit + + def _validate_internal_name(name, type_='name'): if RESERVED in name and not name.startswith(RESERVED): raise HTTPBadRequest(body='Invalid reserved-namespace %s' % (type_)) diff --git a/swift/container/server.py b/swift/container/server.py index 5d89ec4f88..c8d7647aa6 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -34,7 +34,7 @@ from swift.common.db import DatabaseAlreadyExists from swift.common.container_sync_realms import ContainerSyncRealms from swift.common.request_helpers import get_param, \ split_and_validate_path, is_sys_or_user_meta, \ - validate_internal_container, validate_internal_obj + validate_internal_container, validate_internal_obj, constrain_req_limit from swift.common.utils import get_logger, hash_path, public, \ Timestamp, storage_directory, validate_sync_to, \ config_true_value, timing_stats, replication, \ @@ -689,16 +689,8 @@ class ContainerController(BaseStorageServer): delimiter = get_param(req, 'delimiter') marker = get_param(req, 'marker', '') end_marker = get_param(req, 'end_marker') - limit = constraints.CONTAINER_LISTING_LIMIT - given_limit = get_param(req, 'limit') + limit = constrain_req_limit(req, constraints.CONTAINER_LISTING_LIMIT) reverse = config_true_value(get_param(req, 'reverse')) - if given_limit and given_limit.isdigit(): - limit = int(given_limit) - if limit > constraints.CONTAINER_LISTING_LIMIT: - return HTTPPreconditionFailed( - request=req, - body='Maximum limit is %d' - % constraints.CONTAINER_LISTING_LIMIT) out_content_type = listing_formats.get_listing_content_type(req) try: check_drive(self.root, drive, self.mount_check) diff --git a/test/functional/test_object_versioning.py b/test/functional/test_object_versioning.py index 5548f85f47..cfb21d284b 100644 --- a/test/functional/test_object_versioning.py +++ b/test/functional/test_object_versioning.py @@ -209,6 +209,40 @@ class TestObjectVersioning(TestObjectVersioningBase): self.assertTrue( config_true_value(self.env.container.info()['versions_enabled'])) + def test_account_list_containers(self): + cont_listing = self.env.account.containers() + self.assertEqual(cont_listing, [self.env.container.name, + self.env.unversioned_container.name]) + self.env.account.delete_containers() + prefix = Utils.create_name() + + def get_name(i): + return prefix + '-%02d' % i + + num_container = [15, 20] + for i in range(num_container[1]): + name = get_name(i) + container = self.env.account.container(name) + container.create() + + limit = 5 + cont_listing = self.env.account.containers(parms={'limit': limit}) + self.assertEqual(cont_listing, [get_name(i) for i in range(limit)]) + + for i in range(num_container[0], num_container[1]): + name = get_name(i) + container = self.env.account.container(name) + container.update_metadata( + hdrs={self.env.versions_header_key: 'True'}) + + cont_listing = self.env.account.containers(parms={'limit': limit}) + self.assertEqual(cont_listing, [get_name(i) for i in range(limit)]) + + # we're in charge of getting everything back to normal + self.env.account.delete_containers() + self.env.container.create() + self.env.unversioned_container.create() + def assert_previous_version(self, object_name, version_id, content, content_type, expected_headers={}, not_expected_header_keys=[], diff --git a/test/unit/common/middleware/test_object_versioning.py b/test/unit/common/middleware/test_object_versioning.py index d56674d43d..6e4d76550d 100644 --- a/test/unit/common/middleware/test_object_versioning.py +++ b/test/unit/common/middleware/test_object_versioning.py @@ -3067,6 +3067,20 @@ class ObjectVersioningTestAccountOperations(ObjectVersioningBaseTestCase): }] self.assertEqual(expected, json.loads(body)) + req.query_string = 'limit=1' + status, headers, body = self.call_ov(req) + self.assertEqual(status, '200 OK') + self.assertEqual(1, len(json.loads(body))) + + req.query_string = 'limit=foo' + status, headers, body = self.call_ov(req) + self.assertEqual(status, '200 OK') + self.assertEqual(2, len(json.loads(body))) + + req.query_string = 'limit=100000000000000000000000' + status, headers, body = self.call_ov(req) + self.assertEqual(status, '412 Precondition Failed') + def test_list_containers_prefix(self): listing_body = [{ 'bytes': 0, diff --git a/test/unit/common/test_request_helpers.py b/test/unit/common/test_request_helpers.py index 86ff500df3..70e75d53a0 100644 --- a/test/unit/common/test_request_helpers.py +++ b/test/unit/common/test_request_helpers.py @@ -29,6 +29,19 @@ server_types = ['account', 'container', 'object'] class TestRequestHelpers(unittest.TestCase): + + def test_constrain_req_limit(self): + req = Request.blank('') + self.assertEqual(10, rh.constrain_req_limit(req, 10)) + req = Request.blank('', query_string='limit=1') + self.assertEqual(1, rh.constrain_req_limit(req, 10)) + req = Request.blank('', query_string='limit=1.0') + self.assertEqual(10, rh.constrain_req_limit(req, 10)) + req = Request.blank('', query_string='limit=11') + with self.assertRaises(HTTPException) as raised: + rh.constrain_req_limit(req, 10) + self.assertEqual(raised.exception.status_int, 412) + def test_is_user_meta(self): m_type = 'meta' for st in server_types: