FileStorage: fix a potential error when restore is called with wrong …

…txn_prev

if restore is called with an existing txn_prev that does not contain
the oid, the "8-byte redundant transaction length -8" and the beginning
of the next transaction was read as a data record.

The condition to stop was incorrect as discussed in [1], in practice
this error would only happens when restore is passed a bad txn_prev so
this should not be a problem in normal conditions because we are
supposed to find the data record for that oid.

[1] zopefoundation#395 (comment)

CHANGES.rst

@@ -11,6 +11,9 @@
 
 - Fix sorting issue in ``scripts/space.py``.
 
+- Fix behavior of ``FileStorage.restore`` when provided wrong ``prev_txn``.
+  For details see `#397 <https://github.com/zopefoundation/ZODB/pull/397>`_.
+
 
 5.8.1 (2023-07-18)
 ==================

src/ZODB/FileStorage/FileStorage.py

@@ -672,7 +672,7 @@ def _data_find(self, tpos, oid, data):
         tid, tl, status, ul, dl, el = unpack(TRANS_HDR, h)
         status = as_text(status)
         self._file.read(ul + dl + el)
-        tend = tpos + tl + 8
+        tend = tpos + tl
         pos = self._file.tell()
         while pos < tend:
             h = self._read_data_header(pos)

src/ZODB/tests/testFileStorage.py

@@ -259,6 +259,30 @@ def testRestorePrevTxnNotExists(self):
         self._storage.tpc_finish(t)
         self.assertEqual(self._storage.load(oid)[0], b'data1b')
 
+    def testRestorePrevTxnWithoutOid(self):
+        t = TransactionMetaData()
+        self._storage.tpc_begin(t)
+        self._storage.store(b'\1' * 8, b'\0' * 8, b'data1', b'', t)
+        self._storage.tpc_vote(t)
+        tid1 = self._storage.tpc_finish(t)
+
+        # this transaction is also here to detect problems if restore reads
+        # below the end of tid1
+        t = TransactionMetaData()
+        self._storage.tpc_begin(t)
+        self._storage.store(b'\2' * 8, b'\0' * 8, b'data2a', b'', t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+
+        # restore with a transaction not containing the oid
+        t = TransactionMetaData()
+        self._storage.tpc_begin(t)
+        self._storage.restore(b'\2' * 8, b'\0' * 8, b'data2b', b'', tid1, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+        self.assertEqual(self._storage.load(b'\1' * 8)[0], b'data1')
+        self.assertEqual(self._storage.load(b'\2' * 8)[0], b'data2b')
+
     def testCorruptionInPack(self):
         # This sets up a corrupt .fs file, with a redundant transaction
         # length mismatch.  The implementation of pack in many releases of