test: cover ZIP fd lifecycle and add rollback

Add regression tests for two node:zlib ZipFile robustness issues:
- A read in flight when close() is called must complete on a live
descriptor; close() must not release the fd out from under it (which
surfaces as EBADF, or an OS-reused-fd cross-file read).
- If the central-directory rewrite fails after add() has written the
member bytes, both the in-memory state and the on-disk archive must be
rolled back, not left half-updated.
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
PR-URL: #65016
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Author: pipobscure

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});

test/parallel/test-zlib-zip-file-lifecycle.js

@@ -0,0 +1,92 @@
+'use strict';
+
+// Lifecycle-hardening regression tests for node:zlib ZipFile (the on-disk,
+// fd-backed reader/writer). Each asserts the secure behavior, so it fails on
+// the pre-fix code and passes once its fix lands.
+//
+// 1. A read in flight when close() is called must complete on a live
+// descriptor - close() must not pull the fd out from under it (which would
+// surface as EBADF, or worse read another file once the fd number is
+// reused).
+// 2. If the central-directory rewrite fails after an add() has written the
+// member bytes, the in-memory state and the on-disk archive must be rolled
+// back to exactly what they were before the call, not left half-updated.
+
+require('../common');
+constassert=require('assert');
+const{ test }=require('node:test');
+constzlib=require('zlib');
+constfs=require('fs');
+consttmpdir=require('../common/tmpdir');
+
+constSIG_CENTRAL=0x02014b50;
+constSIG_EOCD=0x06054b50;
+
+functionwriteArchive(file,entries){
+constchunks=[];
+for(constchunkofzlib.createZipArchiveSync(entries))chunks.push(chunk);
+fs.writeFileSync(file,Buffer.concat(chunks));
+}
+
+// 1. A read that is in flight when the ZipFile is closed still completes.
+test('close() waits for an in-flight read instead of closing under it',async()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('inflight.zip');
+constpayload=Buffer.alloc(8*1024*1024,0x5a);
+writeArchive(file,[zlib.ZipEntry.createSync('big',payload,{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file);
+constentry=zf.getSync('big');
+constreading=entry.content();// In flight; do not await yet
+awaitzf.close();// Must wait for the read, not close the fd under it
+constdata=awaitreading;// Must resolve with correct bytes, not reject EBADF
+assert.strictEqual(data.length,payload.length);
+assert.ok(data.equals(payload));
+});
+
+// 2. A failed central-directory rewrite during add() is rolled back.
+test('a failed directory rewrite during addEntrySync is rolled back',()=>{
+tmpdir.refresh();
+constfile=tmpdir.resolve('addfail.zip');
+writeArchive(file,[zlib.ZipEntry.createSync('first.txt',Buffer.from('original'),
+{method: 'store'})]);
+
+constzf=zlib.ZipFile.openSync(file,{writable: true});
+consttoAdd=zlib.ZipEntry.createSync('second.txt',Buffer.from('added'),{method: 'store'});
+
+// Fail only the first central-directory write (its buffer starts with the
+// central-header or EOCD signature); the member bytes start with the local
+// header signature and pass through, and the rollback rewrite that follows
+// succeeds so the on-disk archive is restored.
+constrealWriteSync=fs.writeSync;
+letfailNextDirectoryWrite=true;
+fs.writeSync=function(fd,buffer,offset,length,position){
+if(failNextDirectoryWrite&&Buffer.isBuffer(buffer)&&buffer.length-offset>=4){
+constsig=buffer.readUInt32LE(offset);
+if(sig===SIG_CENTRAL||sig===SIG_EOCD){
+failNextDirectoryWrite=false;
+consterr=newError('ENOSPC: simulated no space left on device');
+err.code='ENOSPC';
+throwerr;
+}
+}
+returnrealWriteSync.call(fs,fd,buffer,offset,length,position);
+};
+try{
+assert.throws(()=>zf.addEntrySync(toAdd),{code: 'ENOSPC'});
+}finally{
+fs.writeSync=realWriteSync;
+}
+
+// In-memory: the half-added entry is gone and the original is still readable.
+assert.ok(!zf.has('second.txt'));
+assert.strictEqual(zf.getSync('first.txt').contentSync().toString(),'original');
+zf.closeSync();
+
+// On disk: reopening shows the original, uncorrupted archive.
+constreopened=zlib.ZipFile.openSync(file);
+assert.ok(reopened.has('first.txt'));
+assert.ok(!reopened.has('second.txt'));
+assert.strictEqual(reopened.getSync('first.txt').contentSync().toString(),'original');
+reopened.closeSync();
+});