On Windows, libc++ std::filesystem::remove and remove_all do not
automatically clear the read-only attribute before deleting a file
(unlike MSVC STL). This causes fs.rmSync to fail with EPERM when
trying to remove read-only files in environments where Node.js is
built using clang libc++ (such as Electron).
This commit introduces a Windows-specific helper
ClearReadOnlyAttributeW which clears the FILE_ATTRIBUTE_READONLY
attribute recursively (or for a single file) when
operation_not_permitted is returned, allowing rmSync to
successfully delete read-only files/folders.
Fixes: https://github.com/nodejs/node/issues/64374
Signed-off-by: SparshGarg999 <sparshgarg999@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/64453
Fixes: https://github.com/nodejs/node/issues/64374
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
The embedder docs say each Environment has exactly one `uv_loop_t` and
that an `IsolateData` can be shared between Environments, and
`CreateIsolateData()` takes the loop, so several same-thread
Environments naturally end up on one loop. Nothing mentions that
`FreeEnvironment()` then runs that loop, with JavaScript disallowed on
the isolate, until the freed Environment's handles are gone, so the
other Environments' callbacks can fire inside it. Document that, and
point at it from `FreeEnvironment()` in node.h.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65691
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
SocketAddressLRU::Upsert always inserts an entry before evicting down
to max_size_. With max_size_ == 0, it evicts the entry it just
inserted and then accesses the now-missing key via
map_[address]->second. operator[] recreates the key with a
default-constructed std::list iterator, which is then dereferenced.
This is undefined behavior, observed as a SIGSEGV in Endpoint::Receive
on the first UDP packet accepted by a QuicEndpoint constructed with
{ addressLRUSize: 0 }.
SocketAddressLRU has no useful semantics for a zero-capacity cache,
and Upsert's callers rely on it returning a valid pointer. Reject 0
(and 0n) at the options-parsing boundary instead of changing Upsert's
contract.
Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65827
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Without a startup snapshot (`--no-node-snapshot`, a
`--without-node-snapshot` build, or an embedder Environment that was
bootstrapped from scratch) starting a Worker made a member call through
a null `SnapshotData*`, and so did `NodeMainInstance` while setting
itself up. It only worked because the function called does not touch
`this`; UBSan reports it for every such Worker. The call existed
because `IsolateData::CreateIsolateData()` took an
`EmbedderSnapshotData*` and unwrapped it straight away, so the two
internal callers wrapped their possibly-null `SnapshotData*` with
`AsEmbedderWrapper()` only for it to be unwrapped again.
Let the internal function take the `SnapshotData*` itself, unwrap in
the public `CreateIsolateData()` only, and drop `AsEmbedderWrapper()`,
which has no other users.
Refs: https://github.com/nodejs/node/pull/47731
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65820
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
An embedder that creates its IsolateData without a MultiIsolatePlatform
(allowed; node.h says only Workers need one) and keeps the inspector
segfaulted on the first `console.log()`, `console.time()` or profiler
use after a `node:inspector` session was connected: V8 calls the
inspector client's `currentTimeMS()` there, and
`NodeInspectorClient::currentTimeMS()` dereferenced
`isolate_data()->platform()` unconditionally.
Fall back to the wall clock when there is no platform, which is what
`NodePlatform::CurrentClockTimeMillis()` returns anyway.
Refs: https://github.com/nodejs/node/pull/21917
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65818
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Every stream write created a WriteWrap JS object up front, even though
most writes complete synchronously via uv_try_write() and never use it.
Let stream_base_commons pass null instead of a request object.
StreamBase::Write() already creates the wrap object only when the write
does not complete synchronously; return that object to JS (which
attaches oncomplete/callback to it) and a plain error code otherwise.
Writes that complete synchronously now cross the JS/C++ boundary once
and allocate nothing. Callers that pass in a request object
(child_process IPC, webstreams adapters) behave as before.
Since Http2Stream::DoWrite() can invoke the completion callback
synchronously - before JS has attached oncomplete - such completions
are now recorded on the request object's writeStatus field and replayed
by stream_base_commons after dispatch. This also replaces a Has() plus
name-based MakeCallback() pair with a single Get().
Also pre-create the JS fields of WriteWrap instances in the object
template, as was already done for ShutdownWrap, so that they are
in-object properties.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/64455
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Read buffers for streams that emit their data to JS were allocated per
read: a 64KB backing store, tracked in a map, and then - since reads
rarely fill the whole buffer - reallocated to the right size and copied.
Allocate read buffers from a 64KB slab instead. Reads reserve the
suggested size from the slab and JS receives a view over the slab's
ArrayBuffer at the read's offset, using the offset mechanism that
onStreamRead already supports. Unused reservation space is rewound when
a read returns less than was reserved, so small reads (e.g. TLS records)
share a slab.
This removes the per-read allocations, the map bookkeeping and the
resize copy.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/64455
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
`CommonEnvironmentSetup` created a `CppHeap` for its `CreateParams`
before deciding how to create the isolate, but `NewIsolate()` ignores
`params->cpp_heap` and attaches a heap of its own (or the one from
`IsolateSettings`). The first heap was never attached or destroyed, so
every non-snapshotting setup leaked one `CppHeap`; cppgc's heap
registry keeps it reachable, which is why LSAN stays quiet about it.
Only create the heap on the snapshotting path, where the params go to
the `SnapshotCreator` directly.
Refs: https://github.com/nodejs/node/pull/55337
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65792
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
fs.cp() and fs.promises.cp() walked the tree in JavaScript with several
thread pool round trips per entry (opendir batches, two stat()s, the
copyFile(), a chmod()), all awaited in sequence: a 2 100-file tree took
~215 ms with ~110 ms of that on the main thread, against ~36 ms for
fs.cpSync(), which copies the tree in C++ when no filter is given.
Factor that C++ walk into CopyDirRecursive(), which records the error
instead of throwing so that it can run on any thread, and run it as one
ThreadPoolWork request (CpDirJob) for fs.cp()/fs.promises.cp() when
the destination directory does not exist yet and nothing has to run per
entry (no filter, no dereference, permission model off). Copying into
an existing tree keeps the JavaScript walk and its rules for what may
already be there. The same tree now takes ~30 ms with under 1 ms on the
main thread.
For that job the walk follows the JavaScript walk's rules rather than
cpSync's: it creates every directory with mkdir() and every file with
an exclusive uv_fs_copyfile() (honouring the copyFile() mode flags) and
fails with EEXIST if anything has appeared in their place since the
JavaScript check, so it never opens or follows something it did not
create; sockets, FIFOs and unknown entries are reported back to
JavaScript, which rejects them with the same SystemErrors as before;
relative link targets are made absolute lexically as path.resolve()
does. cpSync keeps merging into existing directories, skipping special
files and canonicalizing link targets.
The walk now uses the error_code overloads of std::filesystem throughout
(directory iteration included), so an unreadable directory inside the
tree is reported as EACCES by both cp() and cpSync() instead of
terminating the process, which cpSync() has done since the walk moved
to C++. Filesystem errors raised inside the walk keep their codes, with
'cp' or 'copyfile'/'mkdir' as the syscall. With preserveTimestamps the
walk stamps each directory it filled, root included, so cp() keeps
preserving directory times and cpSync() without a filter now does too.
Both file copies set the destination's mode before writing the data,
which clears setuid and setgid, so the walk puts the source mode back
after each file it copied; the JavaScript walk already chmod()s there.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65488
Refs: https://github.com/nodejs/node/pull/58461
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
The C++ fast path that fs.cpSync() takes when no filter is given created
the destination directories with default permissions, so a 0700
directory came out of the copy as 0755 (with the default umask). The
JavaScript implementation, which fs.cp(), fs.promises.cp() and
fs.cpSync() with a filter still use, chmod()s every directory it
creates to the mode of its source, and so did cpSync before the port.
Set the source directory's permissions on each directory the copy
creates (the destination root included); directories that already
exist keep theirs, as before.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65488
Refs: https://github.com/nodejs/node/pull/58461
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
With V8_ENABLE_SANDBOX every ArrayBuffer backing store has to be
allocated inside the sandbox, so memory that Node.js or a library
allocated itself cannot be wrapped and has to be copied in. Several
places already special-cased this, each with its own #ifdef, but a
build with the sandbox enabled still failed to compile
(`kMaxSafeBufferSizeForSandbox`), aborted in `crypto.randomUUID()`
(`SecureBuffer` was UNREACHABLE) and threw from every caller of the
malloc-owning `Buffer::New()` (v8.serialize, string transcoding, IPC,
the public `Buffer::New(isolate, data, length)`), and trace_events,
SEA assets and FFI still wrapped outside memory unconditionally.
Add `AdoptIntoBackingStore()`, which wraps the memory as before in
regular builds and copies it into an isolate-allocated backing store,
running the deleter right away, when the sandbox is enabled, and use it
at all of those sites so the #ifdef lives in one place. The trace
category flag cannot be copied since JS polls it, so under the sandbox
`getCategoryEnabledBuffer()` returns nothing and lib falls back to
`isTraceCategoryEnabled()`; FFI zero-copy views throw
ERR_OPERATION_FAILED there for the same reason.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65500
Fixes: https://github.com/nodejs/node/issues/65499
Refs: https://github.com/nodejs/node/pull/62072
Refs: https://github.com/nodejs/node/pull/62858
Refs: https://github.com/nodejs/node/pull/65842
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--permission-audit is documented to perform permission checks without
denying access, publishing each violation through the diagnostics
channel while execution continues. Three code paths still enforced:
* fs.lstat() and fs.symlink() are checked in the JavaScript layer,
which threw ERR_ACCESS_DENIED without consulting audit mode. The
isAuditMode() helper introduced for ffi was never wired into fs.
* Addon loading was disabled upfront in Environment setup, so
process.dlopen() failed with ERR_DLOPEN_DISABLED before reaching
the audit-aware permission check, and no audit event was published.
Skip the throw in the JavaScript checks when audit mode is on, keeping
the permission.has() call first so the diagnostics channel event is
still published. Leave native addons enabled in audit mode while still
applying the kAddon scope, so DLOpen() publishes the denial through
the channel and continues to the regular loading path. This also
covers the worker path, which inherits kNoNativeAddons from the same
option.
Each fs API carries its own copy of the check in its sync, callback
and promise flavour, so the regression test exercises all six call
sites, asserting the --permission enforce behaviour as a control.
Fixes: https://github.com/nodejs/node/issues/65419
Refs: https://github.com/nodejs/node/pull/63047
Assisted-by: claude:opus-5
Signed-off-by: Ji Hoon Kang <ivory.ma9ic@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65659
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Inspector messages for a main thread connection are dispatched
from a V8 interrupt handler. Defer frontend messages in this
case using a microtask. This preserves the existing timing
contract for main thread inspector connections, where
notifications are observable after a microtask checkpoint.
Starting the inspector I/O thread and toggling network tracking
can also call into JS from an interrupt.
Signed-off-by: deepak1556 <hop2deep@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65397
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com>
CleanupHookThunkRun() read thunk->isolate/fun/arg from the
CleanupHookThunk after invoking thunk->fun(). For every
node::ObjectWrap alive at teardown, thunk->fun is
ObjectWrap::CleanupHook, which deletes the wrap; ~ObjectWrap() calls
RemoveEnvironmentCleanupHook() itself, erasing the CleanupHookThunk
from the registry and freeing the node it lives in. The subsequent
read of thunk->isolate/fun/arg to make the (now redundant) second
RemoveEnvironmentCleanupHook() call was therefore a use-after-free.
Cache the fields before running the hook so nothing is read from
`thunk` once it may have been freed.
Taken over from #65196, which has been inactive; the original change is
unmodified apart from the added comment.
This also unblocks #65042, the backport of the cleanup hook registry to
v24.x. Without that registry ~ObjectWrap() asserts during garbage
collection, so every 24.x runtime aborts for ObjectWrap addons
(#65446), as do 26.x runtimes before 26.4.0 when used with newer
headers (#65262).
Fixes: https://github.com/nodejs/node/issues/65195
Refs: https://github.com/nodejs/node/pull/65196
Refs: https://github.com/nodejs/node/pull/65042
Refs: https://github.com/nodejs/node/issues/65446
Refs: https://github.com/nodejs/node/issues/65262
Assisted-by: a closed-source coding agent
Co-authored-by: Sreehari Annam <sreehari.annam@gmail.com>
Signed-off-by: Caleb Everett <everett.caleb@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65630
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com>
A native addon inside a virtual file system could not be require()d:
dlopen() and LoadLibrary() open a shared object by path, and a VFS path
has no inode for them to open.
Read the addon's bytes from the VFS and load them from a private,
self-cleaning image instead, using the smallest on-disk footprint each
platform allows: on Linux an anonymous memfd loaded through
/proc/self/fd, so the bytes never reach the file system; on other POSIX
platforms a file in a 0700 mkdtemp() directory, unlinked right after
loading, with the mapping keeping it alive; on Windows a temp file
opened FILE_FLAG_DELETE_ON_CLOSE whose handle is held until exit.
This is internal. process.dlopen() keeps its documented
(module, filename[, flags]) signature and ignores anything further; the
bytes are passed to a dlopenBinary() reachable only through the
process_methods binding, which the VFS hook calls for VFS paths. Addons
on the real file system are untouched and load directly.
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
PR-URL: https://github.com/nodejs/node/pull/65680
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Every `new Worker()` runs the whole internal bootstrap (realm, node, web
exposure, thread and process-state switches) in its fresh isolate,
compiling ~80 builtins with the code cache; only the main thread
deserializes its principal context from the built-in snapshot. That
bootstrap is about half of a worker's cold start.
The bootstrapped principal context in the snapshot is nearly
thread-neutral: the worker-side switch scripts (is_not_main_thread,
does_not_own_process_state) are written as overrides of the main-thread
ones, and the per-thread values of the `worker` binding were the only
thread-specific data baked into the context. Let a worker deserialize
that same context and EnvSerializeInfo and apply the two worker-side
switches on top:
- worker binding: threadId, threadName, isMainThread, isInternalThread,
ownsProcessState and resourceLimits become lazy properties of the
per-isolate template, computed from the Environment on first read.
- CreateEnvironment(): when a worker (its IsolateData has a Worker)
passes an empty context, deserialize kNodeMainContextIndex and run
internal/bootstrap/switches/is_not_main_thread and, unless the worker
owns process state, does_not_own_process_state after
InitializeMainContext().
- Worker::Run(): take that path when the embedded built-in snapshot is
in use (not an embedder's or a --snapshot-blob one, which has run
application code), browser globals are not disabled and
--no-worker-snapshot was not given; otherwise bootstrap as before.
- is_not_main_thread.js: also delete _debugPause and the profiler idle
notifier helpers that is_main_thread.js installs.
- pre_execution: run the snapshot's deserialize callbacks (Buffer pool,
default resolver, cached cwd) on worker threads too, now that a worker
can come from a snapshot.
- --[no-]worker-snapshot per-isolate option, documented.
Sequential new Worker() -> 'online' -> terminate goes from ~20.9 ms to
~10.3 ms per worker on x64 Linux; --no-worker-snapshot restores the old
number.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65336
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Avoid repeating EC key-pair validation after a pair has already been
established or validated. Invalidate the positive-only cache whenever
public-key mutation can make the pair inconsistent.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65615
Reviewed-By: James M Snell <jasnell@gmail.com>
prepare(), function(), aggregate(), deserialize(), applyChangeset() and
backup() validated the connection, then read their options bag with
Object::Get(). A property getter runs arbitrary JavaScript at that
point, so a getter calling close() invalidates what was just checked.
Five of the six then passed a null sqlite3* to SQLite and crashed;
prepare() reported a spurious "out of memory".
Re-check IsOpen() after option parsing, immediately before the SQLite
call, keeping the early check so invalid calls still fail before any
user code runs. IsOpen() is the only condition a getter can change:
authorizer and callback depths are RAII-managed. createSession()
already parsed options first, so it only gains the early check.
deserialize() also latched the buffer length before reading
options.dbName. A getter that shrank the backing store left the length
too large; CopyContents() then handed the uninitialized remainder to
SQLite, from where serialize() returned it to JavaScript. Check the
CopyContents() result instead of discarding it.
function() and aggregate() cast the callback's length property with
As<Int32>() and no IsInt32() guard. length is configurable, so any type
reached the cast and produced a silently wrong arity.
Fixes: https://github.com/nodejs/node/issues/65586
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65595
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
fs.writeFile(path, data) took three libuv thread pool round trips
(open, write, close), each its own request with its own queue wait,
completion callback and JS/C++ crossing, and fs.promises.writeFile()
did the same through a FileHandle. For the small files applications
write most, the round trips are the cost, and each occupies a pool slot
that concurrent fs, dns.lookup() and crypto work is also queueing for.
Add WriteFileJob next to ReadFileJob: an AsyncWrap + ThreadPoolWork
that opens, writes the whole buffer (looping on short writes) and
closes as one pool task, keeping the buffer alive until it is done.
fs.writeFile() uses it for path arguments without flush;
fs.promises.writeFile() additionally keeps data above one write chunk
(and iterables) on the FileHandle path, so large writes stay abortable
between chunks as before. File descriptors, FileHandles, flush: true
and an active VFS keep their existing paths.
Behavior is otherwise kept: open failures report syscall 'open' with
the path, write failures 'write'; permission errors are delivered
through the callback/promise; an abort signalled while the write is in
flight is still reported as an AbortError; the job is an FSREQCALLBACK
resource for async_hooks and emits the 'write' fs trace event.
Tests that used fs.writeFile() as a proxy for open/close trace events,
or injected FileHandle faults for path-based writes, are adjusted to
keep testing what they test.
The job holds the buffer's backing store, so the memory stays valid if
the buffer is detached or collected before the write finishes; a
resizable ArrayBuffer could still have its pages decommitted by a
shrink, so its contents are copied when the job is created.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65489
Refs: https://github.com/nodejs/node/pull/65327
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Run SQLite backup after-work callbacks inside an internal callback
scope. This drains promise reactions and next ticks before the event
loop can become idle.
Add a child-process regression test where backup is the final active
request.
Assisted-by: Codex
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65666
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Environments created from the built-in snapshot get the builtins' code
cache from that snapshot. An embedder that bootstraps an Environment
from scratch (its own isolate and context, no EmbedderSnapshotData) has
no way to provide one: every builtin the bootstrap touches is compiled
from source in every such process, and each of them then serializes a
fresh cache (SaveCodeCache) that only a later worker thread would ever
consume.
Add node::EmbedderBuiltinCodeCache for that case. Its entries pair a
builtin id with a v8::ScriptCompiler::CachedData; Generate(context)
compiles every builtin in a context of the right kind of isolate and
returns them for a build step to embed, and
SetBuiltinCodeCache(isolate_data, cache) attaches them to an
IsolateData, next to where a snapshot's code cache lives, so that every
Environment created from it afterwards starts with them. The setter
runs CachedData::CompatibilityCheck() against the isolate first and
returns the result, leaving the IsolateData untouched for a cache made
with another V8 version, flag set or read-only snapshot. Entries from a
snapshot still merge with it (RefreshCodeCache() now merges instead of
assuming a single call). CreateEnvironment() plus LoadEnvironment() of
an empty script goes from about 38 ms to 13 ms with a cache attached.
ProcessInitializationFlags::kNoHarvestBuiltinCodeCache stops
serializing caches for builtins compiled without one, for embedders
that supply their own or never create workers. The default is
unchanged because worker threads copy the harvested cache.
A cctest generates a cache, attaches it, checks that a new
Environment's bootstrap compiles from it and that a corrupted cache is
refused; embedtest gains --no-harvest-builtin-code-cache for a test
that a worker does or does not find a harvested cache depending on the
flag.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65352
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
With this change, the TLS pause mechanisms used in node:tls are now made
generic, and used in QUIC to pause the handshake entirely for the
session event, thereby stopping almost all events being emitted
(everything except qlog) so that all the previously existing deferral
machinery is unnecessary.
This is useful immediately to clean up & simplify things, but also
medium term because this makes the 0RTT flow significantly clearer,
which will help with the upcoming dynamic attach for QUIC (which
requires deferring ticket processing & 0RTT delivery until after
user code).
As part of this, this changes the internal flow of how ALPN & SNI are
resolved. Instead of calculating them synchronously within the OpenSSL
callbacks, we precalculate them at the pause, and then the OpenSSL
callbacks just return the precalculated values.
This is intended to only create visible changes to node:quic. Though it
touches node:tls, it preserves all existing behaviour (including
callback order and other details).
Signed-off-by: Tim Perry <pimterry@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65522
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Allow --force-fips to take an optional provider or strict mode. Keep
provider as the current default and explicit compatibility mode.
In strict mode, return failure from the OpenSSL 3.4+ FIPS indicator
callback when it reports a non-approved operation. Install the callback
for enforcement without implicitly enabling diagnostics;
--enable-fips-indicator-events remains the independent observation
opt-in.
Keep the bare form mapped to provider for compatibility, allowing the
default to change to strict in a future major release. Preserve the mode
when forwarding flags to child test processes, and keep the parser's
internal mode storage out of process.allowedNodeEnvironmentFlags.
Document the callback scope limitations and cover provider
compatibility, validation, synchronous crypto, WebCrypto, Workers, and
opt-in event publication.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65645
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
OpenSSL 3.4 added an indicator callback for operations that its FIPS
provider allows after an approved-mode check has been relaxed.
Publish these indicators on the crypto.fips.indicator diagnostics
channel when Node.js is started with --enable-fips-indicator-events.
Leave the OpenSSL callback unset otherwise so ordinary cryptographic
operations have no indicator callback overhead.
Keep the channel observation-only so enforcement remains the
responsibility of the provider configuration, and independent diagnostic
subscribers can coexist.
Install the native callback during process initialization so it is not
changed while Workers or thread-pool jobs can be using OpenSSL. Preserve
callbacks installed by embedders, and queue bounded, coalesced messages
for asynchronous delivery on the main environment.
Document the channel as best-effort telemetry that cannot identify the
originating call or establish FIPS validation.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65645
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
X509Certificate::GetPeerCert() built the certificate objects by deleting
entries from the stack returned by SSL_get_peer_cert_chain(), which is
owned by the SSL session. The first read emptied it, so any later read
by getPeerCertificate() or getPeerX509Certificate(), on either peer, saw
a truncated chain or nothing. Copy each issuer with X509_dup instead and
leave the session's stack untouched.
onServerSocketSecure() only called getPeerX509Certificate() to check
whether a peer certificate was present, building the whole chain on
every server handshake; that is what first exposed the destructive read.
Use a lightweight hasPeerCertificate() binding for the presence check.
Signed-off-by: Tony Gies <tgies@tgies.net>
PR-URL: https://github.com/nodejs/node/pull/65602
Fixes: https://github.com/nodejs/node/issues/65579
Refs: https://github.com/nodejs/node/pull/64677
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
`NewIsolate()` deferred `SetIsolateErrorHandlers()` when snapshot data
was passed, and `CreateEnvironment()` later installed the handlers with
default `IsolateSettings` after deserializing the main context. An
embedder's `fatal_error_callback`, `oom_error_callback`,
`should_abort_on_uncaught_exception_callback` and
`prepare_stack_trace_callback` were therefore dropped whenever a
snapshot was used, and the per-isolate message listener was added even
if `MESSAGE_LISTENER_WITH_ERROR_LEVEL` had been cleared. The only way
to keep custom handlers was to call `SetIsolateUpForNode()` again after
`CreateEnvironment()`.
Install all handlers in `NewIsolate()` regardless of snapshot data, as
its documentation already describes, and stop touching isolate
handlers in `CreateEnvironment()`. The deferral dates from the initial
isolate snapshot work; every handler already copes with a missing
`Environment`, since without a snapshot they are installed before any
context exists, and workers have been calling `SetIsolateUpForNode()`
right after a snapshot `NewIsolate()` anyway.
Refs: https://github.com/nodejs/node/pull/27321
Refs: https://github.com/nodejs/node/pull/45888
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65407
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
normalizeReference() adds URL and Uint8Array support to has()/drop();
the existing string/Buffer behavior is unchanged. BufferValue replaces
Utf8Value so a Buffer/TypedArray reference is copied as raw bytes
instead of a lossy UTF-8 conversion.
Signed-off-by: seungmin Nam <nhjbest22@g.skku.edu>
PR-URL: https://github.com/nodejs/node/pull/65492
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add a thread_local re-entrancy guard to TriggerUncaughtException()
that detects when the JS-level exception handler
(process._fatalException) triggers another exception through the
inspector protocol.
This prevents the infinite loop:
TriggerUncaughtException -> InspectorConsoleCall ->
TriggerUncaughtException -> InspectorConsoleCall -> ...
When re-entrancy is detected, the function prints a diagnostic with the
formatted exception to stderr using FormatCaughtException (which avoids
the inspector path entirely), then aborts the process.
Fixes: https://github.com/nodejs/node/issues/64326
Signed-off-by: Temuulen Undrakhbayar <zerone.offical@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/64327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
CompileSerializeMain() created new "require", "__filename", and
"__dirname" strings via FIXED_ONE_BYTE_STRING() on every call, even
though these strings are already cached on IsolateData/Environment
as require_string(), __filename_string(), and __dirname_string()
(defined via PER_ISOLATE_STRING_PROPERTIES in src/env_properties.h)
and are reused this way elsewhere (e.g. the sibling
RunEmbedderPreload() already obtains Environment* the same way).
Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/65453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sparsh :)