vfs: align virtual file handles with open(2) - #65854
Open
pipobscure wants to merge 2 commits into
Open
Conversation
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 <pip@pipobscure.com>
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 <pip@pipobscure.com>
nodejs-github-bot
added
needs-ci
jasnell
approved these changes
Sep 6, 2026
pipobscure
marked this pull request as ready for review
September 6, 2026 14:24
github-actions
Bot
removed
the
request-ci
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
vfs: align virtual file handles with open(2)
A file descriptor obtained on a mounted path answers several
node:fscalls differently from one on a real file, in both providers:
writeFileSync(path, data, { flag: 'r+' })replaces the whole fileinstead of overwriting bytes from offset 0 and keeping the tail.
O_WRONLYalone truncates, andO_RDONLY | O_CREATopens the filewrite-only and truncates it.
file, so the first read returns nothing; O_APPEND only affects writes.
The ZipProvider handle additionally:
writing a read-only one;
ftruncategrows a file that waspreviously shrunk, where real files read back as zeros;
positionwith a TypeError from mixing number andBigInt 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
writeFilefor non-truncating flags write at offset 0 withoutshrinking; in the ZIP handle use EBADF for access-mode violations,
zero-fill on growth in
#doTruncate, and coercepositionwithNumber()as the memory handle does.Note: Since these are gaps/defects in existing functionality, I decided to create the failing tests first (first commit) and then add the fix/solution as a second commit. That way whoever wants to review this can first prove out the issue, before applying the solution.
This goes with the VFS work by @mcollina and the bug-fix PRs by @trivikr.