From 819ceed6a56ec1f52224cea4c37a655ba1b1ef80 Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Thu, 6 Aug 2026 08:44:37 +0900 Subject: [PATCH] vfs: fix path returned by recursive mkdir `mkdir()` with `recursive: true` returns the first directory that was created. VFS passed the provider's return value straight through, so the path came back in the wrong namespace: `RealFSProvider` returned an absolute path inside its root, leaking that root to the caller, and a mounted VFS returned a VFS-internal path without the mount point. Map the created path back through the provider and mount translations so the returned value is a path the caller can hand to `fs` again. This covers the sync, callback, and promises forms. --- lib/internal/vfs/file_system.js | 9 ++++-- lib/internal/vfs/providers/real.js | 11 +++++-- test/parallel/test-vfs-fs-mkdir-callback.js | 14 +++++++++ test/parallel/test-vfs-fs-mkdirSync.js | 31 +++++++++++++++++-- test/parallel/test-vfs-fs-promises.js | 5 ++- .../test-vfs-real-provider-promises.js | 6 +++- test/parallel/test-vfs-real-provider.js | 16 ++++++++-- 7 files changed, 82 insertions(+), 10 deletions(-) diff --git a/lib/internal/vfs/file_system.js b/lib/internal/vfs/file_system.js index e47ed4c61080..e7441810df75 100644 --- a/lib/internal/vfs/file_system.js +++ b/lib/internal/vfs/file_system.js @@ -327,7 +327,11 @@ class VirtualFileSystem { */ mkdirSync(dirPath, options) { const providerPath = this.#toProviderPath(dirPath); - return this[kProvider].mkdirSync(providerPath, options); + const created = this[kProvider].mkdirSync(providerPath, options); + // With `recursive: true` the provider returns the first directory it + // created as a provider-relative path, which has to be mapped back to a + // path the caller can use. Anything else resolves to `undefined`. + return created === undefined ? undefined : this.#toMountedPath(created); } /** @@ -1122,7 +1126,8 @@ class VirtualFileSystem { async mkdir(dirPath, options) { const providerPath = toProviderPath(dirPath); - return provider.mkdir(providerPath, options); + const created = await provider.mkdir(providerPath, options); + return created === undefined ? undefined : toMountedPath(created); }, async rmdir(dirPath) { diff --git a/lib/internal/vfs/providers/real.js b/lib/internal/vfs/providers/real.js index df9bd00ac1ad..cd3313585538 100644 --- a/lib/internal/vfs/providers/real.js +++ b/lib/internal/vfs/providers/real.js @@ -378,14 +378,21 @@ class RealFSProvider extends VirtualProvider { return fs.promises.readdir(realPath, options); } + // With `recursive: true` the underlying fs returns the first directory that + // was created as a real filesystem path. It has to be mapped back into the + // provider's namespace so the root path is never leaked to the caller. mkdirSync(vfsPath, options) { const realPath = this.#resolvePath(vfsPath); - return fs.mkdirSync(realPath, options); + const created = fs.mkdirSync(realPath, options); + if (created === undefined) return undefined; + return this.#resolvedToVfsPath(created, vfsPath, 'mkdir'); } async mkdir(vfsPath, options) { const realPath = this.#resolvePath(vfsPath); - return fs.promises.mkdir(realPath, options); + const created = await fs.promises.mkdir(realPath, options); + if (created === undefined) return undefined; + return this.#resolvedToVfsPath(created, vfsPath, 'mkdir'); } rmdirSync(vfsPath) { diff --git a/test/parallel/test-vfs-fs-mkdir-callback.js b/test/parallel/test-vfs-fs-mkdir-callback.js index e354cc94d01b..47bd17f42617 100644 --- a/test/parallel/test-vfs-fs-mkdir-callback.js +++ b/test/parallel/test-vfs-fs-mkdir-callback.js @@ -31,6 +31,20 @@ function mounted() { })); } +// Recursive mkdir (cb) reports the first created dir below the mount point +{ + const { myVfs, mountPoint } = mounted(); + fs.mkdir(path.join(mountPoint, 'src/cb-a/b/c'), { recursive: true }, + common.mustSucceed((created) => { + assert.strictEqual(created, path.join(mountPoint, 'src/cb-a')); + assert.strictEqual( + fs.statSync(path.join(mountPoint, 'src/cb-a/b/c')).isDirectory(), + true, + ); + myVfs.unmount(); + })); +} + // rmdir (cb) { const { myVfs, mountPoint } = mounted(); diff --git a/test/parallel/test-vfs-fs-mkdirSync.js b/test/parallel/test-vfs-fs-mkdirSync.js index de3959a06e3b..e5fffbb89a1f 100644 --- a/test/parallel/test-vfs-fs-mkdirSync.js +++ b/test/parallel/test-vfs-fs-mkdirSync.js @@ -4,11 +4,14 @@ // fs.mkdirSync dispatches to VFS, including the `recursive: true` form. require('../common'); +const tmpdir = require('../common/tmpdir'); const assert = require('assert'); const fs = require('fs'); const path = require('path'); const vfs = require('node:vfs'); +tmpdir.refresh(); + const mountPoint = path.resolve('/tmp/vfs-mkdirSync-' + process.pid); const myVfs = vfs.create(); myVfs.mkdirSync('/src', { recursive: true }); @@ -20,12 +23,36 @@ assert.strictEqual( fs.statSync(path.join(mountPoint, 'src/d1')).isDirectory(), true, ); -// Recursive mkdir creates intermediate directories and returns the first one +// Recursive mkdir creates intermediate directories and returns the first one, +// as a path below the mount point rather than a VFS-internal path. const created = fs.mkdirSync(path.join(mountPoint, 'src/a/b/c'), { recursive: true }); -assert.ok(created !== undefined); +assert.strictEqual(created, path.join(mountPoint, 'src/a')); assert.strictEqual( fs.statSync(path.join(mountPoint, 'src/a/b/c')).isDirectory(), true, ); +// Recursive mkdir with nothing left to create returns undefined. +assert.strictEqual( + fs.mkdirSync(path.join(mountPoint, 'src/a/b'), { recursive: true }), + undefined, +); + myVfs.unmount(); + +// The same holds for a mounted RealFSProvider, whose root must not leak into +// the returned path. +{ + const root = path.join(tmpdir.path, 'vfs-mkdirSync-real'); + fs.mkdirSync(root, { recursive: true }); + const realMountPoint = path.resolve('/tmp/vfs-mkdirSync-real-' + process.pid); + const realVfs = vfs.create(new vfs.RealFSProvider(root)); + realVfs.mount(realMountPoint); + + const realCreated = fs.mkdirSync(path.join(realMountPoint, 'x/y/z'), + { recursive: true }); + assert.strictEqual(realCreated, path.join(realMountPoint, 'x')); + assert.strictEqual(fs.existsSync(path.join(root, 'x/y/z')), true); + + realVfs.unmount(); +} diff --git a/test/parallel/test-vfs-fs-promises.js b/test/parallel/test-vfs-fs-promises.js index 0f09c96f83ae..e9bac2f58b3c 100644 --- a/test/parallel/test-vfs-fs-promises.js +++ b/test/parallel/test-vfs-fs-promises.js @@ -39,8 +39,11 @@ const vfs = require('node:vfs'); await fsp.appendFile(p('src/pw.txt'), ' more'); assert.strictEqual(fs.readFileSync(p('src/pw.txt'), 'utf8'), 'pdata more'); - await fsp.mkdir(p('src/pd')); + assert.strictEqual(await fsp.mkdir(p('src/pd')), undefined); await fsp.rmdir(p('src/pd')); + // Recursive mkdir reports the first created directory below the mount point. + assert.strictEqual(await fsp.mkdir(p('src/pr/sub'), { recursive: true }), + p('src/pr')); await fsp.rm(p('src/pw.txt')); assert.strictEqual(fs.existsSync(p('src/pw.txt')), false); diff --git a/test/parallel/test-vfs-real-provider-promises.js b/test/parallel/test-vfs-real-provider-promises.js index 932a30fd086d..932178633d89 100644 --- a/test/parallel/test-vfs-real-provider-promises.js +++ b/test/parallel/test-vfs-real-provider-promises.js @@ -30,7 +30,11 @@ const myVfs = vfs.create(new vfs.RealFSProvider(root)); { code: 'ENOENT' }); // mkdir / readdir / rmdir - await myVfs.promises.mkdir('/d/sub', { recursive: true }); + // Recursive mkdir reports the first created directory as a VFS path. + assert.strictEqual(await myVfs.promises.mkdir('/d/sub', { recursive: true }), + '/d'); + assert.strictEqual(await myVfs.promises.mkdir('/d/sub', { recursive: true }), + undefined); const entries = await myVfs.promises.readdir('/d'); assert.deepStrictEqual(entries.sort(), ['sub']); await myVfs.promises.rmdir('/d/sub'); diff --git a/test/parallel/test-vfs-real-provider.js b/test/parallel/test-vfs-real-provider.js index a54181d8f6dc..61c104d838cf 100644 --- a/test/parallel/test-vfs-real-provider.js +++ b/test/parallel/test-vfs-real-provider.js @@ -96,13 +96,25 @@ fs.mkdirSync(testDir, { recursive: true }); // mkdir / rmdir / recursive mkdir { const realVfs = vfs.create(new vfs.RealFSProvider(testDir)); - realVfs.mkdirSync('/new-dir'); + assert.strictEqual(realVfs.mkdirSync('/new-dir'), undefined); assert.strictEqual(fs.existsSync(path.join(testDir, 'new-dir')), true); realVfs.rmdirSync('/new-dir'); assert.strictEqual(fs.existsSync(path.join(testDir, 'new-dir')), false); - realVfs.mkdirSync('/deep/nested/dir', { recursive: true }); + // Recursive mkdir returns the first created directory as a VFS path, never + // as a path inside the provider root. + const created = realVfs.mkdirSync('/deep/nested/dir', { recursive: true }); + assert.strictEqual(created, '/deep'); assert.strictEqual(fs.existsSync(path.join(testDir, 'deep/nested/dir')), true); + + // Nothing to create resolves to undefined. + assert.strictEqual(realVfs.mkdirSync('/deep/nested', { recursive: true }), + undefined); + // Only the first missing component is reported. + assert.strictEqual(realVfs.mkdirSync('/deep/nested/dir/x/y', + { recursive: true }), '/deep/nested/dir/x'); + fs.rmdirSync(path.join(testDir, 'deep/nested/dir/x/y')); + fs.rmdirSync(path.join(testDir, 'deep/nested/dir/x')); fs.rmdirSync(path.join(testDir, 'deep/nested/dir')); fs.rmdirSync(path.join(testDir, 'deep/nested')); fs.rmdirSync(path.join(testDir, 'deep'));