Skip to content

Commit 0729e5c

Browse files
committed
zlib: roll back a failed add() directory rewrite
add()/addEntrySync() advanced the central-directory offset and adopted the new entry into memory before the final directory rewrite. If that rewrite failed (ENOSPC/EIO after the member bytes were already written), the in-memory state and the on-disk archive were left diverged and half-updated, with no restore, corrupting the next add(). Wrap the rewrite: on failure, restore the previous offset and directory entry and rewrite the original directory back, leaving the archive and handle exactly as before the call, then rethrow. Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
1 parent b41df04 commit 0729e5c

1 file changed

Lines changed: 46 additions & 6 deletions

File tree

lib/internal/zip/file.js

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,11 @@ class ZipFile {
387387
return this.#enqueue(async () =>
388388
this.#doAdd(await ZipEntry.create(filename, data, options)));
389389
}
390-
// Append the entry's bytes where the central directory currently starts,
391-
// then rewrite the directory to include it. On write failure, restore the
392-
// original directory (the partial write may have clobbered it) and rethrow;
393-
// on success, promote a spent stream entry to its on-disk copy.
390+
// Append the entry's bytes where the central directory currently starts, then
391+
// rewrite the directory to include it. If the member write or the directory
392+
// rewrite fails, restore the original directory and drop the half-adopted
393+
// entry so the archive is left exactly as it was, then rethrow; on success,
394+
// promote a spent stream entry to its on-disk copy.
394395
async #doAdd(entry) {
395396
const localOffset = this.#centralDirectoryOffset;
396397
let written = 0;
@@ -413,9 +414,30 @@ class ZipFile {
413414
}
414415
throw err;
415416
}
417+
const previousEntry = MapPrototypeGet(this.#entries, entry.name);
416418
this.#centralDirectoryOffset = localOffset + written;
417419
MapPrototypeSet(this.#entries, entry.name, { central: null, entry, localOffset });
418-
await this.#rewriteCentralDirectory();
420+
try {
421+
await this.#rewriteCentralDirectory();
422+
} catch (err) {
423+
// The directory rewrite failed after the member bytes landed. Undo the
424+
// in-memory adoption and rewrite the original directory back at its old
425+
// offset (where the failed member bytes started), leaving the archive and
426+
// this handle exactly as before the call.
427+
this.#centralDirectoryOffset = localOffset;
428+
if (previousEntry === undefined) {
429+
MapPrototypeDelete(this.#entries, entry.name);
430+
} else {
431+
MapPrototypeSet(this.#entries, entry.name, previousEntry);
432+
}
433+
try {
434+
await this.#rewriteCentralDirectory();
435+
} catch {
436+
// Restoring failed too (the device is likely full or gone); the
437+
// original error is the actionable one.
438+
}
439+
throw err;
440+
}
419441
// The entry now has a stable home in this archive; if it was a spent
420442
// streaming entry, rebind it to that on-disk copy so it stays readable.
421443
entry[kPromote](this.#handle, localOffset);
@@ -453,9 +475,27 @@ class ZipFile {
453475
}
454476
throw err;
455477
}
478+
const previousEntry = MapPrototypeGet(this.#entries, entry.name);
456479
this.#centralDirectoryOffset = localOffset + written;
457480
MapPrototypeSet(this.#entries, entry.name, { central: null, entry, localOffset });
458-
this.#rewriteCentralDirectorySync();
481+
try {
482+
this.#rewriteCentralDirectorySync();
483+
} catch (err) {
484+
// See #doAdd(): undo the in-memory adoption and restore the original
485+
// directory so a failed rewrite leaves the archive exactly as it was.
486+
this.#centralDirectoryOffset = localOffset;
487+
if (previousEntry === undefined) {
488+
MapPrototypeDelete(this.#entries, entry.name);
489+
} else {
490+
MapPrototypeSet(this.#entries, entry.name, previousEntry);
491+
}
492+
try {
493+
this.#rewriteCentralDirectorySync();
494+
} catch {
495+
// Restoring failed too; the original error is the actionable one.
496+
}
497+
throw err;
498+
}
459499
entry[kPromote](this.#handle, localOffset);
460500
return entry;
461501
}

0 commit comments

Comments
 (0)