It's been a bit of a code smell that we create these objects in
different conditional branches, effectively forcing ourselves to
duplicate logic between those branches.
This code predates `std::optional` being available to us, but
now that it is, it's the idiomatic way to resolve this issue.
(This commit also explicitly deletes move and copy members for
these classes; these had previously been omitted for brevity,
but adding them made me feel more confident that there is no
hidden copy operation added through this change.)
PR-URL: https://github.com/nodejs/node/pull/59960
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/59941
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This simplifies the proxy configuration handling code,
adds tests to make sure the proxy support works with IPv6
and throws correct errors for invalid proxy IPs.
Drive-by: remove useless properties from ProxyConfig
PR-URL: https://github.com/nodejs/node/pull/59894
Refs: https://github.com/nodejs/node/issues/57872
Reviewed-By: Aditi Singh <aditisingh1400@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The offset in the allocated memory was calculated in alloc and free,
this makes it a single constant so it only needs to be defined once.
PR-URL: https://github.com/nodejs/node/pull/57810
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
There already is an existing requirement to have matching calls of
`napi_async_init()` and `napi_async_destroy()`, so expecting users
of this API to manually hold onto the resource for the duration of
the `napi_async_context`'s lifetime is unnecessary.
Weak callbacks are generally useful for when a corresponding C++
object should be cleaned up when a JS object is gargbage-collected,
but that is not the case here.
PR-URL: https://github.com/nodejs/node/pull/59828
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This is a follow-up to 234c26cca3. The Node-API interface does
not allow us to enforce that values are stored in a specific location,
e.g. on the stack or not; however, V8 requires `Local<>` handles
to be stored on the stack.
To circumvent this restriction, we add the ability to the async handle
stack to store either `Local<>*` pointers or `Global<>*` pointers, with
Node-API making use of the latter.
PR-URL: https://github.com/nodejs/node/pull/59828
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Restore the Buffer.isBuffer() check to avoid unnecessary Buffer.from()
calls when the input is already a Buffer. This improves performance
by 30-50% for buffer-heavy UDP operations.
Includes benchmark test for fixBufferList function to verify the
performance improvements across different data types and chunk sizes.
PR-URL: https://github.com/nodejs/node/pull/59934
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Consider the default context A with a microtask queue QA, and a
context B with its own microtask queue QB.
Context B is constructed with vm.createContext(..., {microtaskMode:
"afterEvaluate"}). The evaluation in context B can be performed via
vm.Script or vm.SourceTextModule.
The standard (https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob)
dictates that, when resolving a {promise} with {resolution}, from any
context, the {then} method on {promise} should be called within a task
enqueued on the microtask queue from the context associated with {then}.
Specifically, after evaluating a script or module in context B, any
promises created within B, if later resolved within A, will result in a
task to be enqueued back onto QB, even long after we are done evaluating
any code within B.
This creates a challenge for users of node:vm in "afterEvaluate" mode.
In ContextifyScript::EvalMachine() and in ModuleWrap::Evaluate(), we
only drain the microtask queue QB a single time after running the script
or evaluating the module. After that point, the queue will not be
drained unless another script or module is evaluated in the same
context.
In the following scenario, prior to this patch, the log statement will
not be printed:
const microtaskMode = "afterEvaluate";
const context = vm.createContext({}, {microtaskMode});
const source = "";
const module = new vm.SourceTextModule(source, {context});
await module.link(() => null);
await module.evaluate();
console.log("NOT PRINTED");
Within `evaluate()`, there is this `await` statement:
await this[kWrap].evaluate(timeout, breakOnSigint)
Since the promise returned by ModuleWrap::Evaluate() is the top-level
capability for {module}, a promise created within B, V8 will enqueue a
task on QB. But since this is after the PerformCheckpoint() call in
ModuleWrap::Evaluate(), the task in QB is never run. In the meantime,
since QA is empty, the Node process simply exits (with a warning about
the unsettled promise, if it happened to be a top-level await).
While being unable to do `await module.evaluate()` is clearly a problem,
more generally, it is intended that in "afterEvaluate" mode, promises
created in the inner context cannot make progress if, and until, the
microtask queue of the inner context is checkpointed.
Therefore, to address this issue, the fix is narrow:
When the module has its own microtask queue, i.e. in "afterEvaluate"
mode, the inner-context promise returned by
v8::SourceTextModule::Evaluate() is first resolved to an outer-context
promise, then we checkpoint the microtask queue of the inner context,
then we return the outer-context promise we just built.
This ensures that in the statement `await this[kWrap].evaluate(...)`,
the promise returned can be resolved within the outer context, without
involving the microtask queue in the inner context.
Fixes: https://github.com/nodejs/node/issues/59541
Refs: https://issues.chromium.org/issues/441679231
Refs: https://groups.google.com/g/v8-dev/c/YIeRg8CUNS8/m/rEQdFuNZAAAJ
PR-URL: https://github.com/nodejs/node/pull/59801
Refs: https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This refactors internal validation helpers in `child_process` to use
the common validators in `lib/internal/validators.js` where possible.
This improves code consistency and maintainability.
PR-URL: https://github.com/nodejs/node/pull/59416
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Using a C++20 `concept` here makes `is_callable` much simpler
than relying on SFINAE. It is equivalent for function types,
`std::function`, lambdas, and classes with `operator()`,
regardless of argument or return types.
PR-URL: https://github.com/nodejs/node/pull/58169
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This is required to use HTTP/1 websockets on an HTTP/2 server, which is
fairly common as websockets over HTTP/2 is much less widely supported.
This was broken by the recent shouldUpgradeCallback HTTP/1 addition,
which wasn't correctly added to the corresponding allowHttp1 part of
the HTTP/2 implementation.
PR-URL: https://github.com/nodejs/node/pull/59924
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Otherwise the debug() calls would attempt to display it and throws
an error.
PR-URL: https://github.com/nodejs/node/pull/59905
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This patch makes the certificate pre-loading thread load the bundled
and extra certificates from the other thread as well.
PR-URL: https://github.com/nodejs/node/pull/59856
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch makes the off-thread loading lazy and only done when the
`tls` builtin is actually loaded by the application. Thus if the
application never uses tls, it would not get hit by the extra
off-thread loading overhead. paving the way to enable --use-system-ca
by default.
PR-URL: https://github.com/nodejs/node/pull/59856
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/59901
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/59901
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
If any depdencies actually need to run the scripts, the corresponding
script can add --no-ignore-scripts.
PR-URL: https://github.com/nodejs/node/pull/59914
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This pr introduces the support for tagged templates
And an LRU to cache the templates. We introduced a
new object called SqlTagStore that holds the ref
to Lru. This acts as the main object that allows
us to use tagged templates.
PR-URL: https://github.com/nodejs/node/pull/58748
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Previously, you could either add no 'upgrade' event handler, in which
case all upgrades were ignored, or add an 'upgrade' handler and all
upgrade attempts would effectively succeed and skip normal request
handling. This change adds a new shouldUpgradeCallback option to HTTP
servers, which receives the request details and returns a boolean that
controls whether the request should be upgraded.
PR-URL: https://github.com/nodejs/node/pull/59824
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Update `tools/make-v8.sh` so that it can work with either `gcc` or
`clang`.
Adds:
- clang settings when CC/CXX environment variables set to clang/clang++.
- Turns off warnings as errors.
Removes:
- goma settings.
- Machine specific settings (moved to Jenkins job configuration).
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5541431
PR-URL: https://github.com/nodejs/node/pull/59893
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
In the testing proxy server for proxy client tests, the proxy
client might have already closed the connection when the upstream
connection fails. In that case, there's no need for the proxy
server to inform the proxy client about the error.
PR-URL: https://github.com/nodejs/node/pull/59742
Fixes: https://github.com/nodejs/node/issues/59741
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/59891
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
The bind method uses ObjectDefineProperty that shows up
in flamegraphs. This changes it to avoid the utility.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/59867
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Adds the `signatureAlgorithm` property to a X509Certificate allowing
users to retrieve a string representing the algorithm used to sign the
certificate. This string is defined by the OpenSSL library.
Fixes: https://github.com/nodejs/node/issues/59103
PR-URL: https://github.com/nodejs/node/pull/59235
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: https://github.com/nodejs/node/pull/59880
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/59870
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use lookup table instead of regex for strings shorter than 10
characters to improve performance for common short header names
while maintaining compatibility.
PR-URL: https://github.com/nodejs/node/pull/59832
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Anna Henningsen