Skip to content

fix(runtime): extend fs::validate to fsync/fdatasync/fchown/lchown/lchmod/copyFile/write/writev (#2013)#2181

Merged
proggeramlug merged 1 commit into
mainfrom
fix-2013-fs-arg-validation
May 28, 2026
Merged

fix(runtime): extend fs::validate to fsync/fdatasync/fchown/lchown/lchmod/copyFile/write/writev (#2013)#2181
proggeramlug merged 1 commit into
mainfrom
fix-2013-fs-arg-validation

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

Extends the fs::validate helpers from #2035 to the rest-of-fs slice
of #2013 — the 27 assert.throws-style tests the issue update flagged
as still falling through:

API Validation
fsyncSync / fdatasyncSync fd type / range
fchownSync fd, uid, gid (TypeError + RangeError per Node)
lchownSync path-type, uid, gid
lchmodSync path-type (mode validation deferred — see ordering note)
copyFileSync (+ async) src/dest path-type, mode (TypeError on string, RangeError on out-of-range int)
writeSync family fd
writevSync fd, only when the buffers array is non-empty
exists (callback) callback-must-be-function
lchown / lchmod async same as their sync siblings + callback

New validators in fs::validate

  • validate_fd(value) — TypeError for non-number, RangeError for NaN/Infinity/non-integer/out-of-range. Matches Node's validateInt32(fd, 'fd', 0).
  • validate_int32(value, name, min, max) — reused for uid / gid / mode where Node validates a bounded integer.
  • validate_function(name, value) — TypeError if the value isn't a Perry closure (via the existing extract_closure_ptr helper). Mirrors Node's validateFunction.

Range errors are surfaced through js_rangeerror_new and registered in the per-error side table from #2035 so err.code === 'ERR_OUT_OF_RANGE' round-trips correctly.

Deliberate scope limits

  • lchmodSync mode range: Node opens the path first, so a bad mode on a non-existent path surfaces ENOENT before mode validation fires. Validating mode here in Perry would diverge from Node's ordering. The mode-on-existing-path validation is a separate follow-up paired with proper ENOENT propagation.
  • writevSync empty buffers: Node skips fd validation when the buffers array is empty (returns 0 without touching the fd). Perry now gates fd validation on array.length > 0 to match.
  • existsSync is unchanged — it never throws on bad input under Node and Perry already returns false via decode_path_value.
  • Callback-required cases for fsync/fdatasync/fchown/copyFile aren't validated at the top of the callback wrapper — those already dispatch into the sync impl which now validates fd, so an assert.throws(() => fs.fsync(input)) with bad input throws regardless. Required-callback-on-valid-input is left for the next slice.

Verification

  • Added test-parity/node-suite/fs/sync/arg-validation.ts — byte-identical to node --experimental-strip-types across 27 cases covering fd type/range, path-type, uid/gid type/range, copyFile src/dest/mode, write/writev fd, and the writev empty-array no-throw contract.
  • No regressions in the rest of node-suite/fs (filehandle path still uses real fds; lchmodSync mode pass-through unchanged).
  • cargo fmt --all -- --check clean.

Follow-ups (still in #2013)

  • lchmodSync / lchownSync mode + range validation on existing paths (paired with ENOENT propagation).
  • Required-callback validation on fsync/fdatasync/fchown async wrappers.
  • buffer (10), net (10), zlib (10), http (9), process (8), crypto (7), url (2), timers (1) — same pattern, applied per-crate.

…hmod/copyFile/write/writev (#2013)

Extends the #2035 validator surface to the rest-of-fs slice the #2013
update flagged as still falling through. Same Node-compatible error
shape (TypeError [ERR_INVALID_ARG_TYPE] / RangeError [ERR_OUT_OF_RANGE]
with the per-error side table from #2035 carrying `.code`).

New validators in `fs::validate`:

- `validate_fd(value)` — Node-compatible int32 fd check (`>= 0 && <= 2^31-1`).
- `validate_int32(value, name, min, max)` — reused for uid / gid / mode.
- `validate_function(name, value)` — TypeError if the value isn't a Perry
  closure, via `extract_closure_ptr`.

Wired in:

- `fsyncSync` / `fdatasyncSync` / `writeSync*` / `writevSync` — fd.
  `writevSync` skips fd validation when the buffers array is empty
  (matches Node: empty writev returns 0 without touching the fd).
- `fchownSync` — fd, uid, gid.
- `lchownSync` — path, uid, gid.
- `lchmodSync` — path only (mode range deferred; Node opens the path
  first so `ENOENT` wins on non-existent paths).
- `copyFileSync` / async — src, dest, mode (range-checked when supplied).
- `exists` callback — `cb` must be a function (path bad-type is silent in Node).
- `lchown` / `lchmod` async — same as their sync siblings + callback.

To keep the `FileHandle` path safe (it may hold a `-1` fd sentinel from
a failed open), extract `fsync_sync_inner` / `fdatasync_sync_inner` /
`fchown_sync_inner` / `write_string_sync_inner` /
`write_buffer_sync_inner` / `writev_sync_inner` helpers without
validation and route `filehandle_*_impl` through those. The C-ABI
`js_fs_*_sync` entry points still validate as before.

Verification:

- New `test-parity/node-suite/fs/sync/arg-validation.ts` is byte-identical
  to `node --experimental-strip-types` across 27 cases covering fd
  type+range, path-type, uid/gid type+range, copyFile src/dest/mode,
  write/writev fd, and the writev empty-array no-throw contract.
- Direct re-run of `node-suite/fs/fd/{fdatasync, fstat-ftruncate-fsync,
  readv-writev, read-write-optional}.ts`, `callbacks/{chown,
  copyfile-excl, lchmod}.ts`, and `sync/captured-methods.ts` all
  byte-identical with Node — no regressions in the affected paths.
- `cargo fmt --all -- --check` clean.

Follow-ups still in #2013: `lchmodSync` mode + `lchownSync` range on
existing paths (paired with `ENOENT` propagation), required-callback
validation on `fsync`/`fdatasync`/`fchown` async wrappers, and the
non-fs APIs (buffer, net, http, zlib, crypto, process, url, timers).
@proggeramlug proggeramlug merged commit 578ae16 into main May 28, 2026
10 checks passed
@proggeramlug proggeramlug deleted the fix-2013-fs-arg-validation branch May 28, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant