fix(ext/node): reject non-ArrayBufferView data in fs.writeFileSync#33406
Conversation
Node's fs.writeFileSync only accepts string or ArrayBufferView data (see lib/fs.js): non-ArrayBufferView data is routed through validateStringAfterArrayBufferView, which throws ERR_INVALID_ARG_TYPE. The async Promise variant (lib/internal/fs/promises.js) additionally accepts custom iterables via isCustomIterable, but the sync version does not. Deno's writeFileSync applied the iterable check, so fs.appendFileSync(path, []) silently wrote zero bytes instead of throwing. Drop the iterable short-circuit for the sync path — the async writeFile / fsPromises.writeFile paths are unchanged. Enables parallel/test-fs-append-file-sync.js in the node compat suite. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Reviewed — correct and well-targeted. Verified against Node source:
Two observations, both out of scope for this PR but worth a follow-up:
Test plan looks reasonable — |
|
CI failure on `test node_compat debug windows-x86_64` is the same Windows flake we've seen elsewhere this stack (also hit on #33401 and #33404): `parallel/test-child-process-exec-kill-throws.js` — the test asserts `child.killed === true` but occasionally observes `false` on Windows runners (job 72824560998). No relationship to this PR's |
3 participants
Summary
fs.writeFileSynconly acceptsstringorArrayBufferViewfordata. Non-ArrayBufferView input is routed throughvalidateStringAfterArrayBufferView, which throwsERR_INVALID_ARG_TYPE(seelib/fs.jswriteFileSync). Only the async Promise variant atlib/internal/fs/promises.jsbranches onisCustomIterableto support iterables.fs.appendFileSync(path, [])silently wrote zero bytes instead of throwing. Drop the iterable short-circuit for the sync path only;fs.writeFile/fsPromises.writeFileare unchanged.parallel/test-fs-append-file-sync.jsin the node compat suite.Test plan
cargo test --test node_compat -- test-fs-append-file-sync.jspassestest-fs-buffertype-writesync,test-fs-writev-synctest-fs-write-stream.jsstill fails in local runs onupstream/mainas well — pre-existing flake, not caused by this change🤖 Generated with Claude Code