From 6a5b1e3a70a3dfb71a1f543fbc551be22cf6026d Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 11:34:23 +0200 Subject: [PATCH 1/3] vfs: close gaps in the fs hooks for mounted paths Several `node:fs` entry points behave differently for a mounted path than for a real one, because of how the call reaches the VFS hooks: * `fs.watchFile` and `fs.promises.watch` call handler methods that do not exist, so they throw a TypeError instead of watching. * `fs.watch` on a path that does not exist returns a polling watcher instead of throwing ENOENT, and that watcher keeps the process alive. * `fs.utimesSync` and `fs.readdirSync` consult the hook before validating their arguments: numeric-string timestamps are ignored, an object timestamp becomes NaN, and an invalid encoding is accepted. * `fs.futimesSync` and `fs.fchmodSync` on a virtual descriptor are no-ops while the path forms of the same operations work. * `fs.mkdtempSync` with a prefix ending in a separator creates the directory next to the intended parent, because the prefix is resolved as a path before the suffix is appended. * `fs.mkdirSync({ recursive: true })` returns the provider-relative path of the first directory created instead of the mounted path. * Disposing an already closed virtual `Dir` asynchronously rejects with ERR_DIR_CLOSED; the real `Dir` treats disposal as idempotent. This adds a test per gap, stating the real-fs outcome as the expectation. Proposed solution: add `watchFile`, `unwatchFile` and `promisesWatch` handlers backed by the provider's stat watcher and async watcher, and have `watch` stat the path first; move the hook calls in `utimesSync` and `readdirSync` after argument validation, and coerce times with `toUnixTimestamp` in the hook; route `futimes`/`fchmod` to the handle's entry; strip the trailing separator only after computing the temp name in `mkdtemp`; map the recursive `mkdir` result back under the mount point; and make `VirtualDir`'s async dispose a no-op once closed. Signed-off-by: Philipp Dunkel --- test/parallel/test-vfs-fs-hook-gaps.js | 112 +++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 test/parallel/test-vfs-fs-hook-gaps.js diff --git a/test/parallel/test-vfs-fs-hook-gaps.js b/test/parallel/test-vfs-fs-hook-gaps.js new file mode 100644 index 00000000000..a863f179fd2 --- /dev/null +++ b/test/parallel/test-vfs-fs-hook-gaps.js @@ -0,0 +1,112 @@ +// Flags: --experimental-vfs +'use strict'; + +// `node:fs` entry points route mounted paths through the VFS hooks. Where a +// hook is missing, runs before argument validation, or ignores the +// descriptor form of an operation, the same call behaves differently from a +// real path. Each case states the real-fs outcome as the expectation. Cases +// are independent so the runner reports each one. + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const vfs = require('node:vfs'); +const { test } = require('node:test'); + +function mount(populate) { + const layer = vfs.create(); + populate?.(layer); + return layer.mount(); +} + +test('watchFile on a mounted path installs a stat watcher', () => { + const file = path.join(mount((l) => l.writeFileSync('/f', 'x')), 'f'); + fs.watchFile(file, { interval: 10 }, common.mustNotCall()); + fs.unwatchFile(file); +}); + +test('fs.promises.watch on a mounted directory yields events', async () => { + const dir = path.join(mount((l) => l.mkdirSync('/d')), 'd'); + const ac = new AbortController(); + const watcher = fs.promises.watch(dir, { signal: ac.signal }); + setTimeout(() => fs.writeFileSync(path.join(dir, 'x'), '1'), 20); + for await (const event of watcher) { + assert.strictEqual(event.filename, 'x'); + ac.abort(); + break; + } +}); + +test('watch on a missing mounted path throws ENOENT', () => { + const dir = mount(); + // Should the call return a watcher instead, it polls forever, so it is + // closed to let the process exit. + let watcher; + try { + assert.throws(() => { watcher = fs.watch(path.join(dir, 'nope')); }, + { code: 'ENOENT' }); + } finally { + watcher?.close(); + } +}); + +test('utimesSync accepts numeric strings as seconds', () => { + const file = path.join(mount((l) => l.writeFileSync('/f', 'x')), 'f'); + fs.utimesSync(file, '1000', '2000'); + assert.strictEqual(fs.statSync(file).mtimeMs, 2000 * 1000); +}); + +test('utimesSync rejects an invalid time argument', () => { + const file = path.join(mount((l) => l.writeFileSync('/f', 'x')), 'f'); + assert.throws(() => fs.utimesSync(file, {}, {}), { code: 'ERR_INVALID_ARG_TYPE' }); +}); + +test('readdirSync rejects an invalid encoding', () => { + const dir = mount(); + assert.throws(() => fs.readdirSync(dir, { encoding: 'nope' }), + { code: 'ERR_INVALID_ARG_VALUE' }); +}); + +test('futimesSync updates the timestamps through a descriptor', () => { + const file = path.join(mount((l) => l.writeFileSync('/f', 'x')), 'f'); + const fd = fs.openSync(file, 'r+'); + try { + fs.futimesSync(fd, 1000, 2000); + } finally { + fs.closeSync(fd); + } + assert.strictEqual(fs.statSync(file).mtimeMs, 2000 * 1000); +}); + +test('fchmodSync changes the mode through a descriptor', () => { + const file = path.join(mount((l) => l.writeFileSync('/f', 'x')), 'f'); + const fd = fs.openSync(file, 'r+'); + try { + fs.fchmodSync(fd, 0o600); + } finally { + fs.closeSync(fd); + } + assert.strictEqual(fs.statSync(file).mode & 0o777, 0o600); +}); + +test('mkdtempSync with a trailing separator creates the directory inside the prefix', () => { + const dir = path.join(mount((l) => l.mkdirSync('/dir')), 'dir'); + const created = fs.mkdtempSync(dir + path.sep); + assert.ok(created.startsWith(dir + path.sep), `${created} is not inside ${dir}`); + assert.strictEqual(fs.statSync(created).isDirectory(), true); +}); + +test('mkdirSync({ recursive: true }) returns the first directory created', () => { + const dir = mount(); + const created = fs.mkdirSync(path.join(dir, 'a', 'b'), { recursive: true }); + assert.strictEqual(created, path.join(dir, 'a')); +}); + +test('a closed Dir can be disposed asynchronously', async () => { + const dir = mount((l) => l.mkdirSync('/d')); + const handle = fs.opendirSync(dir); + handle.closeSync(); + // Disposal is idempotent on a real Dir. + await handle[Symbol.asyncDispose](); +}); From 13ea60ce509069d080966c14d15718991518fc6a Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 13:04:44 +0200 Subject: [PATCH 2/3] vfs: close the fs hook gaps for mounted paths Make the `node:fs` entry points that reach a mounted path behave as they do for a real one: * Add the `watchFile`, `unwatchFile` and `promisesWatch` handlers, backed by the provider's stat watcher and async watcher, and have `watch` refuse a path that does not exist with ENOENT instead of handing back a watcher that polls forever. * Convert timestamps and validate arguments before the hook runs in `utimes`, `lutimes` and `readdir` (sync, callback and promise forms), so a mounted path gets the same ERR_INVALID_ARG_* errors and the same seconds-since-epoch numbers as a real one. * Pass the mode and times through to the `fchmod` and `futimes` hooks and route them to the handle's entry, so descriptor operations take effect like their path forms; the memory handle validates the way a FileHandle would since one calls it directly. * Treat a `mkdtemp` prefix as text rather than a path when it ends in a separator, so the directory is created inside the intended parent. * Map the first directory a recursive `mkdir` created back under the mount point. * Make disposing an already closed virtual `Dir` a no-op, as on the native `Dir`. The existing file handle test asserted that `chmod()` and `utimes()` without arguments were no-ops; they now validate and apply, so it exercises that instead. Signed-off-by: Philipp Dunkel --- lib/fs.js | 79 ++++++++++++--------------- lib/internal/fs/promises.js | 35 ++++++------ lib/internal/vfs/dir.js | 9 ++- lib/internal/vfs/file_handle.js | 65 ++++++++++++++++++++-- lib/internal/vfs/file_system.js | 52 ++++++++++++------ lib/internal/vfs/setup.js | 58 ++++++++++++++++++-- test/parallel/test-vfs-file-handle.js | 11 +++- 7 files changed, 221 insertions(+), 88 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index b858902cf44..6da91f260ef 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1864,9 +1864,6 @@ function readdir(path, options, callback) { options = undefined; } - const h = vfsState.handlers; - if (h !== null && vfsResult(h.readdir(path, options), callback)) return; - callback = makeCallback(callback); options = getOptions(options); path = getValidatedPath(path); @@ -1874,6 +1871,9 @@ function readdir(path, options, callback) { validateBoolean(options.recursive, 'options.recursive'); } + const h = vfsState.handlers; + if (h !== null && vfsResult(h.readdir(path, options), callback)) return; + if (options.recursive) { readdirRecursive(path, options, callback); return; @@ -1910,17 +1910,20 @@ function readdir(path, options, callback) { * @returns {string | Buffer[] | Dirent[]} */ function readdirSync(path, options) { - const h = vfsState.handlers; - if (h !== null) { - const result = h.readdirSync(path, options); - if (result !== undefined) return result; - } options = getOptions(options); path = getValidatedPath(path); if (options.recursive != null) { validateBoolean(options.recursive, 'options.recursive'); } + // After validation, so a mounted path rejects the same bad arguments as + // a real one. + const h = vfsState.handlers; + if (h !== null) { + const result = h.readdirSync(path, options); + if (result !== undefined) return result; + } + if (options.recursive) { return readdirSyncRecursive(path, options); } @@ -2417,7 +2420,7 @@ function fchmod(fd, mode, callback) { callback = makeCallback(callback); const h = vfsState.handlers; - if (h !== null && vfsVoid(h.fchmod(fd), callback)) return; + if (h !== null && vfsVoid(h.fchmod(fd, mode), callback)) return; if (permission.isEnabled()) { callback(new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.')); @@ -2436,19 +2439,18 @@ function fchmod(fd, mode, callback) { * @returns {void} */ function fchmodSync(fd, mode) { + mode = parseFileMode(mode, 'mode'); + const h = vfsState.handlers; if (h !== null) { - const result = h.fchmodSync(fd); + const result = h.fchmodSync(fd, mode); if (result !== undefined) return; } if (permission.isEnabled()) { throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'); } - binding.fchmod( - fd, - parseFileMode(mode, 'mode'), - ); + binding.fchmod(fd, mode); } /** @@ -2687,18 +2689,15 @@ function chownSync(path, uid, gid) { function utimes(path, atime, mtime, callback) { callback = makeCallback(callback); path = getValidatedPath(path); + atime = toUnixTimestamp(atime); + mtime = toUnixTimestamp(mtime); const h = vfsState.handlers; if (h !== null && vfsVoid(h.utimes(path, atime, mtime), callback)) return; const req = new FSReqCallback(); req.oncomplete = callback; - binding.utimes( - path, - toUnixTimestamp(atime), - toUnixTimestamp(mtime), - req, - ); + binding.utimes(path, atime, mtime, req); } /** @@ -2711,6 +2710,10 @@ function utimes(path, atime, mtime, callback) { */ function utimesSync(path, atime, mtime) { path = getValidatedPath(path); + // Converted before the VFS hook so a mounted path gets the same + // validation and the same seconds-since-epoch numbers as a real one. + atime = toUnixTimestamp(atime); + mtime = toUnixTimestamp(mtime); const h = vfsState.handlers; if (h !== null) { @@ -2718,11 +2721,7 @@ function utimesSync(path, atime, mtime) { if (result !== undefined) return; } - binding.utimes( - path, - toUnixTimestamp(atime), - toUnixTimestamp(mtime), - ); + binding.utimes(path, atime, mtime); } /** @@ -2740,7 +2739,7 @@ function futimes(fd, atime, mtime, callback) { callback = makeCallback(callback); const h = vfsState.handlers; - if (h !== null && vfsVoid(h.futimes(fd), callback)) return; + if (h !== null && vfsVoid(h.futimes(fd, atime, mtime), callback)) return; if (permission.isEnabled()) { callback(new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.')); @@ -2762,9 +2761,12 @@ function futimes(fd, atime, mtime, callback) { * @returns {void} */ function futimesSync(fd, atime, mtime) { + atime = toUnixTimestamp(atime, 'atime'); + mtime = toUnixTimestamp(mtime, 'mtime'); + const h = vfsState.handlers; if (h !== null) { - const result = h.futimesSync(fd); + const result = h.futimesSync(fd, atime, mtime); if (result !== undefined) return; } @@ -2772,11 +2774,7 @@ function futimesSync(fd, atime, mtime) { throw new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.'); } - binding.futimes( - fd, - toUnixTimestamp(atime, 'atime'), - toUnixTimestamp(mtime, 'mtime'), - ); + binding.futimes(fd, atime, mtime); } /** @@ -2791,18 +2789,15 @@ function futimesSync(fd, atime, mtime) { function lutimes(path, atime, mtime, callback) { callback = makeCallback(callback); path = getValidatedPath(path); + atime = toUnixTimestamp(atime); + mtime = toUnixTimestamp(mtime); const h = vfsState.handlers; if (h !== null && vfsVoid(h.lutimes(path, atime, mtime), callback)) return; const req = new FSReqCallback(); req.oncomplete = callback; - binding.lutimes( - path, - toUnixTimestamp(atime), - toUnixTimestamp(mtime), - req, - ); + binding.lutimes(path, atime, mtime, req); } /** @@ -2815,6 +2810,8 @@ function lutimes(path, atime, mtime, callback) { */ function lutimesSync(path, atime, mtime) { path = getValidatedPath(path); + atime = toUnixTimestamp(atime); + mtime = toUnixTimestamp(mtime); const h = vfsState.handlers; if (h !== null) { @@ -2822,11 +2819,7 @@ function lutimesSync(path, atime, mtime) { if (result !== undefined) return; } - binding.lutimes( - path, - toUnixTimestamp(atime), - toUnixTimestamp(mtime), - ); + binding.lutimes(path, atime, mtime); } function writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback) { diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index d5a6b9a2c85..9db35d53cb5 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1668,17 +1668,20 @@ async function readdirRecursiveWithPermissionModel(basePath, options) { } async function readdir(path, options) { - const h = vfsState.handlers; - if (h !== null) { - const promise = h.readdir(path, options); - if (promise !== undefined) return await promise; - } options = getOptions(options); // Make shallow copy to prevent mutating options from affecting results options = copyObject(options); path = getValidatedPath(path); + + // After validation, so a mounted path rejects the same bad arguments as + // a real one. + const h = vfsState.handlers; + if (h !== null) { + const promise = h.readdir(path, options); + if (promise !== undefined) return await promise; + } if (options.recursive) { return readdirRecursive(path, options); } @@ -1954,6 +1957,10 @@ async function chown(path, uid, gid) { async function utimes(path, atime, mtime) { path = getValidatedPath(path); + // Converted before the VFS hook so a mounted path gets the same + // validation and the same seconds-since-epoch numbers as a real one. + atime = toUnixTimestamp(atime); + mtime = toUnixTimestamp(mtime); const h = vfsState.handlers; if (h !== null) { @@ -1962,12 +1969,7 @@ async function utimes(path, atime, mtime) { } return await PromisePrototypeThen( - binding.utimes( - path, - toUnixTimestamp(atime), - toUnixTimestamp(mtime), - kUsePromises, - ), + binding.utimes(path, atime, mtime, kUsePromises), undefined, handleErrorFromBinding, ); @@ -1987,6 +1989,10 @@ async function futimes(handle, atime, mtime) { } async function lutimes(path, atime, mtime) { + path = getValidatedPath(path); + atime = toUnixTimestamp(atime); + mtime = toUnixTimestamp(mtime); + const h = vfsState.handlers; if (h !== null) { const promise = h.lutimes(path, atime, mtime); @@ -1994,12 +2000,7 @@ async function lutimes(path, atime, mtime) { } return await PromisePrototypeThen( - binding.lutimes( - getValidatedPath(path), - toUnixTimestamp(atime), - toUnixTimestamp(mtime), - kUsePromises, - ), + binding.lutimes(path, atime, mtime, kUsePromises), undefined, handleErrorFromBinding, ); diff --git a/lib/internal/vfs/dir.js b/lib/internal/vfs/dir.js index 803aeb40453..f8f4b6d6ce4 100644 --- a/lib/internal/vfs/dir.js +++ b/lib/internal/vfs/dir.js @@ -94,10 +94,17 @@ class VirtualDir { this.closeSync(); } } + + // Disposal is idempotent, as on the native Dir: a handle that was already + // closed by hand is not an error to dispose again. + async [SymbolAsyncDispose]() { + if (!this.#closed) { + this.closeSync(); + } + } } VirtualDir.prototype[SymbolAsyncIterator] = VirtualDir.prototype.entries; -VirtualDir.prototype[SymbolAsyncDispose] = VirtualDir.prototype.close; module.exports = { VirtualDir, diff --git a/lib/internal/vfs/file_handle.js b/lib/internal/vfs/file_handle.js index 7b60c9def2b..2e70b42ed6e 100644 --- a/lib/internal/vfs/file_handle.js +++ b/lib/internal/vfs/file_handle.js @@ -20,6 +20,8 @@ const { const { createEBADF, } = require('internal/vfs/errors'); +const { toUnixTimestamp } = require('internal/fs/utils'); +const { parseFileMode } = require('internal/validators'); // Private symbols const kPath = Symbol('kPath'); @@ -241,10 +243,19 @@ class VirtualFileHandle { } /** - * No-op chmod - VFS files don't have real permissions. + * Changes the file mode. Providers whose handles carry no metadata leave + * this a no-op; those that do override it. + * @param {number} mode The new permission bits + */ + chmodSync(mode) {} + + /** + * @param {number} mode The new permission bits * @returns {Promise} */ - async chmod() {} + async chmod(mode) { + this.chmodSync(mode); + } /** * No-op chown - VFS files don't have real ownership. @@ -253,10 +264,21 @@ class VirtualFileHandle { async chown() {} /** - * No-op utimes - timestamps are handled by the provider. + * Changes the timestamps. Providers whose handles carry no metadata leave + * this a no-op; those that do override it. + * @param {Date|number|string} atime The new access time + * @param {Date|number|string} mtime The new modification time + */ + utimesSync(atime, mtime) {} + + /** + * @param {Date|number|string} atime The new access time + * @param {Date|number|string} mtime The new modification time * @returns {Promise} */ - async utimes() {} + async utimes(atime, mtime) { + this.utimesSync(atime, mtime); + } /** * No-op datasync - VFS is in-memory. @@ -666,6 +688,41 @@ class MemoryFileHandle extends VirtualFileHandle { throw new ERR_INVALID_STATE('stats not available'); } + /** + * Changes the permission bits of the underlying entry, as fchmod(2) does + * through a descriptor. The type bits are kept. + * @param {number} mode The new permission bits + */ + chmodSync(mode) { + this.#checkClosed('fchmod'); + // Validated here as well because a `FileHandle` calls this method + // directly with the caller's argument. + mode = parseFileMode(mode, 'mode'); + if (this.#entry) { + this.#entry.mode = (this.#entry.mode & ~0o7777) | (mode & 0o7777); + this.#entry.ctime = DateNow(); + } + } + + /** + * Changes the timestamps of the underlying entry, as futimes(2) does + * through a descriptor. Accepts what `fs.utimes` accepts; the values are + * validated and converted here because a `FileHandle` calls this method + * directly with the caller's arguments. + * @param {Date|number|string} atime The new access time + * @param {Date|number|string} mtime The new modification time + */ + utimesSync(atime, mtime) { + this.#checkClosed('futimes'); + const atimeMs = toUnixTimestamp(atime, 'atime') * 1000; + const mtimeMs = toUnixTimestamp(mtime, 'mtime') * 1000; + if (this.#entry) { + this.#entry.atime = atimeMs; + this.#entry.mtime = mtimeMs; + this.#entry.ctime = DateNow(); + } + } + /** * Gets file stats. * @param {object} [options] Options diff --git a/lib/internal/vfs/file_system.js b/lib/internal/vfs/file_system.js index 574c076c426..1fe02260b9e 100644 --- a/lib/internal/vfs/file_system.js +++ b/lib/internal/vfs/file_system.js @@ -63,6 +63,17 @@ function normalizeMountedPath(inputPath) { return toNamespacedPath(resolvePath(inputPath)); } +const kTempChars = + 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; + +function randomSuffix() { + let suffix = ''; + for (let i = 0; i < 6; i++) { + suffix += kTempChars[(MathRandom() * kTempChars.length) | 0]; + } + return suffix; +} + let registerVFS; let deregisterVFS; @@ -359,7 +370,10 @@ class VirtualFileSystem { */ mkdirSync(dirPath, options) { const providerPath = this.#toProviderPath(dirPath); - return this[kProvider].mkdirSync(providerPath, options); + const created = this[kProvider].mkdirSync(providerPath, options); + // A recursive mkdir reports the first directory it created, which the + // provider names relative to itself. + return created === undefined ? undefined : this.#toMountedPath(created); } /** @@ -557,17 +571,27 @@ class VirtualFileSystem { * @returns {string} The full path of the created directory */ mkdtempSync(prefix) { - const providerPrefix = this.#toProviderPath(prefix); - const chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; - let suffix = ''; - for (let i = 0; i < 6; i++) { - suffix += chars[(MathRandom() * chars.length) | 0]; - } - const dirPath = providerPrefix + suffix; + const dirPath = this.#toProviderPrefix(prefix) + randomSuffix(); this[kProvider].mkdirSync(dirPath); return this.#toMountedPath(dirPath); } + /** + * Converts a mkdtemp prefix to a provider-relative one. The prefix is + * text that the random suffix is appended to, not a path to resolve: a + * trailing separator means "inside this directory", and resolving would + * drop it and turn `dir/` + suffix into a sibling of `dir`. + * @param {string} prefix The mounted prefix + * @returns {string} + */ + #toProviderPrefix(prefix) { + const last = prefix[prefix.length - 1]; + const trailing = last === '/' || last === sep; + const providerPrefix = this.#toProviderPath(prefix); + if (!trailing) return providerPrefix; + return providerPrefix === '/' ? '/' : `${providerPrefix}/`; + } + /** * Opens a directory synchronously. * @param {string} dirPath The directory path @@ -1106,6 +1130,7 @@ class VirtualFileSystem { // Arrow functions capture `this` for private method access. const toProviderPath = (p) => this.#toProviderPath(p); + const toProviderPrefix = (p) => this.#toProviderPrefix(p); const toMountedPath = (p) => this.#toMountedPath(p); return ObjectFreeze({ @@ -1141,7 +1166,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) { @@ -1235,13 +1261,7 @@ class VirtualFileSystem { }, async mkdtemp(prefix) { - const providerPrefix = toProviderPath(prefix); - const chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; - let suffix = ''; - for (let i = 0; i < 6; i++) { - suffix += chars[(MathRandom() * chars.length) | 0]; - } - const dirPath = providerPrefix + suffix; + const dirPath = toProviderPrefix(prefix) + randomSuffix(); await provider.mkdir(dirPath); return toMountedPath(dirPath); }, diff --git a/lib/internal/vfs/setup.js b/lib/internal/vfs/setup.js index a45cd47a9bf..e29e0b48829 100644 --- a/lib/internal/vfs/setup.js +++ b/lib/internal/vfs/setup.js @@ -511,9 +511,17 @@ function createVfsHandlers() { if (vfd) { vfd.entry.truncateSync(len); return true; } return undefined; }, - fchmodSync: noopFdSync, + fchmodSync(fd, mode) { + const vfd = getVirtualFd(fd); + if (vfd) { vfd.entry.chmodSync(mode); return true; } + return undefined; + }, fchownSync: noopFdSync, - futimesSync: noopFdSync, + futimesSync(fd, atime, mtime) { + const vfd = getVirtualFd(fd); + if (vfd) { vfd.entry.utimesSync(atime, mtime); return true; } + return undefined; + }, fdatasyncSync: noopFdSync, fsyncSync: noopFdSync, readvSync(fd, buffers, position) { @@ -570,9 +578,17 @@ function createVfsHandlers() { if (!vfd) return undefined; return vfd.entry.truncate(len).then(() => true); }, - fchmod: noopFd, + fchmod(fd, mode) { + const vfd = getVirtualFd(fd); + if (!vfd) return undefined; + return vfd.entry.chmod(mode).then(() => true); + }, fchown: noopFd, - futimes: noopFd, + futimes(fd, atime, mtime) { + const vfd = getVirtualFd(fd); + if (!vfd) return undefined; + return vfd.entry.utimes(atime, mtime).then(() => true); + }, fdatasync: noopFd, fsync: noopFd, @@ -605,10 +621,42 @@ function createVfsHandlers() { const pathStr = toPathStr(filename); if (pathStr !== null) { const r = findVFSForPath(pathStr); - if (r !== null) return r.vfs.watch(pathStr, options, listener); + if (r !== null) { + // A provider watcher polls whatever it is given; the real fs + // refuses up front when there is nothing to watch. + if (!r.vfs.existsSync(pathStr)) throw createENOENT('watch', pathStr); + return r.vfs.watch(pathStr, options, listener); + } } return undefined; }, + watchFile(filename, options, listener) { + const pathStr = toPathStr(filename); + if (pathStr === null) return undefined; + const r = findVFSForPath(pathStr); + if (r === null) return undefined; + if (options === null || typeof options !== 'object') { + listener = options; + options = kEmptyObject; + } + return r.vfs.watchFile(pathStr, options, listener); + }, + unwatchFile(filename, listener) { + const pathStr = toPathStr(filename); + if (pathStr === null) return undefined; + const r = findVFSForPath(pathStr); + if (r === null) return undefined; + r.vfs.unwatchFile(pathStr, listener); + return true; + }, + promisesWatch(filename, options) { + const pathStr = toPathStr(filename); + if (pathStr === null) return undefined; + const r = findVFSForPath(pathStr); + if (r === null) return undefined; + if (!r.vfs.existsSync(pathStr)) throw createENOENT('watch', pathStr); + return r.vfs.promises.watch(pathStr, options); + }, readdir(path, options) { const promise = vfsOp(path, (vfs, n) => vfs.promises.readdir(n, options)); diff --git a/test/parallel/test-vfs-file-handle.js b/test/parallel/test-vfs-file-handle.js index d9d919446b8..4b86714fbc5 100644 --- a/test/parallel/test-vfs-file-handle.js +++ b/test/parallel/test-vfs-file-handle.js @@ -42,10 +42,17 @@ myVfs.writeFileSync('/file.txt', 'hello world'); assert.strictEqual(b1.toString(), 'hello'); assert.strictEqual(b2.toString(), ' world'); + // Metadata methods reach the entry the way fchmod(2)/futimes(2) do, and + // validate their arguments the way a FileHandle would. + await handle.chmod(0o600); + assert.strictEqual((await handle.stat()).mode & 0o777, 0o600); + await handle.utimes(1000, 2000); + assert.strictEqual((await handle.stat()).mtimeMs, 2000 * 1000); + await assert.rejects(handle.chmod(), { code: 'ERR_INVALID_ARG_TYPE' }); + await assert.rejects(handle.utimes(), { code: 'ERR_INVALID_ARG_TYPE' }); + // no-op metadata methods - await handle.chmod(); await handle.chown(); - await handle.utimes(); await handle.datasync(); await handle.sync(); From fc111c7905f6afaa49a5f63871efd052f14958b3 Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 20:45:08 +0200 Subject: [PATCH 3/3] vfs: drop comments that restate the code The previous commit added comments that narrate what the adjacent code does. Its commit message already carries the reasoning, so remove them. Signed-off-by: Philipp Dunkel --- lib/fs.js | 4 ---- lib/internal/fs/promises.js | 4 ---- lib/internal/vfs/dir.js | 2 -- lib/internal/vfs/file_handle.js | 12 ------------ lib/internal/vfs/file_system.js | 8 ++------ lib/internal/vfs/setup.js | 2 -- test/parallel/test-vfs-fs-hook-gaps.js | 1 - 7 files changed, 2 insertions(+), 31 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 6da91f260ef..5a5f13586aa 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1916,8 +1916,6 @@ function readdirSync(path, options) { validateBoolean(options.recursive, 'options.recursive'); } - // After validation, so a mounted path rejects the same bad arguments as - // a real one. const h = vfsState.handlers; if (h !== null) { const result = h.readdirSync(path, options); @@ -2710,8 +2708,6 @@ function utimes(path, atime, mtime, callback) { */ function utimesSync(path, atime, mtime) { path = getValidatedPath(path); - // Converted before the VFS hook so a mounted path gets the same - // validation and the same seconds-since-epoch numbers as a real one. atime = toUnixTimestamp(atime); mtime = toUnixTimestamp(mtime); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 9db35d53cb5..631189b6e6f 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1675,8 +1675,6 @@ async function readdir(path, options) { path = getValidatedPath(path); - // After validation, so a mounted path rejects the same bad arguments as - // a real one. const h = vfsState.handlers; if (h !== null) { const promise = h.readdir(path, options); @@ -1957,8 +1955,6 @@ async function chown(path, uid, gid) { async function utimes(path, atime, mtime) { path = getValidatedPath(path); - // Converted before the VFS hook so a mounted path gets the same - // validation and the same seconds-since-epoch numbers as a real one. atime = toUnixTimestamp(atime); mtime = toUnixTimestamp(mtime); diff --git a/lib/internal/vfs/dir.js b/lib/internal/vfs/dir.js index f8f4b6d6ce4..3b0a6140b1e 100644 --- a/lib/internal/vfs/dir.js +++ b/lib/internal/vfs/dir.js @@ -95,8 +95,6 @@ class VirtualDir { } } - // Disposal is idempotent, as on the native Dir: a handle that was already - // closed by hand is not an error to dispose again. async [SymbolAsyncDispose]() { if (!this.#closed) { this.closeSync(); diff --git a/lib/internal/vfs/file_handle.js b/lib/internal/vfs/file_handle.js index 2e70b42ed6e..dd6fa3616da 100644 --- a/lib/internal/vfs/file_handle.js +++ b/lib/internal/vfs/file_handle.js @@ -243,8 +243,6 @@ class VirtualFileHandle { } /** - * Changes the file mode. Providers whose handles carry no metadata leave - * this a no-op; those that do override it. * @param {number} mode The new permission bits */ chmodSync(mode) {} @@ -264,8 +262,6 @@ class VirtualFileHandle { async chown() {} /** - * Changes the timestamps. Providers whose handles carry no metadata leave - * this a no-op; those that do override it. * @param {Date|number|string} atime The new access time * @param {Date|number|string} mtime The new modification time */ @@ -689,14 +685,10 @@ class MemoryFileHandle extends VirtualFileHandle { } /** - * Changes the permission bits of the underlying entry, as fchmod(2) does - * through a descriptor. The type bits are kept. * @param {number} mode The new permission bits */ chmodSync(mode) { this.#checkClosed('fchmod'); - // Validated here as well because a `FileHandle` calls this method - // directly with the caller's argument. mode = parseFileMode(mode, 'mode'); if (this.#entry) { this.#entry.mode = (this.#entry.mode & ~0o7777) | (mode & 0o7777); @@ -705,10 +697,6 @@ class MemoryFileHandle extends VirtualFileHandle { } /** - * Changes the timestamps of the underlying entry, as futimes(2) does - * through a descriptor. Accepts what `fs.utimes` accepts; the values are - * validated and converted here because a `FileHandle` calls this method - * directly with the caller's arguments. * @param {Date|number|string} atime The new access time * @param {Date|number|string} mtime The new modification time */ diff --git a/lib/internal/vfs/file_system.js b/lib/internal/vfs/file_system.js index 1fe02260b9e..afb5fab3eb7 100644 --- a/lib/internal/vfs/file_system.js +++ b/lib/internal/vfs/file_system.js @@ -371,8 +371,6 @@ class VirtualFileSystem { mkdirSync(dirPath, options) { const providerPath = this.#toProviderPath(dirPath); const created = this[kProvider].mkdirSync(providerPath, options); - // A recursive mkdir reports the first directory it created, which the - // provider names relative to itself. return created === undefined ? undefined : this.#toMountedPath(created); } @@ -577,10 +575,8 @@ class VirtualFileSystem { } /** - * Converts a mkdtemp prefix to a provider-relative one. The prefix is - * text that the random suffix is appended to, not a path to resolve: a - * trailing separator means "inside this directory", and resolving would - * drop it and turn `dir/` + suffix into a sibling of `dir`. + * Converts a mkdtemp prefix to a provider-relative one, keeping a + * trailing separator. * @param {string} prefix The mounted prefix * @returns {string} */ diff --git a/lib/internal/vfs/setup.js b/lib/internal/vfs/setup.js index e29e0b48829..6e0170f4a0b 100644 --- a/lib/internal/vfs/setup.js +++ b/lib/internal/vfs/setup.js @@ -622,8 +622,6 @@ function createVfsHandlers() { if (pathStr !== null) { const r = findVFSForPath(pathStr); if (r !== null) { - // A provider watcher polls whatever it is given; the real fs - // refuses up front when there is nothing to watch. if (!r.vfs.existsSync(pathStr)) throw createENOENT('watch', pathStr); return r.vfs.watch(pathStr, options, listener); } diff --git a/test/parallel/test-vfs-fs-hook-gaps.js b/test/parallel/test-vfs-fs-hook-gaps.js index a863f179fd2..a500d796599 100644 --- a/test/parallel/test-vfs-fs-hook-gaps.js +++ b/test/parallel/test-vfs-fs-hook-gaps.js @@ -107,6 +107,5 @@ test('a closed Dir can be disposed asynchronously', async () => { const dir = mount((l) => l.mkdirSync('/d')); const handle = fs.opendirSync(dir); handle.closeSync(); - // Disposal is idempotent on a real Dir. await handle[Symbol.asyncDispose](); });