pack: Fix a buffering issue with PackStreamReader; add tests.

Change-Id: I2695ce332114e213239efaa00bd46b423a670d5c

NEWS

@@ -22,6 +22,9 @@
 
   * Correctly return a tuple from MemoryObjectStore.get_raw. (Dave Borowitz)
 
+  * Fix a bug in reading the pack checksum when there are fewer than 20 bytes
+    left in the buffer. (Dave Borowitz)
+
  API CHANGES
 
   * write_pack no longer takes the num_objects argument and requires an object

dulwich/pack.py

@@ -747,6 +747,13 @@ class PackStreamReader(object):
             buf.seek(0)
             self._rbuf = buf
 
+        if self._buf_len() < 20:
+            # If the read buffer is full, then the last read() got the whole
+            # trailer off the wire. If not, it means there is still some of the
+            # trailer to read. We need to read() all 20 bytes; N come from the
+            # read buffer and (20 - N) come from the wire.
+            self.read(20)
+
         pack_sha = ''.join(self._trailer)
         if pack_sha != self.sha.digest():
             raise ChecksumMismatch(sha_to_hex(pack_sha), self.sha.hexdigest())

dulwich/tests/test_pack.py

@@ -49,6 +49,7 @@ from dulwich.objects import (
 from dulwich.pack import (
     OFS_DELTA,
     REF_DELTA,
+    DELTA_TYPES,
     MemoryPackIndex,
     Pack,
     PackData,
@@ -66,6 +67,7 @@ from dulwich.pack import (
     write_pack,
     unpack_object,
     compute_file_sha,
+    PackStreamReader,
     DeltaChainIterator,
     )
 from dulwich.tests import (
@@ -655,6 +657,52 @@ class DeltifyTests(TestCase):
             list(deltify_pack_objects([(b1, ""), (b2, "")])))
 
 
+class TestPackStreamReader(TestCase):
+
+    def test_read_objects_emtpy(self):
+        f = StringIO()
+        build_pack(f, [])
+        reader = PackStreamReader(f.read)
+        self.assertEqual(0, len(list(reader.read_objects())))
+
+    def test_read_objects(self):
+        f = StringIO()
+        entries = build_pack(f, [
+          (Blob.type_num, 'blob'),
+          (OFS_DELTA, (0, 'blob1')),
+          ])
+        reader = PackStreamReader(f.read)
+        objects = list(reader.read_objects(compute_crc32=True))
+        self.assertEqual(2, len(objects))
+
+        blob, delta = objects
+        bofs, btype, buncomp, blen, bcrc = blob
+        dofs, dtype, duncomp, dlen, dcrc = delta
+
+        self.assertEqual(entries[0][0], bofs)
+        self.assertEqual(Blob.type_num, btype)
+        self.assertEqual('blob', ''.join(buncomp))
+        self.assertEqual(dofs - bofs, blen)
+        self.assertEqual(entries[0][4], bcrc)
+
+        self.assertEqual(entries[1][0], dofs)
+        self.assertEqual(OFS_DELTA, dtype)
+        delta_ofs, delta_chunks = duncomp
+        self.assertEqual(dofs - bofs, delta_ofs)
+        self.assertEqual(create_delta('blob', 'blob1'), ''.join(delta_chunks))
+        self.assertEqual(len(f.getvalue()) - 20 - dofs, dlen)
+        self.assertEqual(entries[1][4], dcrc)
+
+    def test_read_objects_buffered(self):
+        f = StringIO()
+        build_pack(f, [
+          (Blob.type_num, 'blob'),
+          (OFS_DELTA, (0, 'blob1')),
+          ])
+        reader = PackStreamReader(f.read, zlib_bufsize=4)
+        self.assertEqual(2, len(list(reader.read_objects())))
+
+
 class TestPackIterator(DeltaChainIterator):
 
     _compute_crc32 = True