diff --git a/swift/common/db.py b/swift/common/db.py index bd88f881e1..1e3fc385a7 100644 --- a/swift/common/db.py +++ b/swift/common/db.py @@ -16,6 +16,7 @@ """ Database code for Swift """ from contextlib import contextmanager, closing +import base64 import hashlib import json import logging @@ -692,10 +693,10 @@ class DatabaseBroker(object): with open(self.pending_file, 'a+b') as fp: # Colons aren't used in base64 encoding; so they are our # delimiter - fp.write(':') - fp.write(pickle.dumps( + fp.write(b':') + fp.write(base64.b64encode(pickle.dumps( self.make_tuple_for_pickle(record), - protocol=PICKLE_PROTOCOL).encode('base64')) + protocol=PICKLE_PROTOCOL))) fp.flush() def _skip_commit_puts(self): @@ -726,7 +727,7 @@ class DatabaseBroker(object): self.merge_items(item_list) return with open(self.pending_file, 'r+b') as fp: - for entry in fp.read().split(':'): + for entry in fp.read().split(b':'): if entry: try: self._commit_puts_load(item_list, entry) @@ -873,19 +874,24 @@ class DatabaseBroker(object): meta_count = 0 meta_size = 0 for key, (value, timestamp) in metadata.items(): + if key and not isinstance(key, six.text_type): + if not check_utf8(key): + raise HTTPBadRequest('Metadata must be valid UTF-8') + # Promote to a natural string for the checks below + if six.PY3: + key = key.decode('utf8') + if value and not isinstance(value, six.text_type): + if not check_utf8(value): + raise HTTPBadRequest('Metadata must be valid UTF-8') key = key.lower() - if value != '' and (key.startswith('x-account-meta') or - key.startswith('x-container-meta')): + if len(value) != 0 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) - bad_key = key and not check_utf8(key) - bad_value = value and not check_utf8(value) - if bad_key or bad_value: - raise HTTPBadRequest('Metadata must be valid UTF-8') if meta_count > MAX_META_COUNT: raise HTTPBadRequest('Too many metadata items; max %d' % MAX_META_COUNT) diff --git a/test/unit/common/test_db.py b/test/unit/common/test_db.py index 3df59f2957..0f48c94fbb 100644 --- a/test/unit/common/test_db.py +++ b/test/unit/common/test_db.py @@ -23,6 +23,7 @@ from shutil import rmtree, copy from uuid import uuid4 import six.moves.cPickle as pickle +import base64 import json import sqlite3 import itertools @@ -33,6 +34,8 @@ from mock import patch, MagicMock from eventlet.timeout import Timeout from six.moves import range +import six + import swift.common.db from swift.common.constraints import \ MAX_META_VALUE_LENGTH, MAX_META_COUNT, MAX_META_OVERALL_SIZE @@ -271,7 +274,7 @@ class ExampleBroker(DatabaseBroker): conn.commit() def _commit_puts_load(self, item_list, entry): - (name, timestamp, deleted) = pickle.loads(entry.decode('base64')) + (name, timestamp, deleted) = pickle.loads(base64.b64decode(entry)) item_list.append({ 'name': name, 'created_at': timestamp, @@ -288,10 +291,10 @@ class ExampleBroker(DatabaseBroker): self.merge_items([record]) return with open(self.pending_file, 'a+b') as fp: - fp.write(':') - fp.write(pickle.dumps( + fp.write(b':') + fp.write(base64.b64encode(pickle.dumps( (name, timestamp, deleted), - protocol=PICKLE_PROTOCOL).encode('base64')) + protocol=PICKLE_PROTOCOL))) fp.flush() def put_test(self, name, timestamp): @@ -517,7 +520,15 @@ class TestExampleBroker(unittest.TestCase): storage_policy_index=int(self.policy)) self.assertEqual(broker.metadata, {}) self.assertEqual(broker.get_raw_metadata(), '') - key = u'test\u062a'.encode('utf-8') + # This is not obvious. The actual JSON in the database is the same: + # '{"test\\u062a": ["value\\u062a", "0000000001.00000"]}' + # The only difference is what reading it produces on py2 and py3. + # We use native strings for metadata keys (see native_str_keys()), + # so keys are different. + if six.PY2: + key = u'test\u062a'.encode('utf-8') + else: + key = u'test\u062a' value = u'value\u062a' metadata = { key: [value, Timestamp(1).internal] @@ -676,7 +687,7 @@ class TestDatabaseBroker(unittest.TestCase): stub_dict = {} def stub(*args, **kwargs): - for key in stub_dict.keys(): + for key in list(stub_dict.keys()): del stub_dict[key] stub_dict['args'] = args for key, value in kwargs.items(): @@ -1413,20 +1424,20 @@ class TestDatabaseBroker(unittest.TestCase): # load file and merge with open(broker.pending_file, 'wb') as fd: - fd.write(':1:2:99') + fd.write(b':1:2:99') with patch.object(broker, 'merge_items') as mock_merge_items: broker._commit_puts_load = lambda l, e: l.append(e) broker._commit_puts() - mock_merge_items.assert_called_once_with(['1', '2', '99']) + mock_merge_items.assert_called_once_with([b'1', b'2', b'99']) self.assertEqual(0, os.path.getsize(broker.pending_file)) # load file and merge with given list with open(broker.pending_file, 'wb') as fd: - fd.write(':bad') + fd.write(b':bad') with patch.object(broker, 'merge_items') as mock_merge_items: broker._commit_puts_load = lambda l, e: l.append(e) - broker._commit_puts(['not']) - mock_merge_items.assert_called_once_with(['not', 'bad']) + broker._commit_puts([b'not']) + mock_merge_items.assert_called_once_with([b'not', b'bad']) self.assertEqual(0, os.path.getsize(broker.pending_file)) # skip_commits True - no merge @@ -1435,14 +1446,14 @@ class TestDatabaseBroker(unittest.TestCase): broker._initialize = MagicMock() broker.initialize(Timestamp.now()) with open(broker.pending_file, 'wb') as fd: - fd.write(':ignored') + fd.write(b':ignored') with patch.object(broker, 'merge_items') as mock_merge_items: with self.assertRaises(DatabaseConnectionError) as cm: - broker._commit_puts(['hmmm']) + broker._commit_puts([b'hmmm']) mock_merge_items.assert_not_called() self.assertIn('commits not accepted', str(cm.exception)) with open(broker.pending_file, 'rb') as fd: - self.assertEqual(':ignored', fd.read()) + self.assertEqual(b':ignored', fd.read()) def test_put_record(self): db_file = os.path.join(self.testdir, '1.db') @@ -1457,9 +1468,10 @@ class TestDatabaseBroker(unittest.TestCase): mock_commit_puts.assert_not_called() with open(broker.pending_file, 'rb') as fd: pending = fd.read() - items = pending.split(':') + items = pending.split(b':') self.assertEqual(['PINKY'], - [pickle.loads(i.decode('base64')) for i in items[1:]]) + [pickle.loads(base64.b64decode(i)) + for i in items[1:]]) # record appended with patch.object(broker, '_commit_puts') as mock_commit_puts: @@ -1467,15 +1479,16 @@ class TestDatabaseBroker(unittest.TestCase): mock_commit_puts.assert_not_called() with open(broker.pending_file, 'rb') as fd: pending = fd.read() - items = pending.split(':') + items = pending.split(b':') self.assertEqual(['PINKY', 'PERKY'], - [pickle.loads(i.decode('base64')) for i in items[1:]]) + [pickle.loads(base64.b64decode(i)) + for i in items[1:]]) # pending file above cap cap = swift.common.db.PENDING_CAP while os.path.getsize(broker.pending_file) < cap: with open(broker.pending_file, 'ab') as fd: - fd.write('x' * 100000) + fd.write(b'x' * 100000) with patch.object(broker, '_commit_puts') as mock_commit_puts: broker.put_record('direct') mock_commit_puts.called_once_with(['direct']) diff --git a/tox.ini b/tox.ini index 74cc8ce358..5f285c5b01 100644 --- a/tox.ini +++ b/tox.ini @@ -49,6 +49,7 @@ commands = test/unit/common/test_base_storage_server.py \ test/unit/common/test_bufferedhttp.py \ test/unit/common/test_constraints.py \ + test/unit/common/test_db.py \ test/unit/common/test_daemon.py \ test/unit/common/test_direct_client.py \ test/unit/common/test_exceptions.py \