From cb3e193bdb8a39cdc4585b70814a73ac6dc93043 Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 11:34:08 +0200 Subject: [PATCH 1/2] vfs: align virtual file handles with open(2) A file descriptor obtained on a mounted path answers several `node:fs` calls differently from one on a real file, in both providers: * `writeFileSync(path, data, { flag: 'r+' })` replaces the whole file instead of overwriting bytes from offset 0 and keeping the tail. * Numeric open flags are mapped by treating any write-ish bit as "w": `O_WRONLY` alone truncates, and `O_RDONLY | O_CREAT` opens the file write-only and truncates it. * A handle opened with "a+" starts its read offset at the end of the file, so the first read returns nothing; O_APPEND only affects writes. The ZipProvider handle additionally: * throws EISDIR instead of EBADF when reading a write-only handle or writing a read-only one; * leaves stale bytes in place when `ftruncate` grows a file that was previously shrunk, where real files read back as zeros; * rejects a BigInt `position` with a TypeError from mixing number and BigInt arithmetic. This adds a test that runs the same sequence of calls against a memory mount and a ZIP mount and expects the real-fs result, so every divergence shows up as its own failing case. Proposed solution: decode numeric flags bit by bit (O_TRUNC decides truncation, O_CREAT decides creation, O_WRONLY/O_RDWR decide access) instead of collapsing them to a flag string; keep the read offset at 0 for append handles and only force writes to the end; make the handle `writeFile` for non-truncating flags write at offset 0 without shrinking; in the ZIP handle use EBADF for access-mode violations, zero-fill on growth in `#doTruncate`, and coerce `position` with `Number()` as the memory handle does. Signed-off-by: Philipp Dunkel --- test/parallel/test-vfs-handle-semantics.js | 115 +++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 test/parallel/test-vfs-handle-semantics.js diff --git a/test/parallel/test-vfs-handle-semantics.js b/test/parallel/test-vfs-handle-semantics.js new file mode 100644 index 00000000000..3cd38fb7f83 --- /dev/null +++ b/test/parallel/test-vfs-handle-semantics.js @@ -0,0 +1,115 @@ +// Flags: --experimental-vfs +'use strict'; + +// A file handle on a mounted path must answer the same `node:fs` calls the +// way a descriptor on a real file does. These cases run against both the +// memory provider and the ZipProvider, and state the real-fs outcome as the +// expectation. Cases are independent so the runner reports each one. + +require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const zlib = require('zlib'); +const vfs = require('node:vfs'); +const { test } = require('node:test'); + +const { O_WRONLY, O_RDONLY, O_CREAT } = fs.constants; + +// Each provider is described by a function that mounts a fresh layer holding +// one file `f` with the given content and returns that file's path. +const providers = { + memory(content) { + const layer = vfs.create(); + layer.writeFileSync('/f', content); + return path.join(layer.mount(), 'f'); + }, + zip(content) { + const entry = zlib.ZipEntry.createSync('f', Buffer.from(content)); + const chunks = []; + for (const chunk of zlib.createZipArchiveSync([entry])) chunks.push(chunk); + const provider = new vfs.ZipProvider(new zlib.ZipBuffer(Buffer.concat(chunks))); + return path.join(vfs.create(provider).mount(), 'f'); + }, +}; + +for (const { 0: name, 1: fileWith } of Object.entries(providers)) { + test(`${name}: reading a write-only handle fails with EBADF`, () => { + const fd = fs.openSync(fileWith('x'), 'w'); + try { + assert.throws(() => fs.readSync(fd, Buffer.alloc(4), 0, 4, 0), { code: 'EBADF' }); + } finally { + fs.closeSync(fd); + } + }); + + test(`${name}: writing a read-only handle fails with EBADF`, () => { + const fd = fs.openSync(fileWith('x'), 'r'); + try { + assert.throws(() => fs.writeSync(fd, Buffer.from('y')), { code: 'EBADF' }); + } finally { + fs.closeSync(fd); + } + }); + + test(`${name}: extending a file with ftruncate zero-fills the new region`, () => { + const file = fileWith('hello world'); + const fd = fs.openSync(file, 'r+'); + fs.ftruncateSync(fd, 5); + fs.ftruncateSync(fd, 11); + fs.closeSync(fd); + // Shrinking then growing must not resurrect the bytes that were cut off. + assert.strictEqual(fs.readFileSync(file, 'latin1'), 'hello\0\0\0\0\0\0'); + }); + + test(`${name}: readSync accepts a BigInt position`, () => { + const fd = fs.openSync(fileWith('hello'), 'r'); + try { + const buf = Buffer.alloc(4); + const n = fs.readSync(fd, buf, 0, 4, 1n); + assert.strictEqual(buf.toString('utf8', 0, n), 'ello'); + } finally { + fs.closeSync(fd); + } + }); + + test(`${name}: numeric O_WRONLY does not truncate`, () => { + const file = fileWith('hello'); + const fd = fs.openSync(file, O_WRONLY); + fs.writeSync(fd, Buffer.from('J'), 0, 1, 0); + fs.closeSync(fd); + // Only O_TRUNC truncates. + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'Jello'); + }); + + test(`${name}: numeric O_RDONLY | O_CREAT opens an existing file readable and intact`, () => { + const file = fileWith('hello'); + const fd = fs.openSync(file, O_RDONLY | O_CREAT); + try { + const buf = Buffer.alloc(5); + const n = fs.readSync(fd, buf, 0, 5, 0); + assert.strictEqual(buf.toString('utf8', 0, n), 'hello'); + } finally { + fs.closeSync(fd); + } + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'hello'); + }); + + test(`${name}: an "a+" handle reads from the start of the file`, () => { + const fd = fs.openSync(fileWith('abc'), 'a+'); + try { + const buf = Buffer.alloc(3); + // O_APPEND only moves writes to the end; the read offset starts at 0. + const n = fs.readSync(fd, buf, 0, 3, null); + assert.strictEqual(buf.toString('utf8', 0, n), 'abc'); + } finally { + fs.closeSync(fd); + } + }); + + test(`${name}: writeFileSync with flag "r+" overwrites in place without truncating`, () => { + const file = fileWith('hello world'); + fs.writeFileSync(file, 'HEY', { flag: 'r+' }); + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'HEYlo world'); + }); +} From 6c91c334f9d583a9d67dce7221c7a1b1c78be95b Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 13:10:22 +0200 Subject: [PATCH 2/2] vfs: derive handle behaviour from open flag bits Decode open flags once, in `VirtualFileHandle`, into what they ask for (readable, writable, create, exclusive, truncate, append) and let both providers and both handle classes act on those bits instead of on a flag string. Numeric `fs.constants` combinations that have no string spelling keep their meaning: a plain `O_WRONLY` neither creates nor truncates, and `O_RDONLY | O_CREAT` opens an existing file readable and intact. `handle.flags` stays a string, now purely descriptive. On top of that, in both handles: * An append handle no longer starts its read offset at the end of the file; O_APPEND only forces writes there. * `writeFile` writes from the current position, like `filehandle.writeFile()`, so "r+" overwrites in place and keeps any tail while "w" has already truncated. And in the ZipProvider handle: * Access-mode violations are EBADF rather than EISDIR. * `ftruncate` zero-fills the region it grows into instead of exposing bytes cut off by an earlier shrink. * A BigInt `position` is accepted. * `readSync` and `writeSync` return the byte count, as `fs.readSync`, `fs.writeSync` and the memory handle do, instead of the promise-shaped `{ bytesRead }` object. The existing ZipProvider handle test asserted the old EISDIR code, the object-shaped `readSync` result and the end-of-file read offset for append handles; it now asserts the corrected behaviour. Signed-off-by: Philipp Dunkel --- lib/internal/vfs/file_handle.js | 107 ++++++++++-------- lib/internal/vfs/providers/memory.js | 59 ++-------- lib/internal/vfs/providers/ziparchive.js | 106 ++++++++--------- test/parallel/test-vfs-zip-provider-handle.js | 14 ++- 4 files changed, 125 insertions(+), 161 deletions(-) diff --git a/lib/internal/vfs/file_handle.js b/lib/internal/vfs/file_handle.js index 7b60c9def2b..a65fe5f9938 100644 --- a/lib/internal/vfs/file_handle.js +++ b/lib/internal/vfs/file_handle.js @@ -27,6 +27,52 @@ const kFlags = Symbol('kFlags'); const kMode = Symbol('kMode'); const kPosition = Symbol('kPosition'); const kClosed = Symbol('kClosed'); +const kAccess = Symbol('kAccess'); + +const { stringToFlags } = require('internal/fs/utils'); +const { + fs: { O_APPEND, O_CREAT, O_EXCL, O_RDONLY, O_RDWR, O_TRUNC, O_WRONLY }, +} = internalBinding('constants'); + +/** + * Decodes open flags into what they ask for. Both the string spellings and + * the numeric `fs.constants` values are accepted. The bits decide, because + * several numeric combinations (`O_WRONLY` alone, `O_RDONLY | O_CREAT`) + * have no string spelling, and collapsing them to the nearest one changes + * their meaning: a plain `O_WRONLY` must neither create nor truncate. + * @param {string|number} flags + * @returns {{ readable: boolean, writable: boolean, create: boolean, + * exclusive: boolean, truncate: boolean, append: boolean }} + */ +function decodeOpenFlags(flags) { + const bits = typeof flags === 'number' ? flags : stringToFlags(flags); + const access = bits & (O_RDONLY | O_WRONLY | O_RDWR); + return { + __proto__: null, + readable: access !== O_WRONLY, + writable: access !== O_RDONLY, + create: (bits & O_CREAT) !== 0, + exclusive: (bits & O_EXCL) !== 0, + truncate: (bits & O_TRUNC) !== 0, + append: (bits & O_APPEND) !== 0, + }; +} + +/** + * The string spelling closest to numeric flags, for `handle.flags`, which + * has always been a string. Behaviour is never derived from it. + * @param {string|number} flags + * @returns {string} + */ +function flagsToString(flags) { + if (typeof flags !== 'number') return flags; + const { readable, writable, exclusive, truncate, append } = decodeOpenFlags(flags); + const plus = readable && writable ? '+' : ''; + const x = exclusive ? 'x' : ''; + if (append) return `a${x}${plus}`; + if (truncate) return `w${x}${plus}`; + return writable ? 'r+' : 'r'; +} function isCurrentPosition(position) { return position === null || position === undefined || position === -1; @@ -44,7 +90,8 @@ class VirtualFileHandle { */ constructor(path, flags, mode) { this[kPath] = path; - this[kFlags] = flags; + this[kAccess] = decodeOpenFlags(flags); + this[kFlags] = flagsToString(flags); this[kMode] = mode ?? 0o644; this[kPosition] = 0; this[kClosed] = false; @@ -387,19 +434,15 @@ class MemoryFileHandle extends VirtualFileHandle { this.#entry = entry; this.#getStats = getStats; - // Handle different open modes - if (flags === 'w' || flags === 'w+' || - flags === 'wx' || flags === 'wx+') { - // Write mode: truncate + // O_TRUNC empties the file at open time. O_APPEND does not move the + // read offset: it only forces writes to the end, so the position stays + // at 0 and reads start from the beginning as they do on a real file. + if (this[kAccess].truncate) { this.#content = Buffer.alloc(0); this.#size = 0; if (entry) { entry.content = this.#content; } - } else if (flags === 'a' || flags === 'a+' || - flags === 'ax' || flags === 'ax+') { - // Append mode: position at end - this.position = this.#size; } } @@ -407,7 +450,7 @@ class MemoryFileHandle extends VirtualFileHandle { * Throws EBADF if the handle was not opened for writing. */ #checkWritable() { - if (this.flags === 'r') { + if (!this[kAccess].writable) { throw createEBADF('write'); } } @@ -416,8 +459,7 @@ class MemoryFileHandle extends VirtualFileHandle { * Throws EBADF if the handle was not opened for reading. */ #checkReadable() { - const f = this.flags; - if (f === 'w' || f === 'a' || f === 'wx' || f === 'ax') { + if (!this[kAccess].readable) { throw createEBADF('read'); } } @@ -427,8 +469,7 @@ class MemoryFileHandle extends VirtualFileHandle { * @returns {boolean} */ #isAppend() { - const f = this.flags; - return f === 'a' || f === 'a+' || f === 'ax' || f === 'ax+'; + return this[kAccess].append; } /** @@ -605,42 +646,16 @@ class MemoryFileHandle extends VirtualFileHandle { } /** - * Writes data to the file synchronously. - * Replaces content in 'w' mode, appends in 'a' mode. + * Writes data to the file synchronously, from the current position (the + * end, in append mode), the way `filehandle.writeFile()` does. Whether + * earlier content is discarded was decided by the open flags: "w" has + * already truncated, "r+" overwrites in place and keeps any tail. * @param {Buffer|string} data The data to write * @param {object} [options] Options */ writeFileSync(data, options) { - this.#checkClosed('write'); - this.#checkWritable(); - const buffer = typeof data === 'string' ? Buffer.from(data, options?.encoding) : data; - - // In append mode, append to existing content - if (this.#isAppend()) { - const neededSize = this.#size + buffer.length; - if (neededSize > this.#content.length) { - const newCapacity = MathMax(neededSize, this.#content.length * 2); - const newContent = Buffer.alloc(newCapacity); - this.#content.copy(newContent, 0, 0, this.#size); - this.#content = newContent; - } - buffer.copy(this.#content, this.#size); - this.#size = neededSize; - } else { - this.#content = Buffer.from(buffer); - this.#size = buffer.length; - } - - // Update the entry's content, mtime, and ctime - if (this.#entry) { - const now = DateNow(); - this.#entry.content = this.#content.subarray(0, this.#size); - this.#entry.mtime = now; - this.#entry.ctime = now; - } - - this.position = this.#size; + this.writeSync(buffer, 0, buffer.length, null); } /** @@ -721,4 +736,6 @@ class MemoryFileHandle extends VirtualFileHandle { module.exports = { VirtualFileHandle, MemoryFileHandle, + decodeOpenFlags, + kAccess, }; diff --git a/lib/internal/vfs/providers/memory.js b/lib/internal/vfs/providers/memory.js index ce59a061161..2fd9a7d78ca 100644 --- a/lib/internal/vfs/providers/memory.js +++ b/lib/internal/vfs/providers/memory.js @@ -16,7 +16,7 @@ const { Buffer } = require('buffer'); const { isPromise } = require('util/types'); const { posix: pathPosix } = require('path'); const { VirtualProvider } = require('internal/vfs/provider'); -const { MemoryFileHandle } = require('internal/vfs/file_handle'); +const { MemoryFileHandle, decodeOpenFlags } = require('internal/vfs/file_handle'); const { VFSWatcher, VFSStatWatcher, @@ -46,45 +46,12 @@ const { Dirent } = require('internal/fs/utils'); const { kEmptyObject } = require('internal/util'); const { fs: { - O_APPEND, - O_CREAT, - O_EXCL, - O_RDWR, - O_TRUNC, - O_WRONLY, UV_DIRENT_FILE, UV_DIRENT_DIR, UV_DIRENT_LINK, }, } = internalBinding('constants'); -/** - * Converts numeric flags to a string representation. - * If already a string, returns as-is. - * @param {string|number} flags The flags to normalize - * @returns {string} Normalized string flags - */ -function normalizeFlags(flags) { - if (typeof flags === 'string') return flags; - if (typeof flags !== 'number') return 'r'; - - const rdwr = (flags & O_RDWR) !== 0; - const append = (flags & O_APPEND) !== 0; - const excl = (flags & O_EXCL) !== 0; - const write = (flags & O_WRONLY) !== 0 || - (flags & O_CREAT) !== 0 || - (flags & O_TRUNC) !== 0; - - if (append) { - return 'a' + (excl ? 'x' : '') + (rdwr ? '+' : ''); - } - if (write) { - return 'w' + (excl ? 'x' : '') + (rdwr ? '+' : ''); - } - if (rdwr) return 'r+'; - return 'r'; -} - /** * Converts a time argument (Date, number, or string) to milliseconds. * Numbers are treated as seconds (matching Node.js utimes convention). @@ -496,33 +463,23 @@ class MemoryProvider extends VirtualProvider { openSync(path, flags, mode) { const normalized = this.#normalizePath(path); + const access = decodeOpenFlags(flags); - // Normalize numeric flags to string - flags = normalizeFlags(flags); - - // Handle create and exclusive modes - const isCreate = flags === 'w' || flags === 'w+' || - flags === 'a' || flags === 'a+' || - flags === 'wx' || flags === 'wx+' || - flags === 'ax' || flags === 'ax+'; - const isExclusive = flags === 'wx' || flags === 'wx+' || - flags === 'ax' || flags === 'ax+'; - const isWritable = flags !== 'r'; - - // Check readonly for any writable mode - if (this.readonly && isWritable) { + // Creating a file is a write even when the handle itself is read-only. + if (this.readonly && (access.writable || access.create)) { throw createEROFS('open', path); } let entry; try { entry = this.#getEntry(normalized, 'open'); - // Exclusive flag: file must not exist - if (isExclusive) { + // O_EXCL only means anything together with O_CREAT: the file must + // not exist yet. + if (access.create && access.exclusive) { throw createEEXIST('open', path); } } catch (err) { - if (err.code !== 'ENOENT' || !isCreate) throw err; + if (err.code !== 'ENOENT' || !access.create) throw err; // Create the file const parent = this.#ensureParent(normalized, false, 'open'); const name = pathPosix.basename(normalized); diff --git a/lib/internal/vfs/providers/ziparchive.js b/lib/internal/vfs/providers/ziparchive.js index f369cdd22f7..402c92b8b65 100644 --- a/lib/internal/vfs/providers/ziparchive.js +++ b/lib/internal/vfs/providers/ziparchive.js @@ -5,6 +5,7 @@ const { ArrayPrototypePush, MathMax, MathMin, + Number, StringPrototypeIndexOf, StringPrototypeSlice, StringPrototypeStartsWith, @@ -20,8 +21,13 @@ const { }, } = require('internal/errors'); const { VirtualProvider } = require('internal/vfs/provider'); -const { VirtualFileHandle } = require('internal/vfs/file_handle'); const { + VirtualFileHandle, + decodeOpenFlags, + kAccess, +} = require('internal/vfs/file_handle'); +const { + createEBADF, createEEXIST, createEISDIR, createENOENT, @@ -33,12 +39,6 @@ const { createFileStats, createDirectoryStats } = require('internal/vfs/stats'); const { Dirent } = require('internal/fs/utils'); const { fs: { - O_APPEND, - O_CREAT, - O_EXCL, - O_RDWR, - O_TRUNC, - O_WRONLY, UV_DIRENT_DIR, UV_DIRENT_FILE, }, @@ -51,41 +51,30 @@ function normalize(vfsPath) { return StringPrototypeStartsWith(vfsPath, '/') ? StringPrototypeSlice(vfsPath, 1) : vfsPath; } -// Converts numeric open flags (e.g. `fs.constants.O_RDWR`) to the flag strings -// the helpers below understand, so a caller passing `node:fs`-style numeric -// flags through the VFS is handled the same way `fs` would. Strings pass -// through unchanged; anything else falls back to 'r'. -function normalizeFlags(flags) { - if (typeof flags === 'string') return flags; - if (typeof flags !== 'number') return 'r'; - const rdwr = (flags & O_RDWR) !== 0; - const append = (flags & O_APPEND) !== 0; - const excl = (flags & O_EXCL) !== 0; - const write = (flags & O_WRONLY) !== 0 || (flags & O_CREAT) !== 0 || (flags & O_TRUNC) !== 0; - if (append) return 'a' + (excl ? 'x' : '') + (rdwr ? '+' : ''); - if (write) return 'w' + (excl ? 'x' : '') + (rdwr ? '+' : ''); - if (rdwr) return 'r+'; - return 'r'; -} - +// The open flags decide by their bits, not by a string spelling: numeric +// `fs.constants` combinations such as `O_WRONLY` alone or `O_RDONLY | O_CREAT` +// have no string form, so `decodeOpenFlags` is the one place that interprets +// them and these helpers are thin readers over it. They accept both strings +// and numbers so call sites need not care which they were given. function isCurrentPosition(position) { return position === null || position === undefined || position === -1; } function isWriteTruncate(flags) { - return flags === 'w' || flags === 'w+' || flags === 'wx' || flags === 'wx+'; + return decodeOpenFlags(flags).truncate; } -function isAppend(flags) { - return flags === 'a' || flags === 'a+' || flags === 'ax' || flags === 'ax+'; +function isExclusive(flags) { + const access = decodeOpenFlags(flags); + return access.create && access.exclusive; } -function isReadableFlag(flags) { - return flags !== 'w' && flags !== 'a' && flags !== 'wx' && flags !== 'ax'; +function mustExist(flags) { + return !decodeOpenFlags(flags).create; } function isWritableFlag(flags) { - return flags !== 'r'; + return decodeOpenFlags(flags).writable; } /** @@ -131,14 +120,15 @@ class ZipFileHandle extends VirtualFileHandle { this.#name = name; this.#buffer = initial; this.#size = initial.length; - if (isAppend(flags)) this.position = this.#size; } + // Access-mode violations are EBADF, as for any descriptor opened without + // the needed access; EISDIR is for directories only. #checkReadable() { - if (!isReadableFlag(this.flags)) throw createEISDIR('read', this.path); + if (!this[kAccess].readable) throw createEBADF('read'); } #checkWritable() { - if (!isWritableFlag(this.flags)) throw createEISDIR('write', this.path); + if (!this[kAccess].writable) throw createEBADF('write'); } #ensureCapacity(size) { if (size <= this.#buffer.length) return; @@ -151,7 +141,9 @@ class ZipFileHandle extends VirtualFileHandle { #doRead(buffer, offset, length, position) { this.#checkReadable(); const useCurrent = isCurrentPosition(position); - const pos = useCurrent ? this.position : position; + // `position` may be a BigInt, which fs allows; the arithmetic below is + // on numbers. + const pos = useCurrent ? this.position : Number(position); const available = MathMax(0, this.#size - pos); const bytesRead = MathMin(length, available); if (bytesRead > 0) this.#buffer.copy(buffer, offset, pos, pos + bytesRead); @@ -161,14 +153,16 @@ class ZipFileHandle extends VirtualFileHandle { async read(buffer, offset, length, position) { return this.#doRead(buffer, offset, length, position); } + // The synchronous form reports the count alone, like `fs.readSync` and the + // memory handle; the `{ bytesRead, buffer }` shape belongs to the promise. readSync(buffer, offset, length, position) { - return this.#doRead(buffer, offset, length, position); + return this.#doRead(buffer, offset, length, position).bytesRead; } #doWrite(buffer, offset, length, position) { this.#checkWritable(); const useCurrent = isCurrentPosition(position); - const pos = isAppend(this.flags) ? this.#size : (useCurrent ? this.position : position); + const pos = this[kAccess].append ? this.#size : (useCurrent ? this.position : Number(position)); this.#ensureCapacity(pos + length); buffer.copy(this.#buffer, pos, offset, offset + length); if (pos + length > this.#size) this.#size = pos + length; @@ -180,7 +174,7 @@ class ZipFileHandle extends VirtualFileHandle { return this.#doWrite(buffer, offset, length, position); } writeSync(buffer, offset, length, position) { - return this.#doWrite(buffer, offset, length, position); + return this.#doWrite(buffer, offset, length, position).bytesWritten; } #doReadFile(options) { @@ -196,22 +190,14 @@ class ZipFileHandle extends VirtualFileHandle { return this.#doReadFile(options); } - // Replaces content, except in append mode ('a'/'a+'/'ax'/'ax+'), where it - // appends to the existing content instead - matching MemoryFileHandle and - // what makes `appendFile()`/`appendFileSync()` (built on this, by - // VirtualProvider's defaults) actually append. + // Writes from the current position (the end, in append mode), the way + // `filehandle.writeFile()` does. Whether earlier content is discarded was + // decided by the open flags: "w" has already truncated, "r+" overwrites in + // place and keeps any tail. This is what makes `appendFile()` (built on + // this by VirtualProvider's defaults) actually append. #doWriteFile(data, options) { - this.#checkWritable(); const content = typeof data === 'string' ? Buffer.from(data, options?.encoding) : Buffer.from(data); - if (isAppend(this.flags)) { - this.#ensureCapacity(this.#size + content.length); - content.copy(this.#buffer, this.#size); - this.#size += content.length; - } else { - this.#buffer = content; - this.#size = content.length; - } - this.#dirty = true; + this.#doWrite(content, 0, content.length, null); } async writeFile(data, options) { this.#doWriteFile(data, options); @@ -233,6 +219,10 @@ class ZipFileHandle extends VirtualFileHandle { #doTruncate(len) { this.#checkWritable(); this.#ensureCapacity(len); + // Growing exposes bytes past the old size. A fresh buffer is zeroed, but + // one that was shrunk earlier still holds the cut-off content, which a + // real file never hands back. + if (len > this.#size) this.#buffer.fill(0, this.#size, len); this.#size = len; this.#dirty = true; } @@ -330,20 +320,19 @@ class ZipProvider extends VirtualProvider { } async open(path, flags, mode) { - flags = normalizeFlags(flags); const name = normalize(path); const fileEntry = await this.#getEntry(name); if (fileEntry === null && this.#isDirectory(name)) { throw createEISDIR('open', path); } const exists = fileEntry !== null; - if (isWritableFlag(flags) && this.readonly) { + if ((isWritableFlag(flags) || !mustExist(flags)) && this.readonly) { throw createEROFS('open', path); } - if ((flags === 'wx' || flags === 'wx+' || flags === 'ax' || flags === 'ax+') && exists) { + if (isExclusive(flags) && exists) { throw createEEXIST('open', path); } - if (!exists && (flags === 'r' || flags === 'r+')) { + if (!exists && mustExist(flags)) { throw createENOENT('open', path); } let initial = EMPTY_BUFFER; @@ -353,20 +342,19 @@ class ZipProvider extends VirtualProvider { return new ZipFileHandle(path, flags, mode, this.#source, name, initial); } openSync(path, flags, mode) { - flags = normalizeFlags(flags); const name = normalize(path); const fileEntry = this.#getEntrySync(name); if (fileEntry === null && this.#isDirectory(name)) { throw createEISDIR('open', path); } const exists = fileEntry !== null; - if (isWritableFlag(flags) && this.readonly) { + if ((isWritableFlag(flags) || !mustExist(flags)) && this.readonly) { throw createEROFS('open', path); } - if ((flags === 'wx' || flags === 'wx+' || flags === 'ax' || flags === 'ax+') && exists) { + if (isExclusive(flags) && exists) { throw createEEXIST('open', path); } - if (!exists && (flags === 'r' || flags === 'r+')) { + if (!exists && mustExist(flags)) { throw createENOENT('open', path); } let initial = EMPTY_BUFFER; diff --git a/test/parallel/test-vfs-zip-provider-handle.js b/test/parallel/test-vfs-zip-provider-handle.js index 3c88eacbb8c..4263864e241 100644 --- a/test/parallel/test-vfs-zip-provider-handle.js +++ b/test/parallel/test-vfs-zip-provider-handle.js @@ -98,7 +98,7 @@ function buildArchiveSync(entries, comment) { const handle = provider.openSync('/b.txt', 'r+'); const buf = Buffer.alloc(4); - const { bytesRead } = handle.readSync(buf, 0, 4, 2); + const bytesRead = handle.readSync(buf, 0, 4, 2); assert.strictEqual(bytesRead, 4); assert.strictEqual(buf.toString(), '2345'); @@ -124,7 +124,8 @@ function buildArchiveSync(entries, comment) { const provider = new vfs.ZipProvider(zip); const handle = await provider.open('/c.txt', 'a'); - assert.strictEqual(handle.position, 2); // Positioned at EOF on open + // O_APPEND only forces writes to the end; the read offset starts at 0. + assert.strictEqual(handle.position, 0); // Even with an explicit (wrong) position, append mode writes at the end. await handle.write(Buffer.from('z'), 0, 1, 0); @@ -259,7 +260,7 @@ function buildArchiveSync(entries, comment) { assert.throws(() => provider.rmdirSync('/file.txt'), { code: 'ENOTDIR' }); })().then(common.mustCall()); -// --- open(): EEXIST/ENOENT/EISDIR-on-wrong-direction, called directly on +// --- open(): EEXIST/ENOENT/EBADF-on-wrong-direction, called directly on // the provider so the router can't short-circuit before delegating --------- (async () => { const archive = await buildArchive([await zlib.ZipEntry.create('a.txt', Buffer.from('x'))]); @@ -271,12 +272,13 @@ function buildArchiveSync(entries, comment) { await assert.rejects(provider.open('/missing.txt', 'r'), { code: 'ENOENT' }); assert.throws(() => provider.openSync('/missing.txt', 'r'), { code: 'ENOENT' }); - // A handle opened write-only can't be read from, and vice versa. + // A handle opened write-only can't be read from, and vice versa: EBADF, + // as for any descriptor without the needed access. const writeOnly = await provider.open('/w.txt', 'w'); - await assert.rejects(writeOnly.read(Buffer.alloc(1), 0, 1, 0), { code: 'EISDIR' }); + await assert.rejects(writeOnly.read(Buffer.alloc(1), 0, 1, 0), { code: 'EBADF' }); await writeOnly.close(); const readOnly = await provider.open('/a.txt', 'r'); - await assert.rejects(readOnly.write(Buffer.alloc(1), 0, 1, 0), { code: 'EISDIR' }); + await assert.rejects(readOnly.write(Buffer.alloc(1), 0, 1, 0), { code: 'EBADF' }); await readOnly.close(); })().then(common.mustCall());