Merge pull request #10166 from ThomasWaldmann/reader-fmap-partial

chunkers: do not re-read a file map that does not reach EOF

src/borg/chunkers/reader.pyx

@@ -213,8 +213,10 @@ class FileFMAPReader:
                     offset += got
                     range_size -= got
                     yield Chunk(data, size=got, allocation=allocation)
-                if got < wanted:
-                    # We did not get enough data; looks like EOF.
+                if got == 0:
+                    # Nothing read at all - this is EOF. A short read (0 < got < wanted) is
+                    # not EOF: file objects may return less than requested, so just continue
+                    # with the rest of the range.
                     return
 
 
@@ -235,6 +237,7 @@ class FileReader:
         self.offset = 0  # offset into the first buffer object's data
         self.remaining_bytes = 0  # total bytes available in buffer
         self.blockify_gen = None  # generator from FileFMAPReader.blockify
+        self.blockify_done = False  # the blockify generator was exhausted (do not restart it)
         self.fd = fd
         self.fh = fh
         self.fmap = fmap
@@ -265,7 +268,7 @@ class FileReader:
         Fill the buffer with more data from the blockify generator.
         Returns True if more data was added, False if EOF.
         """
-        if self.blockify_gen is None:
+        if self.blockify_gen is None or self.blockify_done:
             return False
 
         try:
@@ -275,7 +278,9 @@ class FileReader:
             self.remaining_bytes += chunk.meta["size"]
             return True
         except StopIteration:
-            self.blockify_gen = None
+            # do NOT clear blockify_gen here: an fmap that does not extend to EOF would
+            # otherwise be replayed from its start by the next read()/readinto() call.
+            self.blockify_done = True
             return False
 
     def read(self, size):
@@ -297,7 +302,7 @@ class FileReader:
                  than requested.
         """
         # Initialize if not already done
-        if self.blockify_gen is None:
+        if self.blockify_gen is None and not self.blockify_done:
             self.buffer = []
             self.offset = 0
             self.remaining_bytes = 0
@@ -437,7 +442,7 @@ class FileReader:
                 return self._readinto_direct(tv, size)
 
         # Initialize if not already done
-        if self.blockify_gen is None:
+        if self.blockify_gen is None and not self.blockify_done:
             self.buffer = []
             self.offset = 0
             self.remaining_bytes = 0

src/borg/testsuite/chunkers/reader_test.py

@@ -410,3 +410,58 @@ def test_filereader_no_direct_for_fifo(tmpdir):
     st = os.stat(fifo_fn)
     reader = FileReader(fd=BytesIO(b""), fh=-1, read_size=1024, sparse=False, fmap=None, st=st)
     assert not reader.direct
+
+
+def test_filereader_fmap_not_covering_eof(tmpdir):
+    """An fmap that ends before EOF must not be replayed once it is exhausted, see #4363.
+
+    --map with --reuse-from reads only the parts of a file that are not reused, so the
+    fmap it gives ends before EOF. FileReader used to restart the blockify generator
+    then, which re-read the file from the current position (appending duplicate data)
+    or looped forever for an fmap that does not start at 0.
+    """
+    fn = str(tmpdir / "file")
+    with open(fn, "wb") as f:
+        f.write(b"0123456789" * 10)  # 100 bytes
+    fd = os.open(fn, os.O_RDONLY)
+    try:
+        for fmap, expected in [
+            ([(0, 30, True)], b"0123456789" * 3),  # prefix only
+            ([(20, 20, True)], b"0123456789" * 2),  # neither at offset 0 nor at EOF
+            ([(0, 10, True), (10, 20, True)], b"0123456789" * 3),  # multiple ranges, still not to EOF
+        ]:
+            os.lseek(fd, 0, os.SEEK_SET)
+            reader = FileReader(fd=None, fh=fd, read_size=1024, sparse=False, fmap=list(fmap))
+            got = b""
+            while True:
+                chunk = reader.read(1024)
+                if not chunk.meta["size"]:
+                    break
+                got += chunk.data
+                assert len(got) <= len(expected)  # do not loop forever if this regresses
+            assert got == expected
+    finally:
+        os.close(fd)
+
+
+def test_filereader_short_reads_are_not_eof():
+    """A file object returning less than requested is not at EOF - keep reading, see #4363."""
+
+    class SmallReadFile:
+        def __init__(self, data):
+            self.data = data
+
+        def read(self, nbytes):
+            chunk, self.data = self.data[:1], self.data[1:]  # always at most 1 byte
+            return chunk
+
+    content = b"a" * 20
+    reader = FileReader(fd=SmallReadFile(content), fh=-1, read_size=1024, sparse=False, fmap=[(0, 20, True)])
+    got = b""
+    while True:
+        chunk = reader.read(1024)
+        if not chunk.meta["size"]:
+            break
+        got += chunk.data
+        assert len(got) <= len(content)
+    assert got == content