From de58db85b8bd04e9d29dd75916f611443342883c Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 10:25:36 +0200 Subject: [PATCH] fix: patch the fd family (open/read/close/fstat) in installFsPatches installFsPatches never patched the descriptor family, so fs.openSync on a path inside a mounted VFS threw ENOENT even though fs.readFileSync on the same path worked: vfs.writeFileSync('/a.txt', 'hello'); vfs.mount('/mnt'); fs.readFileSync('/mnt/a.txt', 'utf8'); // 'hello' fs.openSync('/mnt/a.txt'); // ENOENT VirtualFileSystem already implemented the whole family, and fd.js already allocated virtual fds from 10000+ so they cannot shadow real ones. Only the patch layer was missing. - fd.js gains the fd-keyed operations. A handle carries its own content and position, so which VFS opened it is irrelevant; VirtualFileSystem and the node:fs patches now both delegate here instead of duplicating. - fd.js collapses fs.read/readSync's options-object and short-form overloads, which VirtualFileSystem.readSync did not accept either. - findVFSForWatch becomes findVFSForExistingPath: the open patches need the same "which VFS owns this path, honouring overlay" lookup. Write flags on an overlay mount keep falling through to the real fs, matching how readFileSync and createReadStream already treat overlays. fs.promises.open stays unpatched: it must return a real FileHandle, and VirtualFileHandle has no .fd, createReadStream, readLines or chmod. Falling through preserves today's behaviour rather than handing back a half-shaped object. Reported downstream as yao-pkg/pkg#302. Signed-off-by: robertsLando --- README.md | 4 +- lib/fd.js | 118 ++++++++++++++++++++++++++++++++ lib/file_system.js | 58 ++++------------ lib/module_hooks.js | 95 ++++++++++++++++++++++++-- test/fs-hooks.test.js | 155 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 378 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index a5c834b..55bdc9b 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ vfs.mount('/prefix'); // Start intercepting paths under /prefix vfs.unmount(); // Stop intercepting ``` -`mount()` returns the VFS instance for chaining. When mounted with `moduleHooks: true` (the default), `require()`, `import`, and core `fs` functions (`readFileSync`, `statSync`, `existsSync`, `readdirSync`, `realpathSync`, `watch`, etc.) are patched to serve files from the VFS. +`mount()` returns the VFS instance for chaining. When mounted with `moduleHooks: true` (the default), `require()`, `import`, and core `fs` functions (`readFileSync`, `statSync`, `existsSync`, `readdirSync`, `realpathSync`, `openSync`, `watch`, etc.) are patched to serve files from the VFS. Emits `vfs-mount` and `vfs-unmount` events on `process`. @@ -252,7 +252,7 @@ Higher-level operations (`readFile`, `writeFile`, `copyFile`, `exists`, `access` When `moduleHooks` is enabled (the default), mounting a VFS instance: 1. **Patches `require()` and `import`** — On Node.js 23.5+ uses `Module.registerHooks()`. On older versions falls back to `Module._resolveFilename` + `Module._extensions` patching. -2. **Patches core `fs` functions** — `readFileSync`, `statSync`, `lstatSync`, `readdirSync`, `existsSync`, `realpathSync`, `watch`, `watchFile`, `unwatchFile`. +2. **Patches core `fs` functions** — `readFileSync`, `statSync`, `lstatSync`, `readdirSync`, `existsSync`, `realpathSync`, `watch`, `watchFile`, `unwatchFile`, and the descriptor family `openSync`/`open`, `readSync`/`read`, `closeSync`/`close`, `fstatSync`/`fstat`. This means third-party code using `require()` or `fs.readFileSync()` will transparently pick up files from the VFS. diff --git a/lib/fd.js b/lib/fd.js index 5eb7ef0..018760d 100644 --- a/lib/fd.js +++ b/lib/fd.js @@ -1,5 +1,7 @@ 'use strict'; +const { createEBADF } = require('./errors.js'); + const kFd = Symbol('kFd'); const kEntry = Symbol('kEntry'); @@ -38,9 +40,125 @@ function closeVirtualFd(fd) { return openFDs.delete(fd); } +function requireVirtualFd(fd, syscall) { + const vfd = getVirtualFd(fd); + if (!vfd) { + throw createEBADF(syscall); + } + return vfd; +} + +const DEFAULT_READ_SIZE = 16384; +const kEmptyOptions = Object.freeze({}); + +function fromReadOptions(buffer, options) { + const buf = buffer ?? options.buffer ?? Buffer.alloc(DEFAULT_READ_SIZE); + return [buf, options.offset ?? 0, options.length ?? buf.byteLength, options.position ?? null]; +} + +// fs.read/readSync accept (fd, buffer, offset, length, position), +// (fd, buffer, options), (fd, options) and (fd). Collapse to positional form. +function normalizeReadArgs(buffer, offset, length, position) { + if (buffer == null) { + return fromReadOptions(null, kEmptyOptions); + } + + if (!ArrayBuffer.isView(buffer)) { + return fromReadOptions(null, buffer); + } + + if (offset !== null && typeof offset === 'object') { + return fromReadOptions(buffer, offset); + } + + return [buffer, offset ?? 0, length ?? buffer.byteLength, position ?? null]; +} + +// The callback is always the last function argument, whichever overload was used. +function splitCallback(args) { + const at = args.findIndex((arg) => typeof arg === 'function'); + if (at === -1) { + return null; + } + const callback = args[at]; + args.length = at; + return callback; +} + +// The operations below are keyed on the fd alone: a handle carries its own +// content and position, so which VirtualFileSystem opened it is irrelevant. +// VirtualFileSystem and the node:fs patches both delegate here. + +function closeFdSync(fd) { + const vfd = requireVirtualFd(fd, 'close'); + vfd.entry.closeSync(); + closeVirtualFd(fd); +} + +function readFdSync(fd, buffer, offset, length, position) { + const vfd = requireVirtualFd(fd, 'read'); + const args = normalizeReadArgs(buffer, offset, length, position); + return vfd.entry.readSync(args[0], args[1], args[2], args[3]); +} + +function fstatFdSync(fd, options) { + return requireVirtualFd(fd, 'fstat').entry.statSync(options); +} + +function noop() {} + +function closeFd(fd, callback = noop) { + const vfd = getVirtualFd(fd); + if (!vfd) { + process.nextTick(callback, createEBADF('close')); + return; + } + + vfd.entry.close().then(() => { + closeVirtualFd(fd); + callback(null); + }, callback); +} + +function readFd(fd, buffer, offset, length, position, callback) { + const rest = [buffer, offset, length, position]; + callback = splitCallback(rest) ?? callback; + + const vfd = getVirtualFd(fd); + if (!vfd) { + process.nextTick(callback, createEBADF('read')); + return; + } + + const args = normalizeReadArgs(rest[0], rest[1], rest[2], rest[3]); + vfd.entry.read(args[0], args[1], args[2], args[3]) + .then(({ bytesRead }) => callback(null, bytesRead, args[0]), callback); +} + +function fstatFd(fd, options, callback) { + if (typeof options === 'function') { + callback = options; + options = undefined; + } + + const vfd = getVirtualFd(fd); + if (!vfd) { + process.nextTick(callback, createEBADF('fstat')); + return; + } + + vfd.entry.stat(options).then((stats) => callback(null, stats), callback); +} + module.exports = { VirtualFD, openVirtualFd, getVirtualFd, closeVirtualFd, + closeFdSync, + readFdSync, + fstatFdSync, + closeFd, + readFd, + fstatFd, }; diff --git a/lib/file_system.js b/lib/file_system.js index df20db8..bae77c8 100644 --- a/lib/file_system.js +++ b/lib/file_system.js @@ -11,13 +11,16 @@ const { } = require('./router.js'); const { openVirtualFd, - getVirtualFd, - closeVirtualFd, + closeFdSync, + readFdSync, + fstatFdSync, + closeFd, + readFd, + fstatFd, } = require('./fd.js'); const { createENOENT, createENOTDIR, - createEBADF, ERR_INVALID_STATE, } = require('./errors.js'); const { VirtualReadStream } = require('./streams.js'); @@ -404,28 +407,15 @@ class VirtualFileSystem { } closeSync(fd) { - const vfd = getVirtualFd(fd); - if (!vfd) { - throw createEBADF('close'); - } - vfd.entry.closeSync(); - closeVirtualFd(fd); + closeFdSync(fd); } readSync(fd, buffer, offset, length, position) { - const vfd = getVirtualFd(fd); - if (!vfd) { - throw createEBADF('read'); - } - return vfd.entry.readSync(buffer, offset, length, position); + return readFdSync(fd, buffer, offset, length, position); } fstatSync(fd, options) { - const vfd = getVirtualFd(fd); - if (!vfd) { - throw createEBADF('fstat'); - } - return vfd.entry.statSync(options); + return fstatFdSync(fd, options); } // ==================== FS Operations (Async with Callbacks) ==================== @@ -529,28 +519,11 @@ class VirtualFileSystem { } close(fd, callback) { - const vfd = getVirtualFd(fd); - if (!vfd) { - process.nextTick(callback, createEBADF('close')); - return; - } - - vfd.entry.close() - .then(() => { - closeVirtualFd(fd); - callback(null); - }, (err) => callback(err)); + closeFd(fd, callback); } read(fd, buffer, offset, length, position, callback) { - const vfd = getVirtualFd(fd); - if (!vfd) { - process.nextTick(callback, createEBADF('read')); - return; - } - - vfd.entry.read(buffer, offset, length, position) - .then(({ bytesRead }) => callback(null, bytesRead, buffer), (err) => callback(err)); + readFd(fd, buffer, offset, length, position, callback); } fstat(fd, options, callback) { @@ -559,14 +532,7 @@ class VirtualFileSystem { options = undefined; } - const vfd = getVirtualFd(fd); - if (!vfd) { - process.nextTick(callback, createEBADF('fstat')); - return; - } - - vfd.entry.stat(options) - .then((stats) => callback(null, stats), (err) => callback(err)); + fstatFd(fd, options, callback); } // ==================== Stream Operations ==================== diff --git a/lib/module_hooks.js b/lib/module_hooks.js index 752aee8..e870af7 100644 --- a/lib/module_hooks.js +++ b/lib/module_hooks.js @@ -5,6 +5,15 @@ const { dirname, extname, isAbsolute, resolve } = path; const pathPosix = path.posix; const { pathToFileURL, fileURLToPath } = require('node:url'); const { createENOENT } = require('./errors.js'); +const { + getVirtualFd, + closeFdSync, + readFdSync, + fstatFdSync, + closeFd, + readFd, + fstatFd, +} = require('./fd.js'); const kEmptyObject = Object.freeze(Object.create(null)); const NodeModule = require('node:module'); @@ -230,7 +239,10 @@ function findVFSForReaddir(dirname, options) { return null; } -function findVFSForWatch(filename) { +// Which VFS owns this path? Overlay mounts only claim paths they actually +// hold, so anything else falls through to the real fs. Used by watch and by +// the fd-opening patches. +function findVFSForExistingPath(filename) { const normalized = normalizeVFSPath(filename); for (let i = 0; i < activeVFSList.length; i++) { const vfs = activeVFSList[i]; @@ -1466,7 +1478,7 @@ function installFsPatches() { } else options ??= kEmptyObject; if (typeof filename === 'string') { - const vfsResult = findVFSForWatch(filename); + const vfsResult = findVFSForExistingPath(filename); if (vfsResult !== null) { return vfsResult.vfs.watch(filename, options, listener); } @@ -1482,7 +1494,7 @@ function installFsPatches() { } else options ??= kEmptyObject; if (typeof filename === 'string') { - const vfsResult = findVFSForWatch(filename); + const vfsResult = findVFSForExistingPath(filename); if (vfsResult !== null) { return vfsResult.vfs.watchFile(filename, options, listener); } @@ -1493,7 +1505,7 @@ function installFsPatches() { originalUnwatchFile = fs.unwatchFile; fs.unwatchFile = function unwatchFile(filename, listener) { if (typeof filename === 'string') { - const vfsResult = findVFSForWatch(filename); + const vfsResult = findVFSForExistingPath(filename); if (vfsResult !== null) { vfsResult.vfs.unwatchFile(filename, listener); return; @@ -1501,6 +1513,81 @@ function installFsPatches() { } return originalUnwatchFile.call(fs, filename, listener); }; + + // === File descriptor family === + // + // openSync/open resolve a path to a VFS; the rest route on the fd itself, + // which fd.js allocates from 10000+ so it can never shadow a real one. + + const originalOpenSync = fs.openSync; + fs.openSync = function openSync(path, flags, mode) { + if (typeof path === 'string') { + const vfsResult = findVFSForExistingPath(path); + if (vfsResult !== null) { + return vfsResult.vfs.openSync(path, flags ?? 'r', mode); + } + } + return originalOpenSync.call(fs, path, flags, mode); + }; + + const originalOpen = fs.open; + fs.open = function open(path, flags, mode, callback) { + if (typeof path === 'string') { + const vfsResult = findVFSForExistingPath(path); + if (vfsResult !== null) { + return vfsResult.vfs.open(path, flags, mode, callback); + } + } + return originalOpen.call(fs, path, flags, mode, callback); + }; + + const originalCloseSync = fs.closeSync; + fs.closeSync = function closeSync(fd) { + if (getVirtualFd(fd)) { + return closeFdSync(fd); + } + return originalCloseSync.call(fs, fd); + }; + + const originalClose = fs.close; + fs.close = function close(fd, callback) { + if (getVirtualFd(fd)) { + return closeFd(fd, callback); + } + return originalClose.call(fs, fd, callback); + }; + + const originalReadSync = fs.readSync; + fs.readSync = function readSync(fd, buffer, offset, length, position) { + if (getVirtualFd(fd)) { + return readFdSync(fd, buffer, offset, length, position); + } + return originalReadSync.call(fs, fd, buffer, offset, length, position); + }; + + const originalRead = fs.read; + fs.read = function read(fd, buffer, offset, length, position, callback) { + if (getVirtualFd(fd)) { + return readFd(fd, buffer, offset, length, position, callback); + } + return originalRead.call(fs, fd, buffer, offset, length, position, callback); + }; + + const originalFstatSync = fs.fstatSync; + fs.fstatSync = function fstatSync(fd, options) { + if (getVirtualFd(fd)) { + return fstatFdSync(fd, options); + } + return originalFstatSync.call(fs, fd, options); + }; + + const originalFstat = fs.fstat; + fs.fstat = function fstat(fd, options, callback) { + if (getVirtualFd(fd)) { + return fstatFd(fd, options, callback); + } + return originalFstat.call(fs, fd, options, callback); + }; } function installHooks() { diff --git a/test/fs-hooks.test.js b/test/fs-hooks.test.js index d50f7ef..d7bbd72 100644 --- a/test/fs-hooks.test.js +++ b/test/fs-hooks.test.js @@ -349,3 +349,158 @@ describe('Module hooks — fs.promises patches', () => { assert.strictEqual(content2, 'shared content'); }); }); + +describe('Module hooks — fd family patches', () => { + let vfs; + + afterEach(() => { + if (vfs?.mounted) { + vfs.unmount(); + } + }); + + it('fs.openSync + fs.readSync + fs.closeSync read a VFS file', () => { + vfs = create(); + vfs.writeFileSync('/fd.txt', 'hello from vfs'); + vfs.mount('/vfs-test-fd-sync'); + + const fd = fs.openSync('/vfs-test-fd-sync/fd.txt'); + const buffer = Buffer.alloc(5); + const bytesRead = fs.readSync(fd, buffer, 0, 5, 0); + fs.closeSync(fd); + + assert.strictEqual(bytesRead, 5); + assert.strictEqual(buffer.toString(), 'hello'); + }); + + it('fs.openSync throws ENOENT for a missing VFS file', () => { + vfs = create(); + vfs.writeFileSync('/present.txt', 'x'); + vfs.mount('/vfs-test-fd-missing'); + + assert.throws( + () => fs.openSync('/vfs-test-fd-missing/absent.txt'), + (err) => err.code === 'ENOENT', + ); + }); + + it('fs.readSync accepts the options-object overload', () => { + vfs = create(); + vfs.writeFileSync('/opts.txt', 'abcdefgh'); + vfs.mount('/vfs-test-fd-opts'); + + const fd = fs.openSync('/vfs-test-fd-opts/opts.txt'); + const buffer = Buffer.alloc(3); + const bytesRead = fs.readSync(fd, buffer, { offset: 0, length: 3, position: 2 }); + fs.closeSync(fd); + + assert.strictEqual(bytesRead, 3); + assert.strictEqual(buffer.toString(), 'cde'); + }); + + it('fs.fstatSync returns stats for a VFS fd', () => { + vfs = create(); + vfs.writeFileSync('/stat-fd.txt', 'data'); + vfs.mount('/vfs-test-fd-fstat'); + + const fd = fs.openSync('/vfs-test-fd-fstat/stat-fd.txt'); + const stats = fs.fstatSync(fd); + fs.closeSync(fd); + + assert.ok(stats.isFile()); + assert.strictEqual(stats.size, 4); + }); + + it('sequential fs.readSync calls advance the file position', () => { + vfs = create(); + vfs.writeFileSync('/seq.txt', 'abcdef'); + vfs.mount('/vfs-test-fd-seq'); + + const fd = fs.openSync('/vfs-test-fd-seq/seq.txt'); + const first = Buffer.alloc(3); + const second = Buffer.alloc(3); + fs.readSync(fd, first, 0, 3, null); + fs.readSync(fd, second, 0, 3, null); + fs.closeSync(fd); + + assert.strictEqual(first.toString(), 'abc'); + assert.strictEqual(second.toString(), 'def'); + }); + + it('fs.closeSync on a stale VFS fd throws EBADF', () => { + vfs = create(); + vfs.writeFileSync('/stale.txt', 'x'); + vfs.mount('/vfs-test-fd-stale'); + + const fd = fs.openSync('/vfs-test-fd-stale/stale.txt'); + fs.closeSync(fd); + + assert.throws(() => fs.closeSync(fd), (err) => err.code === 'EBADF'); + }); + + it('fs.open + fs.read + fs.close read a VFS file', (_t, done) => { + vfs = create(); + vfs.writeFileSync('/cb.txt', 'callback content'); + vfs.mount('/vfs-test-fd-cb'); + + fs.open('/vfs-test-fd-cb/cb.txt', 'r', (openErr, fd) => { + assert.ifError(openErr); + const buffer = Buffer.alloc(8); + fs.read(fd, buffer, 0, 8, 0, (readErr, bytesRead) => { + assert.ifError(readErr); + assert.strictEqual(bytesRead, 8); + assert.strictEqual(buffer.toString(), 'callback'); + fs.close(fd, (closeErr) => { + assert.ifError(closeErr); + done(); + }); + }); + }); + }); + + it('fs.fstat returns stats for a VFS fd', (_t, done) => { + vfs = create(); + vfs.writeFileSync('/fstat-cb.txt', 'seven..'); + vfs.mount('/vfs-test-fd-fstat-cb'); + + const fd = fs.openSync('/vfs-test-fd-fstat-cb/fstat-cb.txt'); + fs.fstat(fd, (err, stats) => { + assert.ifError(err); + assert.ok(stats.isFile()); + assert.strictEqual(stats.size, 7); + fs.closeSync(fd); + done(); + }); + }); + + it('real-fs descriptors still work while a VFS is mounted', () => { + vfs = create(); + vfs.writeFileSync('/unused.txt', 'x'); + vfs.mount('/vfs-test-fd-passthrough'); + + const fd = fs.openSync(__filename, 'r'); + const buffer = Buffer.alloc(12); + const bytesRead = fs.readSync(fd, buffer, 0, 12, 0); + const stats = fs.fstatSync(fd); + fs.closeSync(fd); + + assert.strictEqual(bytesRead, 12); + assert.strictEqual(buffer.toString(), "'use strict'"); + assert.ok(stats.size > 0); + }); + + it('an overlay mount leaves non-VFS paths on the real fs', () => { + vfs = create(); + vfs.writeFileSync('/only-here.txt', 'vfs'); + vfs.mount('/vfs-test-fd-overlay', { overlay: true }); + + const fd = fs.openSync('/vfs-test-fd-overlay/only-here.txt'); + assert.strictEqual(fs.fstatSync(fd).size, 3); + fs.closeSync(fd); + + assert.throws( + () => fs.openSync('/vfs-test-fd-overlay/not-here.txt'), + (err) => err.code === 'ENOENT', + ); + }); +});