Skip to content

Commit ea51c14

Browse files
pipobscurepipobscure
authored
vfs: close the fs hook gaps 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. Make them behave as they do for a real path: * Add the `watchFile`, `unwatchFile` and `promisesWatch` handlers, backed by the provider's stat watcher and async watcher; those calls threw a TypeError before. Have `watch` refuse a path that does not exist with ENOENT instead of handing back a watcher that polls forever and keeps the process alive. * 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 instead of being no-ops; 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 instead of returning the provider-relative path. * Make disposing an already closed virtual `Dir` a no-op, as on the native `Dir`, instead of rejecting with ERR_DIR_CLOSED. test-vfs-fs-hook-gaps adds a test per gap, stating the real-fs outcome as the expectation. 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 <pip@pipobscure.com> PR-URL: #65852 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>
1 parent cfdb7e6 commit ea51c14

8 files changed

Lines changed: 304 additions & 89 deletions

File tree

‎lib/fs.js‎

Lines changed: 32 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,16 +1863,16 @@ function readdir(path, options, callback) {
18631863
options = undefined;
18641864
}
18651865

1866-
const h = vfsState.handlers;
1867-
if (h !== null && vfsResult(h.readdir(path, options), callback)) return;
1868-
18691866
callback = makeCallback(callback);
18701867
options = getOptions(options);
18711868
path = getValidatedPath(path);
18721869
if (options.recursive != null) {
18731870
validateBoolean(options.recursive, 'options.recursive');
18741871
}
18751872

1873+
const h = vfsState.handlers;
1874+
if (h !== null && vfsResult(h.readdir(path, options), callback)) return;
1875+
18761876
if (options.recursive) {
18771877
readdirRecursive(path, options, callback);
18781878
return;
@@ -1909,17 +1909,18 @@ function readdir(path, options, callback) {
19091909
* @returns {string | Buffer[] | Dirent[]}
19101910
*/
19111911
function readdirSync(path, options) {
1912-
const h = vfsState.handlers;
1913-
if (h !== null) {
1914-
const result = h.readdirSync(path, options);
1915-
if (result !== undefined) return result;
1916-
}
19171912
options = getOptions(options);
19181913
path = getValidatedPath(path);
19191914
if (options.recursive != null) {
19201915
validateBoolean(options.recursive, 'options.recursive');
19211916
}
19221917

1918+
const h = vfsState.handlers;
1919+
if (h !== null) {
1920+
const result = h.readdirSync(path, options);
1921+
if (result !== undefined) return result;
1922+
}
1923+
19231924
if (options.recursive) {
19241925
return readdirSyncRecursive(path, options);
19251926
}
@@ -2422,7 +2423,7 @@ function fchmod(fd, mode, callback) {
24222423
callback = makeCallback(callback);
24232424

24242425
const h = vfsState.handlers;
2425-
if (h !== null && vfsVoid(h.fchmod(fd), callback)) return;
2426+
if (h !== null && vfsVoid(h.fchmod(fd, mode), callback)) return;
24262427

24272428
if (permission.isEnabled()) {
24282429
callback(new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'));
@@ -2441,19 +2442,18 @@ function fchmod(fd, mode, callback) {
24412442
* @returns {void}
24422443
*/
24432444
function fchmodSync(fd, mode) {
2445+
mode = parseFileMode(mode, 'mode');
2446+
24442447
const h = vfsState.handlers;
24452448
if (h !== null) {
2446-
const result = h.fchmodSync(fd);
2449+
const result = h.fchmodSync(fd, mode);
24472450
if (result !== undefined) return;
24482451
}
24492452

24502453
if (permission.isEnabled()) {
24512454
throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.');
24522455
}
2453-
binding.fchmod(
2454-
fd,
2455-
parseFileMode(mode, 'mode'),
2456-
);
2456+
binding.fchmod(fd, mode);
24572457
}
24582458

24592459
/**
@@ -2692,18 +2692,15 @@ function chownSync(path, uid, gid) {
26922692
function utimes(path, atime, mtime, callback) {
26932693
callback = makeCallback(callback);
26942694
path = getValidatedPath(path);
2695+
atime = toUnixTimestamp(atime);
2696+
mtime = toUnixTimestamp(mtime);
26952697

26962698
const h = vfsState.handlers;
26972699
if (h !== null && vfsVoid(h.utimes(path, atime, mtime), callback)) return;
26982700

26992701
const req = new FSReqCallback();
27002702
req.oncomplete = callback;
2701-
binding.utimes(
2702-
path,
2703-
toUnixTimestamp(atime),
2704-
toUnixTimestamp(mtime),
2705-
req,
2706-
);
2703+
binding.utimes(path, atime, mtime, req);
27072704
}
27082705

27092706
/**
@@ -2716,18 +2713,16 @@ function utimes(path, atime, mtime, callback) {
27162713
*/
27172714
function utimesSync(path, atime, mtime) {
27182715
path = getValidatedPath(path);
2716+
atime = toUnixTimestamp(atime);
2717+
mtime = toUnixTimestamp(mtime);
27192718

27202719
const h = vfsState.handlers;
27212720
if (h !== null) {
27222721
const result = h.utimesSync(path, atime, mtime);
27232722
if (result !== undefined) return;
27242723
}
27252724

2726-
binding.utimes(
2727-
path,
2728-
toUnixTimestamp(atime),
2729-
toUnixTimestamp(mtime),
2730-
);
2725+
binding.utimes(path, atime, mtime);
27312726
}
27322727

27332728
/**
@@ -2745,7 +2740,7 @@ function futimes(fd, atime, mtime, callback) {
27452740
callback = makeCallback(callback);
27462741

27472742
const h = vfsState.handlers;
2748-
if (h !== null && vfsVoid(h.futimes(fd), callback)) return;
2743+
if (h !== null && vfsVoid(h.futimes(fd, atime, mtime), callback)) return;
27492744

27502745
if (permission.isEnabled()) {
27512746
callback(new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.'));
@@ -2767,21 +2762,20 @@ function futimes(fd, atime, mtime, callback) {
27672762
* @returns {void}
27682763
*/
27692764
function futimesSync(fd, atime, mtime) {
2765+
atime = toUnixTimestamp(atime, 'atime');
2766+
mtime = toUnixTimestamp(mtime, 'mtime');
2767+
27702768
const h = vfsState.handlers;
27712769
if (h !== null) {
2772-
const result = h.futimesSync(fd);
2770+
const result = h.futimesSync(fd, atime, mtime);
27732771
if (result !== undefined) return;
27742772
}
27752773

27762774
if (permission.isEnabled()) {
27772775
throw new ERR_ACCESS_DENIED('futimes API is disabled when Permission Model is enabled.');
27782776
}
27792777

2780-
binding.futimes(
2781-
fd,
2782-
toUnixTimestamp(atime, 'atime'),
2783-
toUnixTimestamp(mtime, 'mtime'),
2784-
);
2778+
binding.futimes(fd, atime, mtime);
27852779
}
27862780

27872781
/**
@@ -2796,18 +2790,15 @@ function futimesSync(fd, atime, mtime) {
27962790
function lutimes(path, atime, mtime, callback) {
27972791
callback = makeCallback(callback);
27982792
path = getValidatedPath(path);
2793+
atime = toUnixTimestamp(atime);
2794+
mtime = toUnixTimestamp(mtime);
27992795

28002796
const h = vfsState.handlers;
28012797
if (h !== null && vfsVoid(h.lutimes(path, atime, mtime), callback)) return;
28022798

28032799
const req = new FSReqCallback();
28042800
req.oncomplete = callback;
2805-
binding.lutimes(
2806-
path,
2807-
toUnixTimestamp(atime),
2808-
toUnixTimestamp(mtime),
2809-
req,
2810-
);
2801+
binding.lutimes(path, atime, mtime, req);
28112802
}
28122803

28132804
/**
@@ -2820,18 +2811,16 @@ function lutimes(path, atime, mtime, callback) {
28202811
*/
28212812
function lutimesSync(path, atime, mtime) {
28222813
path = getValidatedPath(path);
2814+
atime = toUnixTimestamp(atime);
2815+
mtime = toUnixTimestamp(mtime);
28232816

28242817
const h = vfsState.handlers;
28252818
if (h !== null) {
28262819
const result = h.lutimesSync(path, atime, mtime);
28272820
if (result !== undefined) return;
28282821
}
28292822

2830-
binding.lutimes(
2831-
path,
2832-
toUnixTimestamp(atime),
2833-
toUnixTimestamp(mtime),
2834-
);
2823+
binding.lutimes(path, atime, mtime);
28352824
}
28362825

28372826
function writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback) {

‎lib/internal/fs/promises.js‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1725,17 +1725,18 @@ async function readdirRecursiveWithPermissionModel(basePath, options) {
17251725
}
17261726

17271727
async function readdir(path, options) {
1728-
const h = vfsState.handlers;
1729-
if (h !== null) {
1730-
const promise = h.readdir(path, options);
1731-
if (promise !== undefined) return await promise;
1732-
}
17331728
options = getOptions(options);
17341729

17351730
// Make shallow copy to prevent mutating options from affecting results
17361731
options = copyObject(options);
17371732

17381733
path = getValidatedPath(path);
1734+
1735+
const h = vfsState.handlers;
1736+
if (h !== null) {
1737+
const promise = h.readdir(path, options);
1738+
if (promise !== undefined) return await promise;
1739+
}
17391740
if (options.recursive) {
17401741
return readdirRecursive(path, options);
17411742
}
@@ -2011,6 +2012,8 @@ async function chown(path, uid, gid) {
20112012

20122013
async function utimes(path, atime, mtime) {
20132014
path = getValidatedPath(path);
2015+
atime = toUnixTimestamp(atime);
2016+
mtime = toUnixTimestamp(mtime);
20142017

20152018
const h = vfsState.handlers;
20162019
if (h !== null) {
@@ -2019,12 +2022,7 @@ async function utimes(path, atime, mtime) {
20192022
}
20202023

20212024
return await PromisePrototypeThen(
2022-
binding.utimes(
2023-
path,
2024-
toUnixTimestamp(atime),
2025-
toUnixTimestamp(mtime),
2026-
kUsePromises,
2027-
),
2025+
binding.utimes(path, atime, mtime, kUsePromises),
20282026
undefined,
20292027
handleErrorFromBinding,
20302028
);
@@ -2044,19 +2042,18 @@ async function futimes(handle, atime, mtime) {
20442042
}
20452043

20462044
async function lutimes(path, atime, mtime) {
2045+
path = getValidatedPath(path);
2046+
atime = toUnixTimestamp(atime);
2047+
mtime = toUnixTimestamp(mtime);
2048+
20472049
const h = vfsState.handlers;
20482050
if (h !== null) {
20492051
const promise = h.lutimes(path, atime, mtime);
20502052
if (promise !== undefined) { await promise; return; }
20512053
}
20522054

20532055
return await PromisePrototypeThen(
2054-
binding.lutimes(
2055-
getValidatedPath(path),
2056-
toUnixTimestamp(atime),
2057-
toUnixTimestamp(mtime),
2058-
kUsePromises,
2059-
),
2056+
binding.lutimes(path, atime, mtime, kUsePromises),
20602057
undefined,
20612058
handleErrorFromBinding,
20622059
);

‎lib/internal/vfs/dir.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,15 @@ class VirtualDir {
9494
this.closeSync();
9595
}
9696
}
97+
98+
async [SymbolAsyncDispose]() {
99+
if (!this.#closed) {
100+
this.closeSync();
101+
}
102+
}
97103
}
98104

99105
VirtualDir.prototype[SymbolAsyncIterator] = VirtualDir.prototype.entries;
100-
VirtualDir.prototype[SymbolAsyncDispose] = VirtualDir.prototype.close;
101106

102107
module.exports = {
103108
VirtualDir,

‎lib/internal/vfs/file_handle.js‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ const {
2020
const {
2121
createEBADF,
2222
} = require('internal/vfs/errors');
23+
const { stringToFlags, toUnixTimestamp } = require('internal/fs/utils');
24+
const { parseFileMode } = require('internal/validators');
2325

2426
// Private symbols
2527
const kPath = Symbol('kPath');
@@ -29,7 +31,6 @@ const kPosition = Symbol('kPosition');
2931
const kClosed = Symbol('kClosed');
3032
const kAccess = Symbol('kAccess');
3133

32-
const { stringToFlags } = require('internal/fs/utils');
3334
const {
3435
fs: { O_APPEND, O_CREAT, O_EXCL, O_RDONLY, O_RDWR, O_TRUNC, O_WRONLY },
3536
} = internalBinding('constants');
@@ -288,10 +289,17 @@ class VirtualFileHandle {
288289
}
289290

290291
/**
291-
* No-op chmod - VFS files don't have real permissions.
292+
* @param {number} mode The new permission bits
293+
*/
294+
chmodSync(mode) {}
295+
296+
/**
297+
* @param {number} mode The new permission bits
292298
* @returns {Promise<void>}
293299
*/
294-
async chmod() {}
300+
async chmod(mode) {
301+
this.chmodSync(mode);
302+
}
295303

296304
/**
297305
* No-op chown - VFS files don't have real ownership.
@@ -300,10 +308,19 @@ class VirtualFileHandle {
300308
async chown() {}
301309

302310
/**
303-
* No-op utimes - timestamps are handled by the provider.
311+
* @param {Date|number|string} atime The new access time
312+
* @param {Date|number|string} mtime The new modification time
313+
*/
314+
utimesSync(atime, mtime) {}
315+
316+
/**
317+
* @param {Date|number|string} atime The new access time
318+
* @param {Date|number|string} mtime The new modification time
304319
* @returns {Promise<void>}
305320
*/
306-
async utimes() {}
321+
async utimes(atime, mtime) {
322+
this.utimesSync(atime, mtime);
323+
}
307324

308325
/**
309326
* No-op datasync - VFS is in-memory.
@@ -681,6 +698,33 @@ class MemoryFileHandle extends VirtualFileHandle {
681698
throw new ERR_INVALID_STATE('stats not available');
682699
}
683700

701+
/**
702+
* @param {number} mode The new permission bits
703+
*/
704+
chmodSync(mode) {
705+
this.#checkClosed('fchmod');
706+
mode = parseFileMode(mode, 'mode');
707+
if (this.#entry) {
708+
this.#entry.mode = (this.#entry.mode & ~0o7777) | (mode & 0o7777);
709+
this.#entry.ctime = DateNow();
710+
}
711+
}
712+
713+
/**
714+
* @param {Date|number|string} atime The new access time
715+
* @param {Date|number|string} mtime The new modification time
716+
*/
717+
utimesSync(atime, mtime) {
718+
this.#checkClosed('futimes');
719+
const atimeMs = toUnixTimestamp(atime, 'atime') * 1000;
720+
const mtimeMs = toUnixTimestamp(mtime, 'mtime') * 1000;
721+
if (this.#entry) {
722+
this.#entry.atime = atimeMs;
723+
this.#entry.mtime = mtimeMs;
724+
this.#entry.ctime = DateNow();
725+
}
726+
}
727+
684728
/**
685729
* Gets file stats.
686730
* @param {object} [options] Options

0 commit comments

Comments
 (0)