From ee1b0b9283b4f0efb11805d480388f93e56e6013 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <perrinjerome@gmail.com>
Date: Wed, 14 Feb 2024 14:41:42 +0000
Subject: [PATCH] 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] https://github.com/zopefoundation/ZODB/pull/395#discussion_r1474127300
---
 CHANGES.rst                         |  3 +++
 src/ZODB/FileStorage/FileStorage.py |  2 +-
 src/ZODB/tests/testFileStorage.py   | 24 ++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 3c8689abe..4bf1f939e 100644
--- a/CHANGES.rst
+++ b/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)
 ==================
diff --git a/src/ZODB/FileStorage/FileStorage.py b/src/ZODB/FileStorage/FileStorage.py
index 80842c9bd..5c76000a7 100644
--- a/src/ZODB/FileStorage/FileStorage.py
+++ b/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)
diff --git a/src/ZODB/tests/testFileStorage.py b/src/ZODB/tests/testFileStorage.py
index bdb5cfdd5..5f545e529 100644
--- a/src/ZODB/tests/testFileStorage.py
+++ b/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