`MemoryTracker::AddNode(const CppgcMixin*)` allocates a
`MemoryRetainerNode` that only stands in for the wrapper's JS node while
its `MemoryInfo()` runs; unlike the other node kinds it is not handed
to the `EmbedderGraph`, and nothing freed it. Every heap snapshot (or
other `BuildEmbedderGraph` call) leaked one node per live `vm.Script`
or `vm` context.
Keep the placeholders in the tracker and free them with it, and make
the already-seen path in `Track(const CppgcMixin*)` add its edge to the
wrapper's JS node like the first visit does, so the graph never refers
to a placeholder.
Refs: https://github.com/nodejs/node/pull/56534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65793
Reviewed-By: James M Snell <jasnell@gmail.com>
Expose SQLite's virtual table API through a new
`database.createModule(name, options)` method, wrapping
`sqlite3_create_module_v2()`. This enables read-only virtual tables
backed by JavaScript data sources, usable either as an eponymous table
(`SELECT * FROM module_name`) or via `CREATE VIRTUAL TABLE t USING
module_name`. Hidden columns pass parameters using table-valued
function syntax (`SELECT * FROM module_name(param1, param2)`).
`options` accepts `columns`, `rows`, `directOnly`, and
`useBigIntArguments`. Column types are validated against INTEGER,
TEXT, REAL, BLOB, and ANY, and column names are quoted when building
the `sqlite3_declare_vtab()` schema.
Rebased from https://github.com/nodejs/node/pull/61544, which was
opened by byteforge38 and became inactive. Changes on top of that
work:
- xColumn reports the value each hidden column was constrained to,
rather than NULL. SQLite treats xBestIndex's `omit` as a hint, so it
may recheck a constraint it already handed to xFilter; against NULL
that recheck rejected every row, and `gs(1, 3) WHERE start = 1`
returned no rows.
- xBestIndex lowers estimatedCost as it consumes constraints. With a
constant cost the planner was free to pick the unconstrained plan and
recheck afterwards, so a correlated parameter such as
`FROM t, gs(t.a, t.a + 1)` also returned no rows.
- Violations of the iteration protocol report a SQLite error instead of
calling PropagateJSError with no JavaScript exception pending. That
left `.all()` returning undefined and `exec()` reporting success.
- xBestIndex passes the constrained hidden-column indices to xFilter
through idxStr rather than an int bitmask, which previously aliased
for parameter indices at or above the width of an int.
- xFilter, xNext, and xColumn take a CallbackDepthGuard. Without it
close() from inside rows(), an iterator's next(), or a row getter
finalized the statement that SQLite was still stepping, crashing the
process.
- xClose calls the iterator's return() method so generator `finally`
blocks run when SQLite stops stepping early, as it does for LIMIT or
a `break` out of a for...of loop. It is skipped while tearing down
from ~StatementSync or ~DatabaseSync, which run from garbage
collection callbacks where JavaScript cannot be executed; an
abandoned generator does not run `finally` in JavaScript either. It
is also skipped when an error is already pending, so that error still
reaches the caller.
- VirtualTableModule holds a BaseObjectWeakPtr<DatabaseSync> to match
UserDefinedFunction instead of a raw pointer.
- createModule() rejects being called from an authorizer callback.
- Documents that values yielded by rows() follow the usual conversion
rules, so a number is stored as REAL and a BigInt as INTEGER even
when a column declares INTEGER, since virtual tables do not apply
column affinity to the values they return.
Refs: https://github.com/nodejs/node/pull/61544
Refs: https://github.com/nodejs/node/issues/63826
Fixes: https://github.com/nodejs/node/issues/61539
Co-authored-by: byteforge38 <stormcraft318@gmail.com>
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
Assisted-by: Claude Opus 5
PR-URL: https://github.com/nodejs/node/pull/65787
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`node_napi_env__::New()` returns nullptr after throwing when an add-on
declares a Node-API version this binary does not support, but
`napi_module_register_by_symbol()` dereferenced the result without
checking it. Loading such an add-on segfaulted instead of surfacing the
error the version check had already produced, so `require()` could not
catch it.
Reproduced on v20.x, v22.x, v24.x and v26.8.2 with a ten-line add-on
whose only distinguishing content is a NAPI_VERSION above
NODE_API_SUPPORTED_VERSION_MAX. `main`, `v22.x-staging` and
`v24.x-staging` all lack the check.
The error path had no test coverage: the message text appears in exactly
one file in the repository, `src/node_api.cc`. A test is added next to
`test_null_init`, which covers the sibling early return in the same
function.
Prepared with assistance from a closed-source coding agent, named in the
pull request description. The design, review and validation are my own:
I verified the fix and the test against a local build, including
removing the four added lines and relinking to confirm the test fails
without them.
Refs: https://github.com/nodejs/node/issues/57233
Signed-off-by: Alexey Karimov <krassx@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66019
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`FileHandle::TransferData` uses `-1` to indicate that it no longer owns
a file descriptor. However, its destructor only closes descriptors
greater than 0.
If a transferred `FileHandle` owns fd 0 and the message is discarded,
the descriptor is left open.
Treat fd 0 like any other valid descriptor and only skip `-1`.
Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66095
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Whether the V8 GC callbacks used for `'gc'` performance entries were
installed was tracked by a boolean in JavaScript, separately from the
native state, and the two could get out of sync. After deserializing a
user-land snapshot built while a `'gc'` PerformanceObserver was active,
the boolean claimed the callbacks were installed, although V8 GC
callbacks do not survive a snapshot. Observing `'gc'` again did not
install them, and disconnecting removed callbacks that had never been
registered, crashing the process.
Track the installation state in `PerformanceState` instead, and replace
the install and remove bindings with a single idempotent
`updateGarbageCollectionTracking()` binding, which registers the
callbacks if and only if there are `'gc'` observers.
Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66097
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The registry behind `AddEnvironmentCleanupHook()` is keyed on
{isolate, fun, arg} and asserts that every insertion is unique. Two
Environments on one isolate that register the same hook, which the
Node-API documentation allows per environment, abort the process on
the second `napi_add_env_cleanup_hook()`.
Key the registry on `arg` only and tell entries apart by Environment:
adding the same hook to one Environment twice still aborts as
documented, and removal prefers the current Environment's registration,
falling back to a matching one from another Environment when there is
no current context. Because the entry to remove after a hook has run can
no longer be found by {isolate, fun, arg} alone, `CleanupHookThunkRun()`
marks its entry as running and erases exactly that entry afterwards; a
removal of a running entry (a hook removing itself, as `~ObjectWrap()`
does) is a no-op, which keeps the use-after-free fixed by #65630 fixed.
Refs: https://github.com/nodejs/node/pull/63985
Refs: https://github.com/nodejs/node/pull/65630
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65777
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Since #65353, FlushForegroundTasksInternal() and the
DelayedTaskScheduler drain their queues with
`for (auto& task : queue.Lock().PopAll())`. In C++20 the Locked
temporary is destroyed at the end of the range initializer, before the
loop body runs. C++23 (P2718R0) extends the lifetime of every temporary
in a range-for initializer to the end of the loop, so the queue's mutex
stays held while the tasks run, and the first task that posts to the
same queue - any V8 foreground task that schedules another one -
deadlocks on the non-recursive mutex. Node.js itself builds with
-std=gnu++20, but an embedder that compiles it as C++23 hangs in the
first foreground task flush.
Store the drained tasks in a local before iterating so the lock is
released independent of the language version.
Refs: https://github.com/nodejs/node/pull/65353
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66066
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Retrieving asymmetricKeyDetails only needs the modulus, public exponent,
and RSA-PSS restrictions. Add a public-only Rsa view so provider-backed
keys do not also extract private components or probe additional primes.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66108
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Pass group names through EC key generation and report provider names
in key details without requiring an OpenSSL NID. Preserve established
curve aliases and synchronous invalid-curve errors.
Filter built-in curves through EC parameter generation and refresh
getCurves() results when FIPS properties change.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66108
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Resolve provider ciphers before serializing private keys and retain the
fetched implementation across encoding configuration copies and async
key generation. Keep format-specific restrictions in the serializers.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66108
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
process._linkedBinding() is subject to the permission model's addon
scope since the check was added to GetLinkedBinding(). For an embedder
that implements part of its runtime as linked bindings this means its
own bootstrap cannot reach them under --permission unless the user
also passes --allow-addons, which allows loading addons from the file
system as well.
Add EnvironmentFlags::kNoAddonPermissionForLinkedBindings. When set,
GetLinkedBinding() skips the addon permission check for that
Environment and the worker threads it creates; process.dlopen() stays
gated and the default behavior is unchanged.
Refs: https://github.com/nodejs/node/pull/65432
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66067
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`performance.nodeTiming.uvMetricsInfo` returns the libuv event loop
metrics as numbers, which are only exact up to
`Number.MAX_SAFE_INTEGER`. Add `uvMetricsInfoBigInt`, which returns
the same metrics as bigints backed by `uint64_t` storage, carrying the
full 64-bit range reported by libuv.
A single native call fills both a `Float64Array` and a
`BigUint64Array`, so `uvMetricsInfo` does not pay for bigint
allocation and conversion. The new property is omitted from
`toJSON()`, as `JSON.stringify()` cannot serialize bigints.
Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66094
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
libuv reports the event loop metrics exposed through
`performance.nodeTiming.uvMetricsInfo` as `uint64_t` counters, but
they were copied into an `Int32Array` (and, before that, converted
using `v8::Integer::New()`), so they wrapped around after 2^31 on
long-running processes.
Store the metrics in a `Float64Array` instead. Values are now exact
up to `Number.MAX_SAFE_INTEGER`.
Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66094
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
When "vfsArchive" names a ZIP archive in the SEA configuration (with
"useVfs": true), --build-sea embeds the archive verbatim as one
reserved asset, and at runtime the SEA virtual file system mounts the
existing ZipProvider over a zero-copy view of the embedded archive
instead of the SEAProvider. The main script is injected into the
in-memory archive index as a stored entry, so the mounted tree looks
the same as with "assets".
The archive can be produced with any ZIP tool or with the ZIP support
in node:zlib; no ZIP serialization logic is added to the build side.
This trades asset read speed (entries are inflated when opened) for a
substantially smaller executable when the assets are compressible.
Also make FindSingleExecutableResource() return the deserialized
SeaResource by reference instead of by value: the copy included the
whole assets map, and getAsset() calls it once per asset read, so
every fs operation served by the SEA virtual file system paid a cost
linear in the number of bundled assets - with 8192 assets, reading
each of them took seconds instead of milliseconds.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/65810
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
`inspector.Session#connectToMainThread()` in a Worker aborted the
process on `CHECK_NOT_NULL(parent_handle_)` in
`Agent::ConnectToMainThread()` when the parent Environment was created
with `kNoCreateInspector`, as embedders that run their own inspector
(or none) do.
Throw the new `ERR_INSPECTOR_NOT_AVAILABLE` in that case, next to the
existing throw for a Worker whose own inspector is not initialized.
Refs: https://github.com/nodejs/node/pull/35025
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65976
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add `Buffer.stringLength(input[, encoding])`, the counterpart of
`Buffer.byteLength()`: it returns the number of UTF-16 code units that
`buf.toString(encoding)` would produce, without decoding.
For UTF-8 the count is computed with simdutf. Invalid input is counted
with the same maximal-subpart replacement that the decoder applies, so
the result always matches `toString().length`. The other encodings are
computed from `byteLength` alone.
This lets code that accumulates streamed input check the result against
`buffer.constants.MAX_STRING_LENGTH` and size its memory budget before
decoding.
Refs: https://github.com/nodejs/node/issues/66062
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/66064
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Requiring `node:trace_events` and calling `createTracing()` aborted
the process, and `getEnabledCategories()` dereferenced a null pointer,
when Node.js runs on an embedder's own platform
(`kNoInitializeNodeV8Platform`): no `tracing::Agent` exists then, and
`lib/trace_events.js` only checked the compile-time `hasTracing` flag
and `ownsProcessState` before handing the null agent to the binding.
Have the binding report whether an agent exists and throw the existing
`ERR_TRACE_EVENTS_UNAVAILABLE` when it does not, as for a
`--without-v8-platform` build.
Refs: https://github.com/nodejs/node/pull/19803
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65954
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
getDOMStorageItems() falls back to reading the live Storage object when
its cached map is empty. When getWebStorage() cannot produce one, the
fallback was skipped and the command still answered with Success and an
empty entries array, so a frontend could not tell an empty store from a
store that cannot be read.
getWebStorage() returns nothing whenever globalThis.localStorage is
missing or is not an object, which is the case for any process started
without --localstorage-file.
Return a ServerError in that case, matching how the other failure paths
in the same command report problems.
Reading the storage key in that same configuration aborted the process:
localstorage_file is an empty string, and std::filesystem::absolute()
throws on it under libstdc++ while returning the current directory
under libc++. Node builds without exceptions, so the throw terminated
the process. Use the error_code overloads of absolute() and
weakly_canonical() and report the failure through DispatchResponse.
Fixes: https://github.com/nodejs/node/issues/65895
Signed-off-by: Avocado <ujubongbong@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65973
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
When no filter is given, cpSync copies directory contents in C++. That
loop recreated every symlink it found, consulting dereference only for
the subdirectory-of-self guards, so a symlink nested in the tree was
copied as a link even with dereference set. Only a symlink passed as src
was dereferenced, because that one is resolved by stat() in JavaScript
before the C++ copy starts.
The directory and regular file branches already follow symlinks, so
links that resolve to those types can fall through to them. A link whose
target cannot be reached has nothing to copy, and uv_fs_stat() reports
the underlying filesystem error.
When following a link, preserve the existing force and errorOnExist
behavior for occupied destinations. Under force, remove an existing
destination first so copying does not write through a destination
symlink.
Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65731
Fixes: https://github.com/nodejs/node/issues/59168
Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ValidateDatabasePath() treats any object exposing a string `href` as a
URL, then asserts the parse result with CHECK(ada::can_parse(location)).
Whether that string parses depends on user input rather than on an
invariant the code guarantees, so a value such as { href: 'not a url' }
aborted the process instead of throwing.
Replace the CHECK with an ERR_INVALID_URL exception, matching how
node_file.cc reports an unparsable URL. Both DatabaseSync() and backup()
validate their path through this function, so both paths are covered.
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Assisted-by: Claude Code
PR-URL: https://github.com/nodejs/node/pull/66026
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Mounting a virtual file system requires calling vfs.mount() from inside
the program, so a program cannot itself be served from one: something
already running has to mount the VFS first.
Add two startup flags. --vfs-mount=<source> mounts a directory or an
archive as a virtual file system, and may be repeated.
--vfs-load=<source> mounts that source the same way and additionally
runs the entry point and all subsequent require()/import resolution
against it rather than against the real file system, so an application
can be run straight out of a directory or a ZIP archive:
node --experimental-vfs --vfs-load=my-app.zip
Both options append to one list, so mounts happen in the order written:
node --experimental-vfs --vfs-mount=a --vfs-load=b --vfs-mount=c
mounts a, b and c in that order and runs b. Mounting the same source
twice mounts it twice, at two mount points, and the entry point comes
from the mount --vfs-load contributed rather than the earlier one.
The provider backing a source is chosen from the source itself rather
than from its name: a directory is served by RealFSProvider and a file
whose bytes are a ZIP archive by ZipProvider, so an archive can carry
any extension. vfs.registerProvider() registers a provider for formats
there is no built-in for; selection is deferred until -r and --import
preloads have run, so a preloaded module can register one before its
source is claimed.
The entry point comes from the mount the way `node <directory>` takes
one, from package.json "main" or index.js. Nothing is consumed as an
entry point argument, so every positional reaches the program unchanged
from argv[2] on, and argv[1] reports the named source rather than the
reserved mount point, which is an opaque implementation detail.
--vfs-load may be given at most once, and is not permitted in
NODE_OPTIONS: which entry point runs is the command line's decision, and
NODE_OPTIONS is parsed first, so an environment variable could otherwise
redirect any invocation. --vfs-mount is permitted there. Workers inherit
the same mounts in the same order but not the loading: a worker mounts
what the parent mounted and runs its own entry point.
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
PR-URL: https://github.com/nodejs/node/pull/65748
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Provider-backed keys can have no numeric OpenSSL ID. Identify and
construct asymmetric keys by algorithm name, replacing the custom PQC
name-to-NID substitution with provider-aware matching in ncrypto.
Centralize known algorithm names, public key-type names, capabilities,
and backend compatibility in ncrypto. Use named key generation jobs and
remove asymmetric EVP_PKEY constants from the internal JavaScript
binding.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/65966
Reviewed-By: James M Snell <jasnell@gmail.com>
Two Environments with default flags alive at the same time (for
example the embedding.md example run on two threads, or two
`CommonEnvironmentSetup`s) aborted the process: `Agent::Start()` bound
one file-level static `uv_async_t` to the current Environment's loop
for every Environment with `kOwnsInspector`, which `kDefaultFlags`
implies, and CHECKed that nobody else had. Environments created one
after another did not abort, but each ran `StartDebugSignalHandler()`
again, which re-initialized the semaphore the watchdog waits on and
spawned another detached watchdog thread, leaking one thread per
Environment.
Give every Agent that asks for the debug signal handler its own async
handle, keep those Agents in a mutex-protected list that the watchdog
(or the Windows remote thread) walks, and set the watchdog up once per
process while still unblocking SIGUSR1 on each Environment's thread.
The handle is heap-allocated, closed by the cleanup hook or `~Agent()`,
whichever runs first, and freed by its close callback. A SIGUSR1 now
reaches every Environment that asked for the handler, and no longer
starts the inspector of one that passed `kNoStartDebugSignalHandler`.
Refs: https://github.com/nodejs/node/pull/25777
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65877
Refs: https://github.com/nodejs/node/pull/44121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The FFI fast-call wrappers key per-function metadata on raw FFI
functions using two per-isolate Symbols (kFastArguments /
kFastBufferInvoke) that were declared in src/env_properties.h.
Everything in env_properties.h is allocated while the startup
snapshot is built, so each Symbol advances the isolate's
identity-hash RNG before Object.prototype / Function.prototype
receive their snapshot identity hashes. In the snapshot produced
for Node 26.4.0+ this shifted those hashes so a function map (a
function whose `length` was redefined) and a plain-object map (an
object literal with an accessor) collide in V8's 64-slot
NormalizedMapCache. Every store into such objects then misses
the inline cache, and the repro reported in the linked issue is
roughly 7x slower.
Create the two Symbols lazily in the FFI binding's Initialize, on
the first run of internalBinding('ffi') at runtime, instead of
declaring them in env_properties.h. They are therefore not
allocated during snapshot serialization and no longer bias the
snapshot's prototype identity hashes. Their export, property
layout, and the fast-call feature behavior are unchanged.
Refs: https://github.com/nodejs/node/issues/66011
Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
Assisted-by: Pi
PR-URL: https://github.com/nodejs/node/pull/66015
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The operating system's dynamic loader cannot open a library that lives
in a mounted virtual file system: the reserved mount path has no real
inode. Native addons already handle this in require(): the loader
hands their bytes to process.dlopen(), which loads them from a
private, self-cleaning image - an anonymous in-memory memfd on Linux.
Make ffi.dlopen() and new DynamicLibrary() do the same transparently.
Mirroring the fs handler integration, the VFS hook installer sets a
library reader into node:ffi while at least one VFS is mounted and
clears it when the last one unmounts; DynamicLibrary consults it
before every load, so the dependency points from the VFS into ffi and
ffi never loads any VFS code. The reader hands the library's bytes to
the native constructor, which loads them from the same kind of image,
released right after the load, while library.path keeps reporting the
virtual path. Libraries on the real file system are unaffected and
load directly, and pay only a null check while no VFS is mounted.
The AddonImage materializer moves from an anonymous namespace in
node_binding.cc to node_binding.h so that node_ffi.cc can reuse it.
On Windows the image is now written and closed before the load,
because the loader shares read alone and a retained writable
delete-on-close handle failed the load with ERROR_SHARING_VIOLATION;
since a mapped image cannot be unlinked there, it is kept with the
module it loaded as and both are released at process exit. On POSIX
the image still never outlives the constructor call, so nothing is
left for dlclose() to clean up.
Also fix the VFS dlopen hook forwarding a missing flags argument as
undefined, which process.dlopen() coerces to 0 - not a valid
dlopen(2) mode - so loading any addon from the real file system
failed with EINVAL while a VFS was mounted.
Co-authored-by: Philipp Dunkel <pipobscure@users.noreply.github.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/65909
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
The current recordable `Histogram` does not maintain any sense
of time. Samples accumulate indefinitely as they are collected.
This implements a relatively simple sliding-window mechanism
that can be count or time based. The key benefit is that the
window remains fixed/bounded while samples are recorded.
The sliding window is chunk based. Internally, it maintains
a ring buffer of a fixed number of individual histograms.
When snapshot() is called, those are materialized into a
single combined histogram. As the window slides, older
chunks (and all of the samples they hold) are dropped from
the window so the window drops however many samples happened
to be in that chunk. This does mean that precision of the
window is determined by the chunk size.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
PR-URL: https://github.com/nodejs/node/pull/65825
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
When session.destroy() runs from a 'stream' handler, MakeCallback drains
nextTick while nghttp2 is still inside mem_recv. Close is deferred for
that window (see #64166), so later HEADERS in the same buffer created
C++ streams without a JS wrapper or onread, and DATA delivery aborted
with Assertion failed: onread->IsFunction().
- Reject new streams while the session is closing
- Destroy the C++ handle if on_headers runs after JS destroy
- Drop DATA when onread is not installed (defensive)
Fixes: https://github.com/nodejs/node/issues/64850
Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65116
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Previously, ClientRequest silently truncated responses that exceeded
request.maxHeadersCount while llhttp continued using omitted headers.
Reject these responses with HPE_HEADER_OVERFLOW so parser state and
exposed headers cannot diverge. This is a semver-major behavior change.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/65010
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
`node --snapshot-blob <file> main.js` aborted with an assertion when
the file was empty (`ReadFileSync()` insists on reading one item), was
not a Node.js snapshot (`CHECK_EQ(magic, kMagic)`) or had a zero-length
startup blob, and read past the end of the buffer when a snapshot was
truncated, because `BlobDeserializer` trusted every length field in the
blob. `EmbedderSnapshotData::FromFile()` is documented to return an
empty pointer for an invalid snapshot and crashed the same way.
Bounds-check each read in `BlobDeserializer` and record the failure,
have `SnapshotData::FromBlob()` print why and return false, and let
`ReadFileSync()` return an empty vector for an empty file. Also add the
missing space in the "built with Node.js version" messages.
Refs: https://github.com/nodejs/node/pull/38905
Refs: https://github.com/nodejs/node/pull/47933
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65955
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
A second Environment created from the same `IsolateData`, which
embedding.md allows and the cctest fixture does, aborted as soon as it
loaded `stream_wrap`, `tcp_wrap`, `pipe_wrap`, `tty_wrap`, `http2` or
the crypto key classes, i.e. on `require('net')`: those bindings
created their per-isolate templates from the per-Environment binding
initializer and stored them with a setter that CHECKs the slot is
empty. Templates that were already cached per isolate (ffi, sqlite,
dtls) instead hit "FunctionTemplate already instantiated" when
`SetConstructorFunction()` renamed them.
Build these templates once per isolate and reuse them, set class names
when the template is created, and store the `ShutdownWrap`, `WriteWrap`
and `Http2Stream` function templates rather than their instance
templates so the second Environment can expose the same constructors.
Refs: https://github.com/nodejs/node/pull/43802
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65978
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Implements quantile-respectful density estimate calcuation
on Histogram. Helps with tail-focused latency analysis
without retaining raw samples.
Baking this directly into Node.js, based on a 1 million
sample, 1k bin workload, this impl is roughly 150-325x
faster than performing the equivalent in Rscript.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
PR-URL: https://github.com/nodejs/node/pull/65806
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The CBOR importer accepts a normalizing_index_offset outside the range
of the allocated counts array. normalize_index() in hdr_histogram.c
applies at most one wrap adjustment of +/-counts_len, so an arbitrary
offset can remain out of bounds and a later record() performs an
out-of-bounds native write in counts_inc_normalised().
Reject offsets that are not representable as int32_t when reading the
CBOR field, and reject offsets outside [0, counts_len) after the
histogram is reconstructed. Add regression tests covering the boundary
at counts_len, offsets beyond it, and values that cannot be represented
as int32_t.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Assisted-by: pi
PR-URL: https://github.com/nodejs/node/pull/65950
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Return the private key, end-entity certificate, and any other
non-matching certificates from a PKCS#12 (.p12/.pfx) bundle as a
KeyObject and X509Certificate instances.
Node.js already parses PKCS#12 in SecureContext::LoadPKCS12, which backs
tls's `pfx` option, but the results are consumed directly into an
SSL_CTX and never reach JavaScript. Callers who need the key or the
certificates for anything other than an immediate TLS connection have to
shell out to `openssl pkcs12` or take a userland dependency.
SecureContext::LoadPKCS12 now uses this same parsing logic.
The binding wraps d2i_PKCS12_bio() and PKCS12_parse() and follows their
semantics, matching the existing TLS path: the first private key is
returned, the end-entity certificate is the one associated with that
key, and any remaining certificates are returned through
`additionalCertificates`. A bundle containing no private key reports
`certificate` as null and returns its certificates through
`additionalCertificates`.
Absent and empty passphrases are kept distinct, since OpenSSL treats
them differently. Bundles that require OpenSSL's legacy provider throw
ERR_CRYPTO_UNSUPPORTED_OPERATION, reusing the error added for the TLS
path.
Signed-off-by: bmuenzenmeyer <brian.muenzenmeyer@gmail.com>
Co-authored-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65627
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
libuv only assigns a pid to the process handle once uv_spawn() has
succeeded, so a child that never started keeps pid 0. Calling kill()
on such a child still reached uv_process_kill(), which ended up in
kill(0, signal) and signalled every process in the caller's own
process group, Node included.
Return ESRCH when the handle has no pid, and zero the pid in the
constructor so the check never reads an unassigned value.
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Shelley Vohr