diff options
author | Daniel Wakefield <daniel.wakefield@hp.com> | 2015-03-04 14:01:55 +0000 |
---|---|---|
committer | Daniel Wakefield <daniel.wakefield@hp.com> | 2015-03-04 14:01:55 +0000 |
commit | 13780f37c3f5e5f18f10f131d1ef39c010457e87 (patch) | |
tree | 88fcf8f9b626a40bac3a74555a3afb7cbb39063a /tests | |
parent | 925c01ebfbdfb6478a3786f24a9572deae40f8f8 (diff) | |
download | python-swiftclient-13780f37c3f5e5f18f10f131d1ef39c010457e87.tar.gz |
Add improvements to MD5 validation.
With MD5Sum checking being added, a concern was brought up that It was
a change with no possibility of reverting to the old behaviour.
This change adds the flag '--ignore-checksum' to the upload subcommand
allowing the checks to be turned off.
Changed occurrences of the magic string for a null md5 to use a descriptive
constant instead.
Updated Error messages generated when validation fails. They should now be more descriptive
and not output a literal newline sequence.
Change-Id: Id1756cbb6700bb7e38f0ee0e75bc535e37f777ed
Diffstat (limited to 'tests')
-rw-r--r-- | tests/unit/test_service.py | 77 | ||||
-rw-r--r-- | tests/unit/test_shell.py | 20 | ||||
-rw-r--r-- | tests/unit/test_swiftclient.py | 5 | ||||
-rw-r--r-- | tests/unit/utils.py | 3 |
4 files changed, 73 insertions, 32 deletions
diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 3309813..000b0ba 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -563,8 +563,8 @@ class TestServiceUpload(testtools.TestCase): if hasattr(self, 'assertDictEqual'): self.assertDictEqual(a, b, m) else: - self.assertTrue(isinstance(a, dict), m) - self.assertTrue(isinstance(b, dict), m) + self.assertIsInstance(a, dict, m) + self.assertIsInstance(b, dict, m) self.assertEqual(len(a), len(b), m) for k, v in a.items(): self.assertIn(k, b, m) @@ -605,7 +605,8 @@ class TestServiceUpload(testtools.TestCase): segment_size=10, segment_index=2, obj_name='test_o', - options={'segment_container': None}) + options={'segment_container': None, + 'checksum': True}) self._assertDictEqual(r, expected_r) @@ -623,6 +624,45 @@ class TestServiceUpload(testtools.TestCase): self.assertEqual(contents.read(), b'b' * 10) self.assertEqual(contents.get_md5sum(), md5(b'b' * 10).hexdigest()) + def test_etag_mismatch_with_ignore_checksum(self): + def _consuming_conn(*a, **kw): + contents = a[2] + contents.read() # Force md5 calculation + return 'badresponseetag' + + with tempfile.NamedTemporaryFile() as f: + f.write(b'a' * 10) + f.write(b'b' * 10) + f.write(b'c' * 10) + f.flush() + + mock_conn = mock.Mock() + mock_conn.put_object.side_effect = _consuming_conn + type(mock_conn).attempts = mock.PropertyMock(return_value=2) + + s = SwiftService() + r = s._upload_segment_job(conn=mock_conn, + path=f.name, + container='test_c', + segment_name='test_s_1', + segment_start=10, + segment_size=10, + segment_index=2, + obj_name='test_o', + options={'segment_container': None, + 'checksum': False}) + + self.assertNotIn('error', r) + self.assertEqual(mock_conn.put_object.call_count, 1) + mock_conn.put_object.assert_called_with('test_c_segments', + 'test_s_1', + mock.ANY, + content_length=10, + response_dict={}) + contents = mock_conn.put_object.call_args[0][2] + # Check that md5sum is not calculated. + self.assertEqual(contents.get_md5sum(), '') + def test_upload_segment_job_etag_mismatch(self): def _consuming_conn(*a, **kw): contents = a[2] @@ -648,10 +688,11 @@ class TestServiceUpload(testtools.TestCase): segment_size=10, segment_index=2, obj_name='test_o', - options={'segment_container': None}) + options={'segment_container': None, + 'checksum': True}) self.assertIn('error', r) - self.assertTrue(r['error'].value.find('md5 did not match') >= 0) + self.assertIn('md5 mismatch', str(r['error'])) self.assertEqual(mock_conn.put_object.call_count, 1) mock_conn.put_object.assert_called_with('test_c_segments', @@ -664,7 +705,7 @@ class TestServiceUpload(testtools.TestCase): def test_upload_object_job_file(self): # Uploading a file results in the file object being wrapped in a - # LengthWrapper. This test sets the options is such a way that much + # LengthWrapper. This test sets the options in such a way that much # of _upload_object_job is skipped bringing the critical path down # to around 60 lines to ease testing. with tempfile.NamedTemporaryFile() as f: @@ -696,12 +737,11 @@ class TestServiceUpload(testtools.TestCase): 'skip_identical': False, 'leave_segments': True, 'header': '', - 'segment_size': 0}) + 'segment_size': 0, + 'checksum': True}) - # Check for mtime and path separately as they are calculated - # from the temp file and will be different each time. mtime = float(r['headers']['x-object-meta-mtime']) - self.assertAlmostEqual(mtime, expected_mtime, delta=1) + self.assertAlmostEqual(mtime, expected_mtime, delta=0.5) del r['headers']['x-object-meta-mtime'] self.assertEqual(r['path'], f.name) @@ -718,7 +758,8 @@ class TestServiceUpload(testtools.TestCase): self.assertIsInstance(contents, utils.LengthWrapper) self.assertEqual(len(contents), 30) # This read forces the LengthWrapper to calculate the md5 - # for the read content. + # for the read content. This also checks that LengthWrapper was + # initialized with md5=True self.assertEqual(contents.read(), b'a' * 30) self.assertEqual(contents.get_md5sum(), md5(b'a' * 30).hexdigest()) @@ -740,7 +781,7 @@ class TestServiceUpload(testtools.TestCase): 'success': True, 'path': None, } - expected_mtime = round(time.time()) + expected_mtime = float(time.time()) mock_conn = mock.Mock() mock_conn.put_object.return_value = '' @@ -755,10 +796,11 @@ class TestServiceUpload(testtools.TestCase): 'skip_identical': False, 'leave_segments': True, 'header': '', - 'segment_size': 0}) + 'segment_size': 0, + 'checksum': True}) mtime = float(r['headers']['x-object-meta-mtime']) - self.assertAlmostEqual(mtime, expected_mtime, delta=10) + self.assertAlmostEqual(mtime, expected_mtime, delta=0.5) del r['headers']['x-object-meta-mtime'] self._assertDictEqual(r, expected_r) @@ -771,7 +813,7 @@ class TestServiceUpload(testtools.TestCase): contents = mock_conn.put_object.call_args[0][2] self.assertIsInstance(contents, utils.ReadableToIterable) self.assertEqual(contents.chunk_size, 65536) - # next retreives the first chunk of the stream or len(chunk_size) + # next retrieves the first chunk of the stream or len(chunk_size) # or less, it also forces the md5 to be calculated. self.assertEqual(next(contents), b'a' * 30) self.assertEqual(contents.get_md5sum(), md5(b'a' * 30).hexdigest()) @@ -801,11 +843,12 @@ class TestServiceUpload(testtools.TestCase): 'skip_identical': False, 'leave_segments': True, 'header': '', - 'segment_size': 0}) + 'segment_size': 0, + 'checksum': True}) self.assertEqual(r['success'], False) self.assertIn('error', r) - self.assertTrue(r['error'].value.find('md5 did not match') >= 0) + self.assertIn('md5 mismatch', str(r['error'])) self.assertEqual(mock_conn.put_object.call_count, 1) expected_headers = {'x-object-meta-mtime': mock.ANY} diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py index 28aea7d..dada0fa 100644 --- a/tests/unit/test_shell.py +++ b/tests/unit/test_shell.py @@ -32,6 +32,7 @@ import swiftclient.utils from os.path import basename, dirname from tests.unit.test_swiftclient import MockHttpTest from tests.unit.utils import CaptureOutput, fake_get_auth_keystone +from swiftclient.utils import EMPTY_ETAG if six.PY2: @@ -314,7 +315,7 @@ class TestShell(unittest.TestCase): 'etag': '2cbbfe139a744d6abbe695e17f3c1991'}, objcontent), ({'content-type': 'text/plain', - 'etag': 'd41d8cd98f00b204e9800998ecf8427e'}, + 'etag': EMPTY_ETAG}, '') ] @@ -370,7 +371,7 @@ class TestShell(unittest.TestCase): @mock.patch('swiftclient.service.Connection') def test_download_no_content_type(self, connection): connection.return_value.get_object.return_value = [ - {'etag': 'd41d8cd98f00b204e9800998ecf8427e'}, + {'etag': EMPTY_ETAG}, ''] # Test downloading whole container @@ -400,8 +401,7 @@ class TestShell(unittest.TestCase): def test_upload(self, connection, walk): connection.return_value.head_object.return_value = { 'content-length': '0'} - connection.return_value.put_object.return_value = ( - 'd41d8cd98f00b204e9800998ecf8427e') + connection.return_value.put_object.return_value = EMPTY_ETAG connection.return_value.attempts = 0 argv = ["", "upload", "container", self.tmpfile, "-H", "X-Storage-Policy:one"] @@ -477,8 +477,7 @@ class TestShell(unittest.TestCase): connection.return_value.get_object.return_value = ({}, json.dumps( [{'name': 'container1/old_seg1'}, {'name': 'container2/old_seg2'}] )) - connection.return_value.put_object.return_value = ( - 'd41d8cd98f00b204e9800998ecf8427e') + connection.return_value.put_object.return_value = EMPTY_ETAG swiftclient.shell.main(argv) connection.return_value.put_object.assert_called_with( 'container', @@ -508,8 +507,7 @@ class TestShell(unittest.TestCase): connection.return_value.head_object.return_value = { 'content-length': '0'} connection.return_value.attempts = 0 - connection.return_value.put_object.return_value = ( - 'd41d8cd98f00b204e9800998ecf8427e') + connection.return_value.put_object.return_value = EMPTY_ETAG argv = ["", "upload", "container", self.tmpfile, "-S", "10", "-C", "container"] with open(self.tmpfile, "wb") as fh: @@ -1660,9 +1658,8 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest): def test_download_with_read_write_access(self): req_handler = self._fake_cross_account_auth(True, True) - empty_str_etag = 'd41d8cd98f00b204e9800998ecf8427e' fake_conn = self.fake_http_connection(403, on_request=req_handler, - etags=[empty_str_etag]) + etags=[EMPTY_ETAG]) args, env = self._make_cmd('download', cmd_args=[self.cont, self.obj.lstrip('/'), @@ -1683,9 +1680,8 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest): def test_download_with_read_only_access(self): req_handler = self._fake_cross_account_auth(True, False) - empty_str_etag = 'd41d8cd98f00b204e9800998ecf8427e' fake_conn = self.fake_http_connection(403, on_request=req_handler, - etags=[empty_str_etag]) + etags=[EMPTY_ETAG]) args, env = self._make_cmd('download', cmd_args=[self.cont, self.obj.lstrip('/'), diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py index 9ebcff5..d199050 100644 --- a/tests/unit/test_swiftclient.py +++ b/tests/unit/test_swiftclient.py @@ -32,6 +32,7 @@ from six.moves import reload_module from .utils import MockHttpTest, fake_get_auth_keystone, StubResponse +from swiftclient.utils import EMPTY_ETAG from swiftclient import client as c import swiftclient.utils import swiftclient @@ -102,8 +103,7 @@ class MockHttpResponse(object): self.requests_params = None self.verify = verify self.md5sum = md5() - # zero byte hash - self.headers = {'etag': '"d41d8cd98f00b204e9800998ecf8427e"'} + self.headers = {'etag': '"%s"' % EMPTY_ETAG} if headers: self.headers.update(headers) @@ -806,6 +806,7 @@ class TestPutObject(MockHttpTest): chunk_size=chunk_size) self.assertNotEquals(etag, contents.get_md5sum()) + self.assertEquals(etag, 'badresponseetag') self.assertEquals(raw_data_md5, contents.get_md5sum()) def test_md5_match(self): diff --git a/tests/unit/utils.py b/tests/unit/utils.py index 88d6d12..3e87ea2 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -25,6 +25,7 @@ from six.moves import reload_module from six.moves.urllib.parse import urlparse, ParseResult from swiftclient import client as c from swiftclient import shell as s +from swiftclient.utils import EMPTY_ETAG def fake_get_auth_keystone(expected_os_options=None, exc=None, @@ -127,7 +128,7 @@ def fake_http_connect(*code_iter, **kwargs): 'last-modified': self.timestamp, 'x-object-meta-test': 'testing', 'etag': - self.etag or '"d41d8cd98f00b204e9800998ecf8427e"', + self.etag or '"%s"' % EMPTY_ETAG, 'x-works': 'yes', 'x-account-container-count': 12345} if not self.timestamp: |