From 5b26516ab23288295aeff3764ae0584777547f4d Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Wed, 17 May 2017 18:26:25 -0700 Subject: [PATCH] Increase name-length limits for internal accounts Double account and container name-length limits for accounts starting with auto_create_account_prefix (default: '.') -- these are used internally by Swift, and may need to have some prefix followed by a user-settable value. Related-Change: Ice703dc6d98108ad251c43f824426d026e1f1d97 Change-Id: Ie1ce5ea49b06ab3002c0bd0fad7cea16cea2598e --- swift/proxy/controllers/account.py | 19 +++---- swift/proxy/controllers/base.py | 15 +++++ swift/proxy/controllers/container.py | 7 +-- test/unit/proxy/test_server.py | 85 +++++++++++++++++++++------- 4 files changed, 91 insertions(+), 35 deletions(-) diff --git a/swift/proxy/controllers/account.py b/swift/proxy/controllers/account.py index 6fc94a9891..76e5c1fa13 100644 --- a/swift/proxy/controllers/account.py +++ b/swift/proxy/controllers/account.py @@ -22,7 +22,6 @@ from swift.common.request_helpers import get_listing_content_type from swift.common.middleware.acl import parse_acl, format_acl from swift.common.utils import public from swift.common.constraints import check_metadata -from swift.common import constraints from swift.common.http import HTTP_NOT_FOUND, HTTP_GONE from swift.proxy.controllers.base import Controller, clear_info_cache, \ set_info_cache @@ -53,11 +52,11 @@ class AccountController(Controller): def GETorHEAD(self, req): """Handler for HTTP GET/HEAD requests.""" - if len(self.account_name) > constraints.MAX_ACCOUNT_NAME_LENGTH: + length_limit = self.get_name_length_limit() + if len(self.account_name) > length_limit: resp = HTTPBadRequest(request=req) resp.body = 'Account name length of %d longer than %d' % \ - (len(self.account_name), - constraints.MAX_ACCOUNT_NAME_LENGTH) + (len(self.account_name), length_limit) # Don't cache this. We know the account doesn't exist because # the name is bad; we don't need to cache that because it's # really cheap to recompute. @@ -113,11 +112,11 @@ class AccountController(Controller): error_response = check_metadata(req, 'account') if error_response: return error_response - if len(self.account_name) > constraints.MAX_ACCOUNT_NAME_LENGTH: + length_limit = self.get_name_length_limit() + if len(self.account_name) > length_limit: resp = HTTPBadRequest(request=req) resp.body = 'Account name length of %d longer than %d' % \ - (len(self.account_name), - constraints.MAX_ACCOUNT_NAME_LENGTH) + (len(self.account_name), length_limit) return resp account_partition, accounts = \ self.app.account_ring.get_nodes(self.account_name) @@ -132,11 +131,11 @@ class AccountController(Controller): @public def POST(self, req): """HTTP POST request handler.""" - if len(self.account_name) > constraints.MAX_ACCOUNT_NAME_LENGTH: + length_limit = self.get_name_length_limit() + if len(self.account_name) > length_limit: resp = HTTPBadRequest(request=req) resp.body = 'Account name length of %d longer than %d' % \ - (len(self.account_name), - constraints.MAX_ACCOUNT_NAME_LENGTH) + (len(self.account_name), length_limit) return resp error_response = check_metadata(req, 'account') if error_response: diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 2007006c8c..a135cc2256 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -46,6 +46,7 @@ from swift.common.utils import Timestamp, config_true_value, \ GreenAsyncPile, quorum_size, parse_content_type, \ document_iters_to_http_response_body from swift.common.bufferedhttp import http_connect +from swift.common import constraints from swift.common.exceptions import ChunkReadTimeout, ChunkWriteTimeout, \ ConnectionTimeout, RangeAlreadyComplete from swift.common.header_key_dict import HeaderKeyDict @@ -1924,3 +1925,17 @@ class Controller(object): resp.headers = headers return resp + + def get_name_length_limit(self): + if self.account_name.startswith(self.app.auto_create_account_prefix): + multiplier = 2 + else: + multiplier = 1 + + if self.server_type == 'Account': + return constraints.MAX_ACCOUNT_NAME_LENGTH * multiplier + elif self.server_type == 'Container': + return constraints.MAX_CONTAINER_NAME_LENGTH * multiplier + else: + raise ValueError( + "server_type can only be 'account' or 'container'") diff --git a/swift/proxy/controllers/container.py b/swift/proxy/controllers/container.py index 33791abf60..85af2bec4e 100644 --- a/swift/proxy/controllers/container.py +++ b/swift/proxy/controllers/container.py @@ -19,7 +19,6 @@ import time from six.moves.urllib.parse import unquote from swift.common.utils import public, csv_append, Timestamp from swift.common.constraints import check_metadata -from swift.common import constraints from swift.common.http import HTTP_ACCEPTED, is_success from swift.proxy.controllers.base import Controller, delay_denial, \ cors_validation, set_info_cache, clear_info_cache @@ -149,11 +148,11 @@ class ContainerController(Controller): if not req.environ.get('swift_owner'): for key in self.app.swift_owner_headers: req.headers.pop(key, None) - if len(self.container_name) > constraints.MAX_CONTAINER_NAME_LENGTH: + length_limit = self.get_name_length_limit() + if len(self.container_name) > length_limit: resp = HTTPBadRequest(request=req) resp.body = 'Container name length of %d longer than %d' % \ - (len(self.container_name), - constraints.MAX_CONTAINER_NAME_LENGTH) + (len(self.container_name), length_limit) return resp account_partition, accounts, container_count = \ self.account_info(self.account_name, req) diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 1cbb5ca6bf..255790184e 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -6764,10 +6764,9 @@ class TestContainerController(unittest.TestCase): def assert_status_map(self, method, statuses, expected, raise_exc=False, missing_container=False): with save_globals(): - kwargs = {} + kwargs = {'missing_container': missing_container} if raise_exc: kwargs['raise_exc'] = raise_exc - kwargs['missing_container'] = missing_container set_http_connect(*statuses, **kwargs) self.app.memcache.store = {} req = Request.blank('/v1/a/c', headers={'Content-Length': '0', @@ -7076,17 +7075,36 @@ class TestContainerController(unittest.TestCase): missing_container=True) def test_PUT_max_container_name_length(self): - with save_globals(): - limit = constraints.MAX_CONTAINER_NAME_LENGTH - controller = proxy_server.ContainerController(self.app, 'account', - '1' * limit) - self.assert_status_map(controller.PUT, - (200, 201, 201, 201), 201, - missing_container=True) - controller = proxy_server.ContainerController(self.app, 'account', - '2' * (limit + 1)) - self.assert_status_map(controller.PUT, (201, 201, 201), 400, - missing_container=True) + limit = constraints.MAX_CONTAINER_NAME_LENGTH + controller = proxy_server.ContainerController(self.app, 'account', + '1' * limit) + self.assert_status_map(controller.PUT, (200, 201, 201, 201), 201, + missing_container=True) + controller = proxy_server.ContainerController(self.app, 'account', + '2' * (limit + 1)) + self.assert_status_map(controller.PUT, (), 400, + missing_container=True) + + # internal auto-created-accounts get higher limits + limit *= 2 + controller = proxy_server.ContainerController(self.app, '.account', + '3' * limit) + self.assert_status_map(controller.PUT, (200, 201, 201, 201), 201, + missing_container=True) + controller = proxy_server.ContainerController(self.app, '.account', + '4' * (limit + 1)) + self.assert_status_map(controller.PUT, (), 400, + missing_container=True) + + self.app.auto_create_account_prefix = 'acc' + controller = proxy_server.ContainerController(self.app, 'account', + '1' * limit) + self.assert_status_map(controller.PUT, (200, 201, 201, 201), 201, + missing_container=True) + controller = proxy_server.ContainerController(self.app, 'account', + '2' * (limit + 1)) + self.assert_status_map(controller.PUT, (), 400, + missing_container=True) def test_PUT_connect_exceptions(self): with save_globals(): @@ -8191,14 +8209,39 @@ class TestAccountController(unittest.TestCase): test_status_map((204, 500, 404), 503) def test_PUT_max_account_name_length(self): - with save_globals(): - self.app.allow_account_management = True - limit = constraints.MAX_ACCOUNT_NAME_LENGTH - controller = proxy_server.AccountController(self.app, '1' * limit) - self.assert_status_map(controller.PUT, (201, 201, 201), 201) - controller = proxy_server.AccountController( - self.app, '2' * (limit + 1)) - self.assert_status_map(controller.PUT, (201, 201, 201), 400) + self.app.allow_account_management = True + limit = constraints.MAX_ACCOUNT_NAME_LENGTH + controller = proxy_server.AccountController(self.app, '1' * limit) + self.assert_status_map(controller.PUT, (201, 201, 201), 201) + controller = proxy_server.AccountController( + self.app, '2' * (limit + 1)) + self.assert_status_map(controller.PUT, (), 400) + + # internal auto-created accounts get higher limits + limit *= 2 + controller = proxy_server.AccountController( + self.app, '.' + '3' * (limit - 1)) + self.assert_status_map(controller.PUT, (201, 201, 201), 201) + controller = proxy_server.AccountController( + self.app, '.' + '4' * limit) + self.assert_status_map(controller.PUT, (), 400) + + self.app.auto_create_account_prefix = 'FOO_' + limit /= 2 + controller = proxy_server.AccountController( + self.app, '.' + '5' * (limit - 1)) + self.assert_status_map(controller.PUT, (201, 201, 201), 201) + controller = proxy_server.AccountController( + self.app, '.' + '6' * limit) + self.assert_status_map(controller.PUT, (), 400) + + limit *= 2 + controller = proxy_server.AccountController( + self.app, 'FOO_' + '7' * (limit - 4)) + self.assert_status_map(controller.PUT, (201, 201, 201), 201) + controller = proxy_server.AccountController( + self.app, 'FOO_' + '8' * (limit - 3)) + self.assert_status_map(controller.PUT, (), 400) def test_PUT_connect_exceptions(self): with save_globals():