From 7bc4647952b98c70b6b66cdf1596e3ba1f6a168e Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 20 Aug 2026 22:11:57 +0200 Subject: [PATCH] chunkers: do not re-read a file map that does not reach EOF, see #4363 FileReader used "blockify_gen is None" for both "not started yet" and "exhausted": _fill_buffer() cleared it on StopIteration, and read() / readinto() take a cleared generator as their cue to build a fresh FileFMAPReader.blockify(), which replays the fmap from its start. dread() reads sequentially and ignores its offset argument, so such a replay continued at the current file position and appended duplicate data. An fmap that does not start at 0 seeks back to its start on every replay and never terminates at all. This stayed invisible as long as every fmap reached EOF, because the replay then hits a 0-byte read immediately: that is true for sparsemap() and for "create --map", which always maps the whole file. "create --map --reuse-from" is the first caller that reads only parts of a file. Track exhaustion in its own flag and never restart the generator. blockify() also has to stop treating a short read as EOF, or dropping the restart truncates the input instead: a file object may return less than requested without being at EOF, see buzhash_self_test test_small_reads, which passed only because of the restart. --- src/borg/chunkers/reader.pyx | 17 ++++--- src/borg/testsuite/chunkers/reader_test.py | 55 ++++++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/borg/chunkers/reader.pyx b/src/borg/chunkers/reader.pyx index dccfbc9129..18bcc9539e 100644 --- a/src/borg/chunkers/reader.pyx +++ b/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 diff --git a/src/borg/testsuite/chunkers/reader_test.py b/src/borg/testsuite/chunkers/reader_test.py index b4bf3a9981..8956680de8 100644 --- a/src/borg/testsuite/chunkers/reader_test.py +++ b/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