Fix empty file hash (#1)

* Populate versions skips empty files

* Strip file path to prevent spaces at the end of URLs

* Fix aiohttp requirements to make sure tests pass

* Merge two try block into one

openwebvulndb/common/hash.py

@@ -110,14 +110,16 @@ def collect(self):
 
                     target_path = join(self.prefix, relative)
 
+                    target_path = target_path.strip()
+
                     self.version_checker.reset()
 
                     sig = Signature(path=target_path, algo=self.hasher.algo)
 
                     sig.hash = self.hasher.hash(full_path, chunk_cb=self.version_checker)
                     sig.contains_version = self.version_checker.contains_version
                     yield sig
-                except OSError as e:
+                except (OSError, ValueError) as e:
                     logger.warn("Error while hashing %s: %s, skipping", target_path, e)
 
 
@@ -128,10 +130,16 @@ def __init__(self, algo):
     def hash(self, file_path, chunk_cb=lambda c: None):
         hash = hashlib.new(self.algo)
         with open(file_path, "rb") as fp:
+            empty = True
             for chunk in iter(lambda: fp.read(4096), b""):
+                if empty and len(chunk) > 0:
+                    empty = False
                 hash.update(chunk)
                 chunk_cb(chunk)
 
+            if empty:
+                raise ValueError("File is empty")
+
             return hash.hexdigest()
 
 

requirements.txt

@@ -1,4 +1,4 @@
-aiohttp>3.1,<3.5
+aiohttp>3.1,<3.3
 easyinject==0.3
 marshmallow==2.15.6
 packaging==16.7

tests/common_test/hash_collector_test.py

@@ -166,6 +166,47 @@ def hash(hasher, file_path, chunk_cb):
             self.assertIn(Signature(path="wp-content/plugins/my-plugin/license.txt", hash="12345", algo="CUST"),
                           signatures)
 
+    def test_exclude_empty_files(self):
+        class FakeHasher:
+
+            algo = "sha256"
+
+            def hash(hasher, file_path, chunk_cb):
+                if file_path == "/path/empty":
+                    raise ValueError("File is empty")
+                return "12345"
+
+        with patch('openwebvulndb.common.hash.walk') as walk:
+            walk.return_value = [
+                ("/path", [], ["readme.txt", "empty", "license.txt"]),
+            ]
+            collector = HashCollector(path="/path", hasher=FakeHasher(),
+                                      prefix="wp-content/plugins/my-plugin", lookup_version="1.2.3")
+
+            signatures = list(collector.collect())
+
+            self.assertIn(Signature(path="wp-content/plugins/my-plugin/readme.txt", hash="12345", algo="sha256"),
+                          signatures)
+            self.assertIn(Signature(path="wp-content/plugins/my-plugin/license.txt", hash="12345", algo="sha256"),
+                          signatures)
+
+    def test_strip_filenames(self):
+        with patch('openwebvulndb.common.hash.walk') as walk:
+            walk.return_value = [("/some/path/random1234", [], ["readme.txt ", "license.txt "])]
+            collector = HashCollector(path="/some/path/random1234", hasher=MagicMock(),
+                                      prefix="wp-content/plugins/my-plugin")
+            collector.hasher.algo = "sha256"
+            collector.hasher.hash.return_value = "12345"
+
+            signatures = list(collector.collect())
+
+            walk.assert_called_with("/some/path/random1234")
+
+            self.assertIn(Signature(path="wp-content/plugins/my-plugin/readme.txt", hash="12345", algo="sha256"),
+                          signatures)
+            self.assertIn(Signature(path="wp-content/plugins/my-plugin/license.txt", hash="12345", algo="sha256"),
+                          signatures)
+
 
 class HasherTest(TestCase):
 
@@ -195,6 +236,14 @@ def test_callback_applied_to_chunks(self):
 
             check_chunk.assert_called_with(b"hello world")
 
+    def test_raise_value_error_if_file_is_empty(self):
+        m = mock_open(read_data=b"")
+
+        with patch('openwebvulndb.common.hash.open', m, create=True):
+            hasher = Hasher('SHA256')
+            with self.assertRaises(ValueError):
+                hasher.hash("/some/file.txt")
+
 
 class VersionCheckerTest(TestCase):