InternalCallbackScope looks up the Environment from the isolate two
times per call, inside async_context_frame::exchange, and it keeps the
prior async context frame in a v8::Global also when there is no frame,
that is the common case. Every call from native code into JS pays this:
MakeCallback, CallbackScope, AsyncWrap, Node-API.
Now the scope passes the Environment it already has, the option is read
with an inline accessor instead of copying the shared_ptr, and the
global handle is created only when the prior frame is not undefined.
benchmark/napi/make_callback, Node 26.3.0 built with and without this
change, Linux x64, 30 runs: from 202-208 ns to 155-159 ns per call.
Refs: https://github.com/nodejs/performance/issues/24
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66316
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Compare http.isValidHeaderName() and http.isValidHeaderValue() with
http.validateHeaderName() and http.validateHeaderValue() wrapped in
try/catch, for valid and invalid input, and for both 'strict' and
'relaxed' header value validation.
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66334
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Add non-throwing counterparts of http.validateHeaderName() and
http.validateHeaderValue() that return a boolean instead of throwing.
Rejecting an invalid header with the existing validators costs a few
microseconds, because an error object and its stack trace are created,
compared to ~20ns for the boolean check. Userland HTTP implementations
such as undici (fetch Headers, request options) therefore keep private
copies of the token and field-value tables from _http_common. These new
functions let them reuse the core implementation.
isValidHeaderValue() accepts an optional `httpValidation` option
('strict' or 'relaxed') that has the same meaning as the option of the
same name on http.createServer() and http.request().
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66334
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
The relative resolve cache was keyed by a concatenated
${parent.path}\x00${request} string, allocating a new key for every
require() call including fully cached ones. Key the cache by the
parent directory first (a Map keyed by the already-retained
module.path string) and then by the request (a dictionary object,
whose property access internalizes dynamically-constructed
specifiers). Faster on every measured workload shape and slightly
smaller in memory, since the concatenated keys are no longer
retained.
Signed-off-by: Sam Attard <sattard@anthropic.com>
PR-URL: https://github.com/nodejs/node/pull/63884
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
The compat response defers 'finish' and 'close' for a HEAD request
until response.end(), because the stream of a headers-only response
closes as soon as the headers are sent. The same deferral also applied
to a HEAD stream that closed before any response was sent, for example
when the client cancelled it or the session was destroyed. Nothing was
left to call end(), so the response never emitted 'close' and the abort
could not be observed on it.
Defer only once the headers were sent, and otherwise close the response
as for any other method. The writable side of a HEAD stream is finished
from the start, so 'finish' is emitted only after the headers were
sent, and an aborted HEAD response does not report success.
Assisted-by: Opus 5.5
Signed-off-by: Robert Nagy <ronagy@icloud.com>
PR-URL: https://github.com/nodejs/node/pull/66310
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The shared "no pending request" records in the writable stream were
`__proto__: null` literals, which V8 creates in dictionary mode. They
sit in inFlightWriteRequest, closeRequest and pendingAbortRequest
whenever nothing is pending, and their promise field is checked several
times per write, so those loads did a hash lookup on every write and
every pipe. They are now built as plain literals and get their null
prototype afterwards, which keeps them in fast mode.
The readable controllers also initialized their state slot with an
empty object that setup replaced immediately. That throwaway allocation
is gone, matching the writable and transform controllers.
Add a writable-write benchmark: nothing in benchmark/webstreams drove
WritableStreamDefaultWriter.write() directly.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/66230
Reviewed-By: Mattias Buelens <mattias@buelens.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
When --process-timeout expires, --report-on-process-timeout asked
every Worker for a subreport and waited without a time limit. A Worker
blocked in a synchronous native call never answers, so the watchdog
force-exited the process before the report was written. That left a
truncated, invalid JSON file, and the forced-exit message was glued
onto the "Writing Node.js report to file" line.
For reports triggered by --process-timeout, wait at most two seconds
for Worker subreports and leave out Worker threads that have not
responded by then. The subreport state is now shared with the interrupt
callbacks, so a Worker that answers late does not touch freed memory.
Other report triggers are unchanged.
Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5.5
PR-URL: https://github.com/nodejs/node/pull/66304
Fixes: https://github.com/nodejs/node/issues/66303
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Cache the per-message callback lookups on the parser's JS object instead
of retaining callback functions in strong v8::Global handles. A callback
that captures its parser can otherwise keep the parser alive.
Clear the cache when a parser is initialized or freed so reused parsers
can load replacement callbacks and idle parsers do not retain them.
Header field names remain non-internalized because they are supplied by
clients.
Assisted-by: pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/66152
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Necessarily semver-major.
When `--permission` is on, every env var not matched by
`--allow-env` is removed at startup. It takes names,
prefix patterns (`PREFIX_*`), or `*`, repeatable or
comma-sep'd.
There are a range of env vars that Node.js itself uses,
and a default range that are generally known to be safe
in common usage. These are never scrubbed. These include
things like `NODE_OPTIONS`, `NODE_EXTRA_CA_CERTS`, `PATH`,
`HOME`, etc. `NODE_ENV` is not in the defaults and must
be allowed explicitly.
Proxy vars (`HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`) are
also not in the defaults since they can carry credentials.
When `--use-env-proxy` or `NODE_USE_ENV_PROXY` is set and
any of them were removed, a single warning naming them is
emitted.
Env vars can be dropped at runtime after reading using
`permission.drop()`. This is a stronger protection than
using `process.env.FOO = undefined` because it will
scrub the env var also from the environment block.
On Linux, the removed entries are overwritten in the
initial environment block. fs reads of any other
process's /proc/<pid>/environ, ancestors included, are
denied regardless of `--allow-fs-read`. A process's own
is readable only with `--allow-env=*`. Symlinks are
resolved before the check so paths like
/dev/fd/../../<ppid>/environ are caught. The check only
canonicalizes paths that statfs() reports are on procfs.
On Windows, removal also clears the C runtime's copy
of the environ using _wputenv_s.
Reading a removed name returns undefined, warns once per
name, and publishes to a diagnostics channel.
Env file keys are allowed. If the user had reason to pass
in an env file the assumption is they meant to allow them.
File-source config (node.config.json and NODE_OPTIONS
from a .env file) can only narrow the allow list.
Embedders must call ScrubProcessEnvironment() themselves
on startup. This is left up to the embedder to determine
the exact timing but needs to be called before startup
actually happens.
Child processes are started with `--allow-env=*`. Those
either receive the explicit env they were started with
or only the env they inherit from the parent. Since the
parent process is scrubbed, and the child cannot read
any other process's /proc/<pid>/environ, it should never
see more than the parent can.
Main part of the impl was done by hand. Docs, tests,
verification pass, and cleanup nits were automated.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
PR-URL: https://github.com/nodejs/node/pull/66132
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Skip the %XX walk in unescapeBuffer when the input has no '%'.
Add a dedicated '&'/'=' scanner for the default parse path so
it does not build separator code arrays or run the multi-char
state machine.
Official benchmark/querystring/querystring-parse.js:
encodemany is about 38% faster, manyblankpairs about 17%,
encodelast about 10%, noencode about 8%.
Official querystring-unescapebuffer.js with no escapes is
about 36% faster.
Assisted-by: a closed-source coding agent
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/66175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
`isInt32()` accepts -0 because `-0 === (-0 | 0)`, but V8 does not
represent -0 as an Int32 value, so `Value::IsInt32()` rejects it. The
utf8 fast paths of `readFileSync()` and `writeFileSync()` hand the value
straight to the binding, which then took it for a path and aborted on
the null check.
Coerce -0 to 0 before the call, matching `getValidatedFd()` and the rest
of fs, where -0 is a valid way to name file descriptor 0.
Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65888
Fixes: https://github.com/nodejs/node/issues/65886
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Node.js GitHub Bot