From d0d5533940146b453e53d003b5f461afef9e00e4 Mon Sep 17 00:00:00 2001 From: Jianjian Huo Date: Thu, 2 Nov 2023 16:50:53 -0700 Subject: [PATCH] Utils: fix Namespace and ShardRange attribute encoding in py2. Ensure name/account/container are always consistent and always encode utf8 in py2. Co-Authored-By: Alistair Coles Co-Authored-By: Matthew Oliver Change-Id: Ia5374f55adf80fef92a92d916b3f89297463c673 --- swift/common/utils/__init__.py | 44 +++++++++++++++++------ test/unit/common/test_utils.py | 66 ++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 10 deletions(-) diff --git a/swift/common/utils/__init__.py b/swift/common/utils/__init__.py index 6af137bb2e..9c8761f3a0 100644 --- a/swift/common/utils/__init__.py +++ b/swift/common/utils/__init__.py @@ -4546,7 +4546,7 @@ class Namespace(object): :param upper: the upper bound of object names contained in the namespace; the upper bound *is* included in the namespace. """ - __slots__ = ('_lower', '_upper', 'name') + __slots__ = ('_lower', '_upper', '_name') @functools.total_ordering class MaxBound(NamespaceOuterBound): @@ -4641,6 +4641,14 @@ class Namespace(object): raise TypeError('must be a string type') return self._encode(bound) + @property + def name(self): + return self._name + + @name.setter + def name(self, path): + self._name = self._encode(path) + @property def lower(self): return self._lower @@ -5024,7 +5032,7 @@ class ShardRange(Namespace): CLEAVING_STATES = SHRINKING_STATES + SHARDING_STATES __slots__ = ( - 'account', 'container', + '_account', '_container', '_timestamp', '_meta_timestamp', '_state_timestamp', '_epoch', '_deleted', '_state', '_count', '_bytes', '_tombstones', '_reported') @@ -5034,13 +5042,13 @@ class ShardRange(Namespace): object_count=0, bytes_used=0, meta_timestamp=None, deleted=False, state=None, state_timestamp=None, epoch=None, reported=False, tombstones=-1, **kwargs): + self._account = self._container = None super(ShardRange, self).__init__(name=name, lower=lower, upper=upper) - self.account = self.container = self._timestamp = \ - self._meta_timestamp = self._state_timestamp = self._epoch = None + self._timestamp = self._meta_timestamp = self._state_timestamp = \ + self._epoch = None self._deleted = False self._state = None - self.name = name self.timestamp = timestamp self.deleted = deleted self.object_count = object_count @@ -5197,18 +5205,34 @@ class ShardRange(Namespace): return timestamp return Timestamp(timestamp) + @property + def account(self): + return self._account + + @account.setter + def account(self, value): + self._account = self._encode(value) + + @property + def container(self): + return self._container + + @container.setter + def container(self, value): + self._container = self._encode(value) + @property def name(self): return '%s/%s' % (self.account, self.container) @name.setter - def name(self, path): - path = self._encode(path) - if not path or len(path.split('/')) != 2 or not all(path.split('/')): + def name(self, name): + name = self._encode(name) + if not name or len(name.split('/')) != 2 or not all(name.split('/')): raise ValueError( "Name must be of the form '/', got %r" % - path) - self.account, self.container = path.split('/') + name) + self._account, self._container = name.split('/') @property def timestamp(self): diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 0d022ab150..630a1a5f59 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -7456,6 +7456,23 @@ class TestNamespace(unittest.TestCase): self.assertEqual(exp_upper, ns.upper_str) self.assertEqual(exp_upper + '\x00', ns.end_marker) + def test_unicode_name(self): + shard_bounds = ('', 'ham', 'pie', u'\N{SNOWMAN}', u'\U0001F334', '') + bounds = [(l, u) for l, u in zip(shard_bounds[:-1], shard_bounds[1:])] + namespaces = [utils.Namespace('.shards_a/c_%s' % upper, lower, upper) + for lower, upper in bounds] + if six.PY2: + exp_bounds = [(l.encode('utf8'), u.encode('utf8')) + for l, u in bounds] + else: + exp_bounds = bounds + for i in range(len(exp_bounds)): + self.assertEqual(namespaces[i].name, + '.shards_a/c_%s' % exp_bounds[i][1]) + self.assertEqual(namespaces[i].lower_str, exp_bounds[i][0]) + + self.assertEqual(namespaces[i].upper_str, exp_bounds[i][1]) + def test_entire_namespace(self): # test entire range (no boundaries) entire = utils.Namespace('a/test', None, None) @@ -8027,6 +8044,55 @@ class TestShardRange(unittest.TestCase): self._check_to_from_dict('l', 'u') self._check_to_from_dict('', '') + def _check_name_account_container(self, sr, exp_name): + # check that the name, account, container properties are consistent + exp_account, exp_container = exp_name.split('/') + if six.PY2: + self.assertEqual(exp_name.encode('utf8'), sr.name) + self.assertEqual(exp_account.encode('utf8'), sr.account) + self.assertEqual(exp_container.encode('utf8'), sr.container) + else: + self.assertEqual(exp_name, sr.name) + self.assertEqual(exp_account, sr.account) + self.assertEqual(exp_container, sr.container) + + def test_name(self): + # constructor + path = 'a/c' + sr = utils.ShardRange(path, 0, 'l', 'u') + self._check_name_account_container(sr, path) + # name setter + path = 'a2/c2' + sr.name = path + self._check_name_account_container(sr, path) + + # constructor + path = u'\u1234a/\n{SNOWMAN}' + sr = utils.ShardRange(path, 0, 'l', 'u') + self._check_name_account_container(sr, path) + # name setter + path = u'\n{SNOWMAN}/\u1234c' + sr.name = path + self._check_name_account_container(sr, path) + + def test_account(self): + path = 'a/c' + sr = utils.ShardRange(path, 0, 'l', 'u') + self._check_name_account_container(sr, path) + sr.account = 'a2' + self._check_name_account_container(sr, 'a2/c') + sr.account = u'\n{SNOWMAN}' + self._check_name_account_container(sr, u'\n{SNOWMAN}/c') + + def test_container(self): + path = 'a/c' + sr = utils.ShardRange(path, 0, 'l', 'u') + self._check_name_account_container(sr, path) + sr.container = 'c2' + self._check_name_account_container(sr, 'a/c2') + sr.container = u'\n{SNOWMAN}' + self._check_name_account_container(sr, u'a/\n{SNOWMAN}') + def test_timestamp_setter(self): ts_1 = next(self.ts_iter) sr = utils.ShardRange('a/test', ts_1, 'l', 'u', 0, 0, None)