Skip to content

vfs: apply open(2) effects to ZipProvider handles - #65853

Open
pipobscure wants to merge 3 commits into
nodejs:mainfrom
pipobscure:vfs-zip-handle-commit
Open

vfs: apply open(2) effects to ZipProvider handles#65853
pipobscure wants to merge 3 commits into
nodejs:mainfrom
pipobscure:vfs-zip-handle-commit

Conversation

@pipobscure

@pipobscure pipobscure commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

vfs: apply open(2) effects to ZipProvider handles

A ZipProvider handle keeps its content in memory and adds the entry to
the archive when it is closed, and only if something was written. The
effects a real open(2) has at open time are therefore lost, and so is
the metadata the entry already carried:

  • open(path, 'w') followed by close() neither truncates an existing
    entry nor creates a missing one; the same holds for "a" on a missing
    file. Tools that touch or truncate by open-then-close do nothing.
  • Rewriting an entry (append, or an in-place write through "r+") re-adds
    it with the mode argument open() received (fs's default 0o666),
    not the mode the entry had, so a 0o755 script silently loses its
    executable bit.
  • fstat on a handle reports that same open() mode and the current
    time instead of the entry's mode and modification time.
  • Renaming a file onto an existing directory succeeds and leaves a name
    that is both a file and a directory; real file systems refuse with
    EISDIR.

This adds a test for each of these against a mounted ZipBuffer, stating
the real-fs outcome as the expectation.

Proposed solution: mark the handle dirty at open time when the flags
imply creation or truncation, so close always commits; carry the
existing entry's mode and modification time on the handle, use them
for fstat and for the re-added entry, and only fall back to the
open() mode for a newly created entry; and reject rename onto an
existing directory with EISDIR before touching the archive.

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.

A ZipProvider handle keeps its content in memory and adds the entry to
the archive when it is closed, and only if something was written. The
effects a real `open(2)` has at open time are therefore lost, and so is
the metadata the entry already carried:

* `open(path, 'w')` followed by `close()` neither truncates an existing
  entry nor creates a missing one; the same holds for "a" on a missing
  file. Tools that touch or truncate by open-then-close do nothing.
* Rewriting an entry (append, or an in-place write through "r+") re-adds
  it with the `mode` argument `open()` received (fs's default 0o666),
  not the mode the entry had, so a 0o755 script silently loses its
  executable bit.
* `fstat` on a handle reports that same `open()` mode and the current
  time instead of the entry's mode and modification time.
* Renaming a file onto an existing directory succeeds and leaves a name
  that is both a file and a directory; real file systems refuse with
  EISDIR.

This adds a test for each of these against a mounted ZipBuffer, stating
the real-fs outcome as the expectation.

Proposed solution: mark the handle dirty at open time when the flags
imply creation or truncation, so close always commits; carry the
existing entry's mode and modification time on the handle, use them
for `fstat` and for the re-added entry, and only fall back to the
`open()` mode for a newly created entry; and reject `rename` onto an
existing directory with EISDIR before touching the archive.

Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
A ZipProvider handle adds its entry to the archive when it is closed.
Give that model the effects a real open(2) has up front, and keep the
metadata an entry already carries:

* A handle whose flags create or truncate the file starts out dirty,
  so closing it without a write still creates the missing entry or
  truncates the existing one.
* The handle remembers the entry's own mode and modification time.
  The re-added entry keeps that mode instead of taking the `mode`
  argument `open()` was given (fs's default 0o666), so a 0o755 script
  survives an append or an in-place write; only a newly created file
  takes the mode from `open()`.
* `fstat` reports that mode and, until the handle has changed the
  file, that modification time.
* `rename` refuses to move a file onto an existing directory with
  EISDIR before touching the archive.

Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem. labels Sep 6, 2026
@pipobscure pipobscure changed the title Vfs zip handle commit vfs: apply open(2) effects to ZipProvider handles Sep 6, 2026
Comment thread lib/internal/vfs/providers/ziparchive.js Outdated
The options passed to `createFileStats()` and to the archive's `add()`
and `addSync()` are plain literals, so a property added to
`Object.prototype` would reach those callees as if it had been passed
on purpose. Create them with a null prototype so only the fields set
here are visible.

Refs: nodejs#65853
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
@pipobscure
pipobscure marked this pull request as ready for review September 6, 2026 14:33
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. vfs Issues and PRs related to the virtual filesystem subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants