Commit 3d85c94cfad for nodejs
commit 3d85c94cfaddf661b5fe7a859d4ee08a08797924
Author: Philipp Dunkel <pipobscure@users.noreply.github.com>
Date: Wed Sep 23 03:42:24 2026 +0200
vfs: support recursive readdir in ZipProvider
ZipProvider threw ERR_METHOD_NOT_IMPLEMENTED for readdir() with
`{ recursive: true }`, although an archive already stores every member
under its full path, so a recursive listing is the same scan of the
entry names as a flat one, keeping the whole path below the directory
instead of only its first segment.
List every member below the directory, and every directory their paths
pass through, whether the archive holds an entry for it or only implies
it. Names are relative to the listed directory and joined with `/`, as
MemoryProvider returns them, so VirtualFileSystem reports the right
parentPath for each Dirent. A name that is both a member and a
directory is listed once, as a directory, as a flat listing already
did.
The entries are now collected in a map rather than looked up in an
array, so listing a directory no longer takes time quadratic in the
number of its children.
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
PR-URL: https://github.com/nodejs/node/pull/66127
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git a/doc/api/vfs.md b/doc/api/vfs.md
index 68dba4b8e66..6f6fc53a49e 100644
--- a/doc/api/vfs.md
+++ b/doc/api/vfs.md
@@ -645,11 +645,11 @@ the VFS API. `provider.readonly` reflects the archive's own
`ZipFile` is writable only when opened with `{ writable: true }`.
Directories are recognized both explicitly (an entry whose name ends in `/`)
-and implicitly (any entry name starting with `"<dir>/"`). `readdir()` does
-not support `{ recursive: true }`. Because a ZIP member cannot be edited or
-read in place - only fully written or fully decompressed - a file opened for
-writing only commits its content (as a new archive entry) when the handle is
-closed.
+and implicitly (any entry name starting with `"<dir>/"`), and are listed by
+`readdir()`, including with `{ recursive: true }`, either way. Because a ZIP
+member cannot be edited or read in place - only fully written or fully
+decompressed - a file opened for writing only commits its content (as a new
+archive entry) when the handle is closed.
Every method has a synchronous counterpart (`openSync()`, `statSync()`,
`readdirSync()`, and so on), backed by the equally complete synchronous
diff --git a/lib/internal/vfs/providers/ziparchive.js b/lib/internal/vfs/providers/ziparchive.js
index 23ac09cad51..297d960a1f2 100644
--- a/lib/internal/vfs/providers/ziparchive.js
+++ b/lib/internal/vfs/providers/ziparchive.js
@@ -1,11 +1,11 @@
'use strict';
const {
- ArrayPrototypeIndexOf,
ArrayPrototypePush,
MathMax,
MathMin,
Number,
+ SafeMap,
StringPrototypeIndexOf,
StringPrototypeSlice,
StringPrototypeStartsWith,
@@ -17,7 +17,6 @@ const { Buffer } = require('buffer');
const {
codes: {
ERR_INVALID_ARG_TYPE,
- ERR_METHOD_NOT_IMPLEMENTED,
},
} = require('internal/errors');
const { VirtualProvider } = require('internal/vfs/provider');
@@ -457,44 +456,42 @@ class ZipProvider extends VirtualProvider {
if (!stats.isDirectory()) throw createENOTDIR('scandir', path);
const prefix = name === '' ? '' : `${name}/`;
const withFileTypes = options?.withFileTypes === true;
- const names = [];
- const isDir = [];
+ const recursive = options?.recursive === true;
+ // Each listed path, relative to the directory, mapped to whether it is a
+ // directory.
+ const children = new SafeMap();
for (const key of this.#source.keys()) {
if (!StringPrototypeStartsWith(key, prefix)) continue;
const rest = StringPrototypeSlice(key, prefix.length);
- if (rest === '') continue; // The directory's own explicit entry
- const slash = StringPrototypeIndexOf(rest, '/');
- const childName = slash === -1 ? rest : StringPrototypeSlice(rest, 0, slash);
- const childIsDir = slash !== -1;
- const existingIndex = ArrayPrototypeIndexOf(names, childName);
- if (existingIndex !== -1) {
- if (childIsDir) isDir[existingIndex] = true;
- continue;
+ // An archive need not hold entries for the directories above a member,
+ // so every directory the member's path passes through is listed too.
+ let start = 0;
+ let slash = StringPrototypeIndexOf(rest, '/');
+ while (slash !== -1) {
+ children.set(StringPrototypeSlice(rest, 0, slash), true);
+ if (!recursive) break;
+ start = slash + 1;
+ slash = StringPrototypeIndexOf(rest, '/', start);
+ }
+ if (slash === -1 && start < rest.length && !children.has(rest)) {
+ children.set(rest, false);
}
- ArrayPrototypePush(names, childName);
- ArrayPrototypePush(isDir, childIsDir);
}
const result = [];
- for (let i = 0; i < names.length; i++) {
+ for (const { 0: childName, 1: isDir } of children) {
if (withFileTypes) {
- ArrayPrototypePush(result, new Dirent(names[i], isDir[i] ? UV_DIRENT_DIR : UV_DIRENT_FILE, name));
+ ArrayPrototypePush(result, new Dirent(childName, isDir ? UV_DIRENT_DIR : UV_DIRENT_FILE, name));
} else {
- ArrayPrototypePush(result, names[i]);
+ ArrayPrototypePush(result, childName);
}
}
return result;
}
async readdir(path, options) {
- if (options?.recursive) {
- throw new ERR_METHOD_NOT_IMPLEMENTED('readdir with { recursive: true } on a ZipProvider');
- }
const name = normalize(path);
return this.#readdirEntries(path, name, options, await this.stat(path));
}
readdirSync(path, options) {
- if (options?.recursive) {
- throw new ERR_METHOD_NOT_IMPLEMENTED('readdirSync with { recursive: true } on a ZipProvider');
- }
const name = normalize(path);
return this.#readdirEntries(path, name, options, this.statSync(path));
}
diff --git a/test/parallel/test-vfs-zip-provider-readdir-recursive.js b/test/parallel/test-vfs-zip-provider-readdir-recursive.js
new file mode 100644
index 00000000000..553ae98cfcd
--- /dev/null
+++ b/test/parallel/test-vfs-zip-provider-readdir-recursive.js
@@ -0,0 +1,83 @@
+// Flags: --experimental-vfs
+'use strict';
+
+// A recursive readdir() of a ZipProvider lists every member below the
+// directory, together with every directory their paths pass through, whether
+// or not the archive holds an entry for it.
+
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const zlib = require('zlib');
+const vfs = require('node:vfs');
+
+async function buildArchive(entries) {
+ const chunks = [];
+ for await (const chunk of zlib.createZipArchive(entries)) chunks.push(chunk);
+ return Buffer.concat(chunks);
+}
+
+(async () => {
+ const archive = await buildArchive([
+ await zlib.ZipEntry.create('top.txt', Buffer.from('top')),
+ // Directories only implied by a member's path, several levels deep.
+ await zlib.ZipEntry.create('a/b/c/deep.txt', Buffer.from('deep')),
+ // An explicit directory entry, before and after members inside it.
+ await zlib.ZipEntry.create('a/b/', Buffer.alloc(0)),
+ await zlib.ZipEntry.create('a/b/sibling.txt', Buffer.from('sibling')),
+ await zlib.ZipEntry.create('empty/', Buffer.alloc(0)),
+ ]);
+ const provider = new vfs.ZipProvider(new zlib.ZipBuffer(archive));
+
+ const all = [
+ 'a', 'a/b', 'a/b/c', 'a/b/c/deep.txt', 'a/b/sibling.txt', 'empty', 'top.txt',
+ ];
+ const dirs = new Set(['a', 'a/b', 'a/b/c', 'empty']);
+
+ // Each directory is listed once, whether implied, explicit, or both.
+ assert.deepStrictEqual(provider.readdirSync('/', { recursive: true }).sort(), all);
+ assert.deepStrictEqual((await provider.readdir('/', { recursive: true })).sort(), all);
+
+ const dirents = provider.readdirSync('/', { recursive: true, withFileTypes: true });
+ assert.deepStrictEqual(dirents.map((d) => d.name).sort(), all);
+ for (const dirent of dirents) {
+ assert.strictEqual(dirent.isDirectory(), dirs.has(dirent.name), dirent.name);
+ assert.strictEqual(dirent.isFile(), !dirs.has(dirent.name), dirent.name);
+ }
+
+ // Listing a subdirectory yields paths relative to it.
+ assert.deepStrictEqual(provider.readdirSync('/a/b', { recursive: true }).sort(),
+ ['c', 'c/deep.txt', 'sibling.txt']);
+ assert.deepStrictEqual(provider.readdirSync('/empty', { recursive: true }), []);
+ assert.throws(() => provider.readdirSync('/top.txt', { recursive: true }),
+ { code: 'ENOTDIR' });
+ assert.throws(() => provider.readdirSync('/missing', { recursive: true }),
+ { code: 'ENOENT' });
+
+ // A non-recursive listing is unchanged.
+ assert.deepStrictEqual(provider.readdirSync('/').sort(), ['a', 'empty', 'top.txt']);
+ assert.deepStrictEqual(provider.readdirSync('/a/b').sort(), ['c', 'sibling.txt']);
+
+ // A name that is both a member and a directory is listed as a directory.
+ {
+ const clash = new vfs.ZipProvider(new zlib.ZipBuffer(await buildArchive([
+ await zlib.ZipEntry.create('x', Buffer.from('file')),
+ await zlib.ZipEntry.create('x/y.txt', Buffer.from('nested')),
+ ])));
+ const entries = clash.readdirSync('/', { recursive: true, withFileTypes: true });
+ assert.deepStrictEqual(entries.map((d) => [d.name, d.isDirectory()]).sort(),
+ [['x', true], ['x/y.txt', false]]);
+ }
+
+ // Through node:fs, each Dirent reports its own parent directory.
+ {
+ const archiveVfs = vfs.create(provider);
+ const mountPoint = archiveVfs.mount();
+ const listed = fs.readdirSync(mountPoint, { recursive: true, withFileTypes: true })
+ .map((d) => path.join(d.parentPath, d.name)).sort();
+ assert.deepStrictEqual(listed, all.map((p) => path.join(mountPoint, p)).sort());
+ assert.deepStrictEqual(fs.readdirSync(mountPoint, { recursive: true }).sort(), all);
+ archiveVfs.unmount();
+ }
+})().then(common.mustCall());
diff --git a/test/parallel/test-vfs-zip-provider.js b/test/parallel/test-vfs-zip-provider.js
index ef86e6cb6a4..c93c8721453 100644
--- a/test/parallel/test-vfs-zip-provider.js
+++ b/test/parallel/test-vfs-zip-provider.js
@@ -72,10 +72,9 @@ async function buildArchive(entries, comment) {
assert.strictEqual(byName.get('dir').isDirectory(), true);
await assert.rejects(archiveVfs.promises.readdir('/a.txt'), { code: 'ENOTDIR' });
- await assert.rejects(
- archiveVfs.promises.readdir('/', { recursive: true }),
- { code: 'ERR_METHOD_NOT_IMPLEMENTED' },
- );
+ const recursiveEntries = await archiveVfs.promises.readdir('/', { recursive: true });
+ assert.deepStrictEqual(recursiveEntries.sort(),
+ ['a.txt', 'dir', 'dir/b.txt', 'empty-dir']);
// readFile / writeFile round trip (new file).
assert.strictEqual(await archiveVfs.promises.readFile('/a.txt', 'utf8'), 'hello');
@@ -232,10 +231,8 @@ async function buildArchive(entries, comment) {
assert.throws(() => archiveVfs.statSync('/missing.txt'), { code: 'ENOENT' });
assert.deepStrictEqual(archiveVfs.readdirSync('/').sort(), ['a.txt', 'dir']);
assert.throws(() => archiveVfs.readdirSync('/a.txt'), { code: 'ENOTDIR' });
- assert.throws(
- () => archiveVfs.readdirSync('/', { recursive: true }),
- { code: 'ERR_METHOD_NOT_IMPLEMENTED' },
- );
+ assert.deepStrictEqual(archiveVfs.readdirSync('/', { recursive: true }).sort(),
+ ['a.txt', 'dir', 'dir/b.txt']);
// readFile/writeFile/appendFile round trip.
assert.strictEqual(archiveVfs.readFileSync('/a.txt', 'utf8'), 'hello');