The child test process sends framed report messages and raw user stdout
over one pipe, using the bytes FF 0F to mark the start of a frame. User
output can contain those same bytes, so #processRawBuffer could read a
plausible size from stray stdout and hand the bytes to the v8
deserializer. The deserializer then threw. Because the call had no error
handling, the exception aborted the whole test run.
Read the frame before advancing the buffer and wrap the deserialize in a
try/catch. When the read fails, leave the buffer untouched and stop
parsing frames so #drainRawBuffer emits the stray byte as stdout and
rescans for the next real header. This turns a fatal crash into
recoverable stdout and preserves any real frames that follow the stray
bytes.
Fixes: https://github.com/nodejs/node/issues/66164
Signed-off-by: Muhammad Faizan Uddin <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66273
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Stopping a Worker while it is running an FFI callback aborted the whole
process with "Callbacks cannot throw an exception". This happened on
worker.terminate(), on process.exit() inside the callback, and when the
main thread exited while the Worker was in a callback, since exit
terminates all Workers.
All three stop the Worker by terminating execution, and InvokeCallback
treated the termination as a thrown exception. Check HasTerminated()
first and return a zeroed result so the native caller can unwind.
Callbacks that throw still abort.
Signed-off-by: Trivikram Kamat <[email protected]>
Assisted-by: claude:opus-5.5
PR-URL: https://github.com/nodejs/node/pull/66389
Fixes: https://github.com/nodejs/node/issues/66388
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
close() destroys the connection but keeps the DatabaseSync object, and
open() did not replay the state held on it.
An authorizer set with setAuthorizer() was not reinstalled, silently
dropping a deny-all policy. Limits written through db.limits.* reverted
to the constructor values. Extension loading was re-enabled from the
constructor ceiling rather than the current setting, so the connection
flag contradicted a previous enableLoadExtension(false), though
loadExtension() itself stayed blocked by its own check.
Signed-off-by: Guilherme Araújo <[email protected]>
Assisted-by: Claude Code
PR-URL: https://github.com/nodejs/node/pull/66042
Reviewed-By: Trivikram Kamat <[email protected]>
When spawn()/spawnSync() are called without options.env,
normalizeSpawnArguments() copied process.env with a spread and then
walked the copy to build the KEY=value array uv_spawn() takes.
Spreading the process.env proxy costs one enumerator callback plus a
query and a getter interceptor per variable, each doing a linear
getenv() scan and allocating; with a couple of hundred variables that
was the single largest JS-side cost of spawning a process.
Add KVStore::Pairs() (Enumerate() + Get() by default, one
uv_os_environ() pass for the real environment, skipping hidden
variables on Windows exactly like Enumerate() does), expose it as
process_wrap.getEnvPairs(), and use it for the default-environment
case. A user supplied options.env and the permission model case keep
the existing code. On Windows the same sort/first-wins-case-insensitive
filter is applied to the pairs. The variables copyProcessEnvToEnv()
propagates are part of the real environment by definition, and its
entries cannot contain null bytes, so those steps only remain on the
options.env path.
Signed-off-by: Shelley Vohr <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/65325
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
GetValidatedSize() checks the value against
static_cast<double>(SIZE_MAX), which rounds up to 2^64 on 64-bit
platforms. A length or offset of 2 ** 64 gets through, and the cast to
size_t after it is undefined behavior. With GCC on x64 it gives 0, so
ffi.setUint8(ptr, 2 ** 64, 42) writes to ptr instead of throwing.
Anything above Number.MAX_SAFE_INTEGER may already have been rounded
by the time it gets here, so reject those values too. The export*()
helpers already cap their length there, and so does setInt64() for
number values. SIZE_MAX is still the limit on 32-bit platforms.
When buffer.constants.MAX_LENGTH is Number.MAX_SAFE_INTEGER, as on
64-bit builds without the V8 sandbox, toBuffer() and toArrayBuffer()
now throw ERR_OUT_OF_RANGE for MAX_LENGTH + 1 instead of
ERR_BUFFER_TOO_LARGE.
Signed-off-by: Christian Aurich Zanettini Martins <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66216
Reviewed-By: René <[email protected]>
When mkdir() fails with ENOENT, the recursive algorithm assumes the
parent is missing, creates it and retries. On procfs mkdir() keeps
returning ENOENT although the parent exists, and under a concurrent
rmdir() the parent can disappear again, so the retry loop never
terminates: `fs.mkdirSync('/proc/x', { recursive: true })` spins at
100% CPU.
Retry a path that failed with ENOENT only once and report ENOENT the
second time, like the non-recursive call does.
Fixes: https://github.com/nodejs/node/issues/66268
Signed-off-by: marcopiraccini <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66340
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
With the FreeEnvironment() fix for sibling Environments and the handle
cleanup depth tracked per thread, only the Environment being freed
loses JavaScript while FreeEnvironment() runs the shared loop. Callbacks
of the other Environments on that loop run their JavaScript as usual.
Update embedding.md and the comment in node.h, which still describe
JavaScript as disallowed on the whole isolate.
Signed-off-by: Shelley Vohr <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
An Environment created with kNoCreateInspector threw a bare string from
inspector.Session#connect(), inspector.open() and the other Agent entry
points, so callers could not tell the condition apart by error code. Use
the ERR_INSPECTOR_NOT_AVAILABLE code that connectToMainThread() already
throws for the same situation.
Signed-off-by: Shelley Vohr <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
The FreeEnvironment() fix for sibling Environments keeps the depth of
nested Environment::CleanupHandles() calls on the IsolateData, so that
InternalCallbackScope can re-allow JavaScript for sibling Environments
while one of them is being freed. Environments that each have their own
IsolateData on the same isolate and loop never see that counter and
still fail with "illegal access". Environments that share a loop share a
thread, so keep the depth in a thread_local instead.
Refs: https://github.com/nodejs/node/pull/65977
Signed-off-by: Shelley Vohr <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66239
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
The TTY cases inherit TERM, CI and the color override variables. These
can disable colors in a case expecting styling, or force colors in a
case expecting plain text. The tests skip these cases without a TTY,
which hides the dependency in many standalone runs.
Set a color-capable TERM and only the environment variables specified
by each case. Remove the RISC-V flaky expectations.
Refs: https://github.com/nodejs/build/issues/4099#issuecomment-5070947806
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Claude, Codex
PR-URL: https://github.com/nodejs/node/pull/66320
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
common.isAlive() sends SIGCONT while polling exiting workers. On Linux,
this can discard a pending SIGSTOP from LeakSanitizer's ptrace attach,
leaving its thread-suspension loop waiting indefinitely.
Use signal 0 to check existence without changing the process state.
This also makes the liveness check work on Windows, where SIGCONT is
unsupported. Treat only ESRCH as a dead process and propagate other
errors. Remove the ASan flaky expectation.
Refs: https://github.com/nodejs/node/issues/39655
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Claude, Codex
PR-URL: https://github.com/nodejs/node/pull/66320
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Windows directory watchers report last-access time updates. Reading cold
fixtures in the child can therefore trigger test:watch:restarted before
it exits, failing the mustNotCall assertion without printing the error.
Read the fixtures before starting watch mode and remove the Windows
flaky expectation. With old access times and a delayed child exit, the
original fails 10/10 runs and the change passes 100/100 on Windows.
Also remove the argv variant's stale flaky expectation. It passes
repeated local Windows runs and the inspected CI history after #66035.
Refs: https://github.com/nodejs/node/issues/66056
Refs: https://github.com/nodejs/node/pull/66035
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66320
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
The second request in the headers-timeout test could finish its headers
before the next periodic timeout check ran. Leave it incomplete across
multiple checking intervals before sending the remaining headers.
Remove both macOS keepalive flaky expectations. c3aa86d678 already
extended the request-timeout test's margin to allow multiple checks.
Refs: https://github.com/nodejs/node/issues/42741
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Claude, Codex
PR-URL: https://github.com/nodejs/node/pull/66320
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Wait for the target to finish starting before attaching the debugger.
Capture the CLI close event when it is spawned so quit() also completes
after an early exit, and terminate the target before awaiting cleanup.
Add a regression test for quitting an already exited CLI and remove the
Windows flaky entry.
Refs: https://github.com/nodejs/node/issues/63212
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66320
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
A Windows socket bound to 127.0.0.1 can accept connections intended for
another process's wildcard listener on the same port. A competing
inspector can then answer the HTTP request with 400 or send plaintext
to the HTTPS client.
Bind both fixture servers to the address used by their requests to
prevent the competing bind. Bypass environment proxies so local HTTP
and HTTPS requests reach the fixture servers directly, and remove the
Windows flaky expectation.
Fixes: https://github.com/nodejs/node/issues/59090
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Claude, Codex
PR-URL: https://github.com/nodejs/node/pull/66320
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Cipher.update(string, badEncoding, ...) and Decipher.update with
the same shape silently produced incorrect output: the binding
skipped the unrecognized encoding and fell back to a default,
giving the user wrong ciphertext or plaintext with no signal.
Sub-cases 1 and 2 from issue #45189 (bad output encoding to
update/final) were addressed in PR #45990. This commit completes
the fix for sub-case 3 (bad input encoding) per panva's comment
deferring it to a follow-up PR for CITGM testing. When `data` is
a string and `inputEncoding` is non-null but does not normalize
to a known encoding, throw ERR_UNKNOWN_ENCODING. Buffer /
TypedArray / DataView data paths are unaffected (the binding
ignores `inputEncoding` for non-string data anyway).
Fixes: https://github.com/nodejs/node/issues/45189
Refs: https://github.com/nodejs/node/pull/45990
Signed-off-by: Maruthan G <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66247
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PrepareFunction() looked up the function cache before parsing the
signature. Parsing can run user getters, and a getter that calls
lib.close() clears the cache, which invalidates the iterator. For a
function that was already cached, reading it afterwards used freed
memory and crashed the process.
Look up the cache after the signature is parsed. If the library was
closed during parsing, ResolveSymbol() now throws
ERR_FFI_LIBRARY_CLOSED.
Signed-off-by: Trivikram Kamat <[email protected]>
Assisted-by: claude:opus-5.5
PR-URL: https://github.com/nodejs/node/pull/66368
Fixes: https://github.com/nodejs/node/issues/66367
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Exporting a key in 'raw', 'raw-public' or 'raw-seed' format when the
key type does not match (e.g. an ECDSA private key as 'raw', or an
ML-KEM public key as 'raw-seed') fell through to the generic
NotSupportedError. The Web Crypto and modern-algos export key steps
require an InvalidAccessError in these cases.
Mirror exportKeySpki() and exportKeyPkcs8(): select the exporter per
algorithm first, then check the key type, and drop the type guards
around the call sites in exportKeySync(). Formats an algorithm does
not support (e.g. 'raw' for ML-DSA) still throw NotSupportedError.
This also fixes wrapKey(), which uses the same export path.
Assisted-by: a closed-source coding agent
Signed-off-by: koreahghg <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66217
Reviewed-By: Filip Skokan <[email protected]>
Return SQLITE_CONSTRAINT from xBestIndex() when a hidden column has
an equality constraint that is not usable at the current point in the
query plan. Accepting such a plan makes xFilter() call rows() with
null, silently dropping rows and returning an empty result, e.g. for
"SELECT DISTINCT * FROM m(t.a) JOIN t ON t.a = m.a". Cost estimates
alone cannot enforce parameter dependencies; rejecting the plan lets
SQLite pick an ordering where the parameter value is available.
Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: opencode
PR-URL: https://github.com/nodejs/node/pull/66215
Fixes: https://github.com/nodejs/node/issues/66214
Reviewed-By: Guilherme Araújo <[email protected]>
Passing decodeStrings: false to a zlib stream, or to an async
convenience method, let strings reach the native handle, which
aborts the process on its Buffer check. Force decodeStrings back to
true, as is already done for encoding and objectMode.
Signed-off-by: James Ross <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66359
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
zstdCompress() writes its input and ends the frame in separate calls,
so zstd cannot infer the input size the way it does for
zstdCompressSync(), and sizes its tables for an unbounded stream.
Default pledgedSrcSize to the input's byte length. The output is now
identical to zstdCompressSync() and several times faster at higher
levels. An explicit pledgedSrcSize, or a string with a custom
defaultEncoding, keeps the current behavior.
Signed-off-by: James Ross <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66358
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Close the outside port and remove its message forwarding listeners
synchronously. Closing the port alone is asynchronous and can leave
queued messages dispatching when terminate() runs inside a listener.
Allow the current event to finish while discarding subsequent messages,
including those drained when the backing thread exits.
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66354
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Aviv Keller <[email protected]>
The outside port must serialize and transfer messages even when it has
no peer. Retain that port after the backing thread exits and create a
closed port when the entry script fetch fails. This preserves clone
errors and transfer side effects in both cases.
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66354
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Aviv Keller <[email protected]>
Evaluate blob module sources under their original URL instead of
rewrapping them in a data URL. This preserves import.meta.url and module
identity while retaining the captured source after URL revocation.
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66354
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Aviv Keller <[email protected]>
Web IDL converts every argument before entering the method algorithm.
Perform all USVString conversions before rejecting module workers or
parsing URLs, so later conversions can throw or revoke blob URLs first.
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66354
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Aviv Keller <[email protected]>
Web IDL overload resolution treats functions as objects, accepts a null
iterator as missing, and retrieves the iterator method only once. Reuse
that method during sequence conversion in both postMessage entry points.
Signed-off-by: Filip Skokan <[email protected]>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66354
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Aviv Keller <[email protected]>
SQLite serves TEXT up to SQLITE_MAX_LENGTH, past what V8 can represent
as a string. V8 returns an empty handle without throwing, and the
user-defined function path then suppressed the SQLite error as if a
JavaScript exception were pending. exec() reported success for a
statement that never ran, and get() returned undefined.
Throw ERR_STRING_TOO_LONG when the value cannot be converted, and
suppress a SQLite error only when an exception is actually pending.
Assisted-by: Claude Code
Signed-off-by: Guilherme Araújo <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/66209
Reviewed-By: Trivikram Kamat <[email protected]>
dependabot[bot]