diff --git a/doc/api/vfs.md b/doc/api/vfs.md index bc4a94f80e8b..15da3f680bb0 100644 --- a/doc/api/vfs.md +++ b/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 `"/"`). `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 `"/"`), 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 9ed3a64a4e0e..90ae1db1ffe6 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'); @@ -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)); } 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 000000000000..553ae98cfcd7 --- /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 843a606a5deb..5100e41c2894 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'); @@ -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');