Skip to content

Commit 6f3bdfa

Browse files
committed
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. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com>
1 parent 3f2fc8a commit 6f3bdfa

7 files changed

Lines changed: 82 additions & 10 deletions

File tree

lib/internal/vfs/file_system.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,11 @@ class VirtualFileSystem {
327327
*/
328328
mkdirSync(dirPath, options) {
329329
const providerPath = this.#toProviderPath(dirPath);
330-
return this[kProvider].mkdirSync(providerPath, options);
330+
const created = this[kProvider].mkdirSync(providerPath, options);
331+
// With `recursive: true` the provider returns the first directory it
332+
// created as a provider-relative path, which has to be mapped back to a
333+
// path the caller can use. Anything else resolves to `undefined`.
334+
return created === undefined ? undefined : this.#toMountedPath(created);
331335
}
332336

333337
/**
@@ -1122,7 +1126,8 @@ class VirtualFileSystem {
11221126

11231127
async mkdir(dirPath, options) {
11241128
const providerPath = toProviderPath(dirPath);
1125-
return provider.mkdir(providerPath, options);
1129+
const created = await provider.mkdir(providerPath, options);
1130+
return created === undefined ? undefined : toMountedPath(created);
11261131
},
11271132

11281133
async rmdir(dirPath) {

lib/internal/vfs/providers/real.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,14 +378,21 @@ class RealFSProvider extends VirtualProvider {
378378
return fs.promises.readdir(realPath, options);
379379
}
380380

381+
// With `recursive: true` the underlying fs returns the first directory that
382+
// was created as a real filesystem path. It has to be mapped back into the
383+
// provider's namespace so the root path is never leaked to the caller.
381384
mkdirSync(vfsPath, options) {
382385
const realPath = this.#resolvePath(vfsPath);
383-
return fs.mkdirSync(realPath, options);
386+
const created = fs.mkdirSync(realPath, options);
387+
if (created === undefined) return undefined;
388+
return this.#resolvedToVfsPath(created, vfsPath, 'mkdir');
384389
}
385390

386391
async mkdir(vfsPath, options) {
387392
const realPath = this.#resolvePath(vfsPath);
388-
return fs.promises.mkdir(realPath, options);
393+
const created = await fs.promises.mkdir(realPath, options);
394+
if (created === undefined) return undefined;
395+
return this.#resolvedToVfsPath(created, vfsPath, 'mkdir');
389396
}
390397

391398
rmdirSync(vfsPath) {

test/parallel/test-vfs-fs-mkdir-callback.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@ function mounted() {
3131
}));
3232
}
3333

34+
// Recursive mkdir (cb) reports the first created dir below the mount point
35+
{
36+
const { myVfs, mountPoint } = mounted();
37+
fs.mkdir(path.join(mountPoint, 'src/cb-a/b/c'), { recursive: true },
38+
common.mustSucceed((created) => {
39+
assert.strictEqual(created, path.join(mountPoint, 'src/cb-a'));
40+
assert.strictEqual(
41+
fs.statSync(path.join(mountPoint, 'src/cb-a/b/c')).isDirectory(),
42+
true,
43+
);
44+
myVfs.unmount();
45+
}));
46+
}
47+
3448
// rmdir (cb)
3549
{
3650
const { myVfs, mountPoint } = mounted();

test/parallel/test-vfs-fs-mkdirSync.js

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44
// fs.mkdirSync dispatches to VFS, including the `recursive: true` form.
55

66
require('../common');
7+
const tmpdir = require('../common/tmpdir');
78
const assert = require('assert');
89
const fs = require('fs');
910
const path = require('path');
1011
const vfs = require('node:vfs');
1112

13+
tmpdir.refresh();
14+
1215
const mountPoint = path.resolve('/tmp/vfs-mkdirSync-' + process.pid);
1316
const myVfs = vfs.create();
1417
myVfs.mkdirSync('/src', { recursive: true });
@@ -20,12 +23,36 @@ assert.strictEqual(
2023
fs.statSync(path.join(mountPoint, 'src/d1')).isDirectory(), true,
2124
);
2225

23-
// Recursive mkdir creates intermediate directories and returns the first one
26+
// Recursive mkdir creates intermediate directories and returns the first one,
27+
// as a path below the mount point rather than a VFS-internal path.
2428
const created = fs.mkdirSync(path.join(mountPoint, 'src/a/b/c'),
2529
{ recursive: true });
26-
assert.ok(created !== undefined);
30+
assert.strictEqual(created, path.join(mountPoint, 'src/a'));
2731
assert.strictEqual(
2832
fs.statSync(path.join(mountPoint, 'src/a/b/c')).isDirectory(), true,
2933
);
3034

35+
// Recursive mkdir with nothing left to create returns undefined.
36+
assert.strictEqual(
37+
fs.mkdirSync(path.join(mountPoint, 'src/a/b'), { recursive: true }),
38+
undefined,
39+
);
40+
3141
myVfs.unmount();
42+
43+
// The same holds for a mounted RealFSProvider, whose root must not leak into
44+
// the returned path.
45+
{
46+
const root = path.join(tmpdir.path, 'vfs-mkdirSync-real');
47+
fs.mkdirSync(root, { recursive: true });
48+
const realMountPoint = path.resolve('/tmp/vfs-mkdirSync-real-' + process.pid);
49+
const realVfs = vfs.create(new vfs.RealFSProvider(root));
50+
realVfs.mount(realMountPoint);
51+
52+
const realCreated = fs.mkdirSync(path.join(realMountPoint, 'x/y/z'),
53+
{ recursive: true });
54+
assert.strictEqual(realCreated, path.join(realMountPoint, 'x'));
55+
assert.strictEqual(fs.existsSync(path.join(root, 'x/y/z')), true);
56+
57+
realVfs.unmount();
58+
}

test/parallel/test-vfs-fs-promises.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,11 @@ const vfs = require('node:vfs');
3939
await fsp.appendFile(p('src/pw.txt'), ' more');
4040
assert.strictEqual(fs.readFileSync(p('src/pw.txt'), 'utf8'), 'pdata more');
4141

42-
await fsp.mkdir(p('src/pd'));
42+
assert.strictEqual(await fsp.mkdir(p('src/pd')), undefined);
4343
await fsp.rmdir(p('src/pd'));
44+
// Recursive mkdir reports the first created directory below the mount point.
45+
assert.strictEqual(await fsp.mkdir(p('src/pr/sub'), { recursive: true }),
46+
p('src/pr'));
4447
await fsp.rm(p('src/pw.txt'));
4548
assert.strictEqual(fs.existsSync(p('src/pw.txt')), false);
4649

test/parallel/test-vfs-real-provider-promises.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ const myVfs = vfs.create(new vfs.RealFSProvider(root));
3030
{ code: 'ENOENT' });
3131

3232
// mkdir / readdir / rmdir
33-
await myVfs.promises.mkdir('/d/sub', { recursive: true });
33+
// Recursive mkdir reports the first created directory as a VFS path.
34+
assert.strictEqual(await myVfs.promises.mkdir('/d/sub', { recursive: true }),
35+
'/d');
36+
assert.strictEqual(await myVfs.promises.mkdir('/d/sub', { recursive: true }),
37+
undefined);
3438
const entries = await myVfs.promises.readdir('/d');
3539
assert.deepStrictEqual(entries.sort(), ['sub']);
3640
await myVfs.promises.rmdir('/d/sub');

test/parallel/test-vfs-real-provider.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,25 @@ fs.mkdirSync(testDir, { recursive: true });
9696
// mkdir / rmdir / recursive mkdir
9797
{
9898
const realVfs = vfs.create(new vfs.RealFSProvider(testDir));
99-
realVfs.mkdirSync('/new-dir');
99+
assert.strictEqual(realVfs.mkdirSync('/new-dir'), undefined);
100100
assert.strictEqual(fs.existsSync(path.join(testDir, 'new-dir')), true);
101101
realVfs.rmdirSync('/new-dir');
102102
assert.strictEqual(fs.existsSync(path.join(testDir, 'new-dir')), false);
103103

104-
realVfs.mkdirSync('/deep/nested/dir', { recursive: true });
104+
// Recursive mkdir returns the first created directory as a VFS path, never
105+
// as a path inside the provider root.
106+
const created = realVfs.mkdirSync('/deep/nested/dir', { recursive: true });
107+
assert.strictEqual(created, '/deep');
105108
assert.strictEqual(fs.existsSync(path.join(testDir, 'deep/nested/dir')), true);
109+
110+
// Nothing to create resolves to undefined.
111+
assert.strictEqual(realVfs.mkdirSync('/deep/nested', { recursive: true }),
112+
undefined);
113+
// Only the first missing component is reported.
114+
assert.strictEqual(realVfs.mkdirSync('/deep/nested/dir/x/y',
115+
{ recursive: true }), '/deep/nested/dir/x');
116+
fs.rmdirSync(path.join(testDir, 'deep/nested/dir/x/y'));
117+
fs.rmdirSync(path.join(testDir, 'deep/nested/dir/x'));
106118
fs.rmdirSync(path.join(testDir, 'deep/nested/dir'));
107119
fs.rmdirSync(path.join(testDir, 'deep/nested'));
108120
fs.rmdirSync(path.join(testDir, 'deep'));

0 commit comments

Comments
 (0)