Skip to content

fix(runtime): Node arg-validation errors on fs + assert.throws code/name matching (#2013, #2014)#2035

Merged
proggeramlug merged 1 commit into
mainfrom
fix/stdlib-arg-validation-2013-2014
May 27, 2026
Merged

fix(runtime): Node arg-validation errors on fs + assert.throws code/name matching (#2013, #2014)#2035
proggeramlug merged 1 commit into
mainfrom
fix/stdlib-arg-validation-2013-2014

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

Addresses the cross-cutting Node error-validation parity theme surfaced by the #800 sweep after #1924 fixed assert.throws. Two coupled issues:

They're handled together because verifying #2013 honestly requires #2014's matcher to actually compare error codes.

#2014assert.throws object validator

  • Error objects now hold user-assigned own properties. ErrorHeader is a fixed #[repr(C)] struct with no field-storage region, so err.code = "X" / err.errno = -2 / custom fields were silently dropped and read back as undefined. A per-error side table (keyed by the error pointer, owned-String for string values / raw bits for immediates — GC-safe) now backs these; the object setter routes GC_TYPE_ERROR sets to it, and the .code/.name/… getters check it first (so a user override also works).
  • object_matcher_matches: dropped the over-lenient carve-out that treated a missing actual code/errno as a match — a wrong/absent code is now a mismatch, matching Node.
  • expected_error_matches: a plain object validator ({ code: … }) now matches only via its own keys; it no longer falls through to the instanceof / builtin-constructor checks that could spuriously accept any error.
const e: any = new Error("x"); e.code = "ERR_A";
assert.throws(() => { throw e; }, { code: "ERR_B" });  // before: passed (bug) — now: throws ✓

#2013fs argument validation

New reusable fs::validate module:

  • validate_path (path-only functions) → ERR_INVALID_ARG_TYPE (TypeError) on any non path-like input (string / Buffer / file: URL are accepted).
  • validate_path_or_fd (readFileSync / writeFileSync) → treats a number as a file descriptor and throws EBADF for an unregistered/closed fd, otherwise ERR_INVALID_ARG_TYPE. fd validity is checked against Perry's synthetic FD_REGISTRY (fds start at 100), not the OS — an early fcntl-based version wrongly rejected valid Perry fds and broke the fd read/write tests; fixed.

Wired into accessSync, statSync, lstatSync, readdirSync, mkdirSync, unlinkSync, rmdirSync, readlinkSync, openSync, readFileSync, writeFileSync.

fs.readFileSync(123 as any);  // before: returned ""    — now: throws EBADF ✓
fs.accessSync(true as any);   // before: code=undefined — now: ERR_INVALID_ARG_TYPE ✓

Verification

  • Byte-identical to Node v25 across the full fs validation matrix (path-only → ERR_INVALID_ARG_TYPE; readFile/writeFile number → EBADF, other bad types → ERR_INVALID_ARG_TYPE) and the assert.throws repro/positive/negative cases.
  • No regressions in the fs or assert node-suites. Remaining failures are pre-existing and unrelated to this change: assert.rejects is unimplemented; deepStrictEqual Map/Set/cause gaps; CallTracker (removed in Node 20); cp symlink-deref; rmdirSync({recursive}) removal error.
  • cargo fmt --all -- --check clean.

Follow-ups (not in scope here)

…ame matching (#2013, #2014)

#2014 — assert.throws object validator:
- Error objects now store user-assigned own properties (err.code = "X",
  err.errno = -2, custom fields) via a per-error side table consulted by the
  object setter and the .code/.name/... getters. ErrorHeader is a fixed
  #[repr(C)] struct with no field-storage region, so these assignments were
  previously dropped and read back as undefined.
- object_matcher_matches: drop the over-lenient carve-out that treated a
  missing actual code/errno as a match, so a wrong code is now rejected.
- expected_error_matches: a plain object validator matches only via its own
  keys, no longer falling through to the instanceof / builtin-constructor
  checks that could spuriously accept any error.

#2013 — fs argument validation:
- New fs::validate module. validate_path (path-only functions) throws
  ERR_INVALID_ARG_TYPE on non path-like input; validate_path_or_fd
  (readFileSync/writeFileSync) treats a numeric argument as a file
  descriptor and throws EBADF for an unregistered/closed fd, otherwise
  ERR_INVALID_ARG_TYPE. fd validity is checked against Perry's synthetic
  FD_REGISTRY (fds start at 100), not the OS.
- Wired into accessSync, statSync, lstatSync, readdirSync, mkdirSync,
  unlinkSync, rmdirSync, readlinkSync, openSync, readFileSync, writeFileSync.

Verified byte-identical to Node v25 across the fs and assert validation
cases. No regressions in the fs or assert node-suites (remaining failures are
pre-existing and unrelated: assert.rejects is unimplemented, deepStrictEqual
Map/Set/cause gaps, CallTracker, cp/symlink deref, rmdirSync recursive-option
removal).
@proggeramlug proggeramlug merged commit 8fedff8 into main May 27, 2026
10 checks passed
@proggeramlug proggeramlug deleted the fix/stdlib-arg-validation-2013-2014 branch May 27, 2026 16:28
proggeramlug added a commit that referenced this pull request May 28, 2026
…hmod/copyFile/write/writev (#2013) (#2181)

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).

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug added a commit that referenced this pull request May 28, 2026
…chmodSync/chmodSync/renameSync (#2013) (#2328)

PR #2035 added the validation pattern for `fs.readFileSync` /
`fs.accessSync` / `fs.statSync` / `fs.openSync`; the issue's
follow-up call-out lists ~27 fs functions that still hit
"Missing expected exception" because they don't yet validate. This
gap-fills the fd-only sync surface (`closeSync`, `readSync`,
`readvSync`, `writeSync`, `writevSync`, `fchmodSync`, `fsyncSync`,
`fdatasyncSync`, `fchownSync`) and the path-mutator sync surface
(`chmodSync`, `renameSync`) so each:

  - throws `TypeError [ERR_INVALID_ARG_TYPE]` on a non-fd / non-path
    first argument (matches Node);
  - throws `Error [EBADF]` on a numeric fd that isn't open in
    perry-runtime's `FD_REGISTRY` (matches Node's syscall surface).

The new `validate_fd_open(value, syscall)` helper combines the
existing `validate_fd` type+range probe with an `FD_REGISTRY`
membership check + EBADF throw. Callers that need the validation
order preserved (`fchownSync`: validate uid+gid BEFORE EBADF, per
Node's `validateInteger` ordering) keep the bare `validate_fd` and
issue the EBADF after the rest of the int32 validations. `validate_fd`
itself stays single-purpose so the FileHandle wrappers — which can
legitimately hold a `-1` sentinel from a failed open — keep their
silent no-op fallback.

The parity test (`test-parity/node-suite/fs/sync/arg-validation.ts`)
gets 13 new probes covering each new shape against
`node --experimental-strip-types`; the existing legacy block stays
byte-identical. `cargo test -p perry-runtime --lib` still 746/746.

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug added a commit that referenced this pull request May 29, 2026
…gth (#2013) (#2440)

Buffer.alloc/allocUnsafe/allocUnsafeSlow now throw ERR_INVALID_ARG_TYPE on a
non-number size and ERR_OUT_OF_RANGE on NaN/Infinity/negatives; Buffer.concat
throws ERR_INVALID_ARG_TYPE for a non-Array list / non-Buffer element and
validates totalLength; Buffer.byteLength rejects a non string/Buffer/
ArrayBuffer/TypedArray first argument. Previously these coerced silently, so
assert.throws-style tests saw 'Missing expected exception'.

Reuses the fs::validate primitives (#2035) — the shared validation home the
issue calls out — extending describe_received to render strings/bigints.
New runtime validators are reached from the Buffer factory codegen
(env_clones.rs / array_methods.rs) and the byteLength/concat runtime paths.

Verified byte-identical to Node v25 across 120 buffer node-suite cases
(incl. 3 new arg-validation tests); no regressions.

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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