vfs: support recursive readdir in ZipProvider

doc/api/vfs.md

@@ -640,11 +640,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

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');
@@ -437,44 +436,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));
   }

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());

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');
@@ -214,10 +213,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');