Skip to content

Commit 130e1dd

Browse files
committed
gh-156434: Restore entry offsets when repack() fails
_ZipRepacker.repack() updates ZipInfo.header_offset for each member before moving that member's bytes, so a failing _copy_bytes() left the in-memory offsets describing a layout that was never written. remove() has already set _didModify, so a later close() committed a central directory built from those offsets, and the archive became unreadable even though the caller had handled the error. ZipFile.repack() now snapshots the offsets of the surviving and the removed entries and restores them if the repack raises. The copy only ever moves data to lower offsets, so a partial move never overwrites its own source and the archive falls back to the state of a remove() with no repack().
1 parent fe3a26f commit 130e1dd

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

Lib/test/test_zipfile/test_core.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2662,6 +2662,49 @@ def test_overlap_with_archive_comment(self):
26622662
fz.seek(0)
26632663
self.assertEqual(fz.read(), expected)
26642664

2665+
def test_repack_failed_write_restores_header_offsets(self):
2666+
# A failed repack() must leave the in-memory offsets describing the
2667+
# file that is actually on disk, so that a later close() does not
2668+
# commit a central directory pointing at data that was never written.
2669+
class FlakyBytesIO(io.BytesIO):
2670+
countdown = None
2671+
2672+
def write(self, data):
2673+
if self.countdown is not None:
2674+
self.countdown -= 1
2675+
if self.countdown < 0:
2676+
raise OSError(errno.ENOSPC, 'No space left on device')
2677+
return super().write(data)
2678+
2679+
names = ['a.txt', 'b.txt', 'c.txt', 'd.txt']
2680+
buf = io.BytesIO()
2681+
with zipfile.ZipFile(buf, 'w') as zh:
2682+
for name in names:
2683+
zh.writestr(name, name[0].upper().encode() * 5000)
2684+
2685+
fz = FlakyBytesIO(buf.getvalue())
2686+
with zipfile.ZipFile(fz, 'a') as zh:
2687+
zi = zh.remove('b.txt')
2688+
expected = [z.header_offset for z in zh.infolist()]
2689+
expected_removed = zi.header_offset
2690+
2691+
fz.countdown = 1
2692+
with self.assertRaises(OSError):
2693+
zh.repack([zi], chunk_size=4096)
2694+
2695+
self.assertEqual([z.header_offset for z in zh.infolist()], expected)
2696+
self.assertEqual(zi.header_offset, expected_removed)
2697+
2698+
# The archive is still usable once the write error clears.
2699+
fz.countdown = None
2700+
2701+
fz.seek(0)
2702+
with zipfile.ZipFile(fz) as zh:
2703+
self.assertIsNone(zh.testzip())
2704+
self.assertEqual(zh.namelist(), ['a.txt', 'c.txt', 'd.txt'])
2705+
for name in zh.namelist():
2706+
self.assertEqual(zh.read(name), name[0].upper().encode() * 5000)
2707+
26652708
class ZipRepackerTests(unittest.TestCase):
26662709
def _generate_local_file_entry(self, arcname, raw_bytes,
26672710
compression=zipfile.ZIP_STORED,

Lib/zipfile/__init__.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,12 +2434,18 @@ def repack(self, removed=None, *, strict_descriptor=True,
24342434
)
24352435

24362436
self._writing = True
2437+
header_offsets = [(zinfo, zinfo.header_offset)
2438+
for zinfo in (*self.filelist, *(removed or ()))]
24372439
try:
24382440
repacker = _ZipRepacker(
24392441
strict_descriptor=strict_descriptor,
24402442
chunk_size=chunk_size,
24412443
)
24422444
repacker.repack(self, removed)
2445+
except BaseException:
2446+
for zinfo, header_offset in header_offsets:
2447+
zinfo.header_offset = header_offset
2448+
raise
24432449
finally:
24442450
self._writing = False
24452451

0 commit comments

Comments
 (0)