From aa40337deda851935700897c116723b220bac148 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 20:42:36 -0300 Subject: [PATCH 1/7] test that invalid blobs raise proper exception --- tests/unit/stream/test_stream_descriptor.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/stream/test_stream_descriptor.py b/tests/unit/stream/test_stream_descriptor.py index a0ffdb764..686eaadef 100644 --- a/tests/unit/stream/test_stream_descriptor.py +++ b/tests/unit/stream/test_stream_descriptor.py @@ -4,6 +4,7 @@ import tempfile import shutil import json +from lbrynet.blob.blob_file import BlobFile from torba.testcase import AsyncioTestCase from lbrynet.conf import Config from lbrynet.error import InvalidStreamDescriptorError @@ -107,3 +108,15 @@ class TestRecoverOldStreamDescriptors(AsyncioTestCase): self.assertEqual(stream_hash, descriptor.get_stream_hash()) self.assertEqual(sd_hash, descriptor.calculate_old_sort_sd_hash()) self.assertNotEqual(sd_hash, descriptor.calculate_sd_hash()) + + async def test_decode_corrupt_blob_raises_proper_exception(self): + loop = asyncio.get_event_loop() + tmp_dir = tempfile.mkdtemp() + self.addCleanup(lambda: shutil.rmtree(tmp_dir)) + sd_hash = '9313d1807551186126acc3662e74d9de29cede78d4f133349ace846273ef116b9bb86be86c54509eb84840e4b032f6b2' + with open(os.path.join(tmp_dir, sd_hash), 'wb') as handle: + handle.write(b'doesnt work') + with self.assertRaises(InvalidStreamDescriptorError): + await StreamDescriptor.from_stream_descriptor_blob( + loop, tmp_dir, BlobFile(loop, tmp_dir, sd_hash) + ) From 4e7d88311fbb5a41a7837387e2b49256d8380041 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 20:42:51 -0300 Subject: [PATCH 2/7] raise proper exception on undecode-able blobs --- lbrynet/stream/descriptor.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lbrynet/stream/descriptor.py b/lbrynet/stream/descriptor.py index ad676a38e..80d067ca4 100644 --- a/lbrynet/stream/descriptor.py +++ b/lbrynet/stream/descriptor.py @@ -131,7 +131,10 @@ class StreamDescriptor: assert os.path.isfile(blob.file_path) with open(blob.file_path, 'rb') as f: json_bytes = f.read() - decoded = json.loads(json_bytes.decode()) + try: + decoded = json.loads(json_bytes.decode()) + except json.JSONDecodeError: + raise InvalidStreamDescriptorError("Does not decode as valid JSON") if decoded['blobs'][-1]['length'] != 0: raise InvalidStreamDescriptorError("Does not end with a zero-length blob.") if any([blob_info['length'] == 0 for blob_info in decoded['blobs'][:-1]]): From edb5083a5e015baa9681624fe1adf10b783de0d8 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 20:53:04 -0300 Subject: [PATCH 3/7] test that corrupted sd blobs gets delete if fails to decode --- tests/unit/stream/test_stream_descriptor.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/stream/test_stream_descriptor.py b/tests/unit/stream/test_stream_descriptor.py index 686eaadef..36353c772 100644 --- a/tests/unit/stream/test_stream_descriptor.py +++ b/tests/unit/stream/test_stream_descriptor.py @@ -109,14 +109,17 @@ class TestRecoverOldStreamDescriptors(AsyncioTestCase): self.assertEqual(sd_hash, descriptor.calculate_old_sort_sd_hash()) self.assertNotEqual(sd_hash, descriptor.calculate_sd_hash()) - async def test_decode_corrupt_blob_raises_proper_exception(self): + async def test_decode_corrupt_blob_raises_proper_exception_and_deletes_corrupt_file(self): loop = asyncio.get_event_loop() tmp_dir = tempfile.mkdtemp() self.addCleanup(lambda: shutil.rmtree(tmp_dir)) sd_hash = '9313d1807551186126acc3662e74d9de29cede78d4f133349ace846273ef116b9bb86be86c54509eb84840e4b032f6b2' with open(os.path.join(tmp_dir, sd_hash), 'wb') as handle: handle.write(b'doesnt work') + blob = BlobFile(loop, tmp_dir, sd_hash) + self.assertTrue(blob.file_exists) with self.assertRaises(InvalidStreamDescriptorError): await StreamDescriptor.from_stream_descriptor_blob( loop, tmp_dir, BlobFile(loop, tmp_dir, sd_hash) ) + self.assertFalse(blob.file_exists) From fcdd9295b4221839aaac0d17f3caf38d3fc3767b Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 20:53:21 -0300 Subject: [PATCH 4/7] delete upon failing to decode JSON from sd blob --- lbrynet/stream/descriptor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lbrynet/stream/descriptor.py b/lbrynet/stream/descriptor.py index 80d067ca4..d73369110 100644 --- a/lbrynet/stream/descriptor.py +++ b/lbrynet/stream/descriptor.py @@ -134,6 +134,7 @@ class StreamDescriptor: try: decoded = json.loads(json_bytes.decode()) except json.JSONDecodeError: + blob.delete() raise InvalidStreamDescriptorError("Does not decode as valid JSON") if decoded['blobs'][-1]['length'] != 0: raise InvalidStreamDescriptorError("Does not end with a zero-length blob.") From 61132312dd3acc9a54c4ed16b9b151c3024e036b Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 21:00:14 -0300 Subject: [PATCH 5/7] test nullify length on blob delete --- tests/unit/stream/test_stream_descriptor.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/stream/test_stream_descriptor.py b/tests/unit/stream/test_stream_descriptor.py index 36353c772..0f095e39d 100644 --- a/tests/unit/stream/test_stream_descriptor.py +++ b/tests/unit/stream/test_stream_descriptor.py @@ -118,8 +118,11 @@ class TestRecoverOldStreamDescriptors(AsyncioTestCase): handle.write(b'doesnt work') blob = BlobFile(loop, tmp_dir, sd_hash) self.assertTrue(blob.file_exists) + self.assertIsNotNone(blob.length) with self.assertRaises(InvalidStreamDescriptorError): await StreamDescriptor.from_stream_descriptor_blob( - loop, tmp_dir, BlobFile(loop, tmp_dir, sd_hash) + loop, tmp_dir, blob ) self.assertFalse(blob.file_exists) + # fixme: this is an emergency PR, plase move this to blob_file tests later + self.assertIsNone(blob.length) From b834f29634c2c2523a839017dc4dbb4590d8d52e Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 21:00:28 -0300 Subject: [PATCH 6/7] nullify length on blob delete --- lbrynet/blob/blob_file.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lbrynet/blob/blob_file.py b/lbrynet/blob/blob_file.py index 74ecc1e5a..2206b886e 100644 --- a/lbrynet/blob/blob_file.py +++ b/lbrynet/blob/blob_file.py @@ -151,6 +151,7 @@ class BlobFile: os.remove(self.file_path) self.verified.clear() self.finished_writing.clear() + self.length = None def decrypt(self, key: bytes, iv: bytes) -> bytes: """ From f49578cdbc74b1a4831e34d6ddd06a3fe53a4ca1 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 21 Feb 2019 21:14:51 -0300 Subject: [PATCH 7/7] fix integration test --- tests/integration/test_file_commands.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_file_commands.py b/tests/integration/test_file_commands.py index 8ac19e2ed..1d09a92af 100644 --- a/tests/integration/test_file_commands.py +++ b/tests/integration/test_file_commands.py @@ -133,6 +133,7 @@ class FileCommands(CommandTestCase): # restore blob os.rename(missing_blob.file_path + '__', missing_blob.file_path) self.server_blob_manager.blobs.clear() + missing_blob = self.server_blob_manager.get_blob(missing_blob_hash) await self.server_blob_manager.blob_completed(missing_blob) await asyncio.wait_for(self.wait_files_to_complete(), timeout=1)