v20.7.0 proposal by UlisesGascon · Pull Request #49592 · nodejs/node

@rluvaton @UlisesGascon

PR-URL: #48787
Backport-PR-URL: #49225
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

@rluvaton @UlisesGascon

PR-URL: #48877
Backport-PR-URL: #49225
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

@rluvaton @UlisesGascon

fix #48475

PR-URL: #48915
Backport-PR-URL: #49225
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

@rluvaton @UlisesGascon

PR-URL: #48925
Backport-PR-URL: #49225
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

@rluvaton @UlisesGascon

PR-URL: #48913
Backport-PR-URL: #49225
Fixes: #48867
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

@rluvaton @UlisesGascon

no longer needed after #48915 fix

PR-URL: #48989
Backport-PR-URL: #49225
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

@cjihrig @UlisesGascon

This commit adds each test's line and column number to the reporter
output. This will aid in debugging test suite failures when error
stacks are not helpful, test suites are large, or tests have the
same name. This data is also exposed on the spec reporter.

This commit also replaces the filename that was previously being
reported, with the filename where the test actually exists. These
are normally correct, but could be wrong if tests were run from
a file other than the user's entrypoint.

PR-URL: #48975
Backport-PR-URL: #49225
Fixes: #48457
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

@cjihrig @UlisesGascon

This commit reverts the revert in
bb52656. It also includes the
fix for the issue that required the revert
(#49059 (comment))
and an additional common.mustCall() in the added test.

Refs: #49059
Refs: #49110
PR-URL: #49116
Backport-PR-URL: #49225
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

@RafaelGSS @UlisesGascon

@joyeecheung @UlisesGascon

We previously only return startup data for the first slot for
BaseObjects because we can already serialize all the necessary
information in one go, but slots that do not get special startup
data would be serialized verbatim which means that the pointer
addresses are going to be part of the snapshot blob, resulting
in indeterminism.

This patch updates the serialization routines and capture information
for both of the two slots - the first slot with type information
and memory management type (which we can use in the future for
cppgc-managed objects) and the second slot with data about the
object itself. This way the embeedder slots can be serialized
in a reproducible manner in the snapshot.

PR-URL: #48996
Refs: nodejs/build#3043
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

@michalbiesek @UlisesGascon

Signed-off-by: Michal Biesek <michalbiesek@gmail.com>
PR-URL: #49106
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@mhdawson @UlisesGascon

Fix warning about dereferencing null env

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48954
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@tniessen @UlisesGascon

getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires
some careful justification but the default encoding can be eliminated
from sig.js entirely.

In Sign.prototype.update, we can safely remove the conditional
assignment of getDefaultEncoding() to encoding. This is because
SignUpdate() in crypto_sig.cc internally calls node::crypto::Decode,
which returns UTF8 for falsy encoding values. In other words, with the
conditional assignment, StringBytes::Write() ultimately receives the
encoding BUFFER, and without the conditional assignment, it receives the
encoding UTF8. However, StringBytes::Write() treats both encodings
identically, so there is no need to deviate from the internal default
encoding UTF8.

In Sign.prototype.sign, we can also safely remove the conditional
assignment of getDefaultEncoding() to encoding. Whether encoding is
falsy or 'buffer' makes no difference.

In Verify.prototype.verify, we can also safely remove the conditional
assignment of getDefaultEncoding() to sigEncoding. This is because the
function passes the sigEncoding to getArrayBufferOrView(), which passes
it to Buffer.from(). If sigEncoding is 'buffer', getArrayBufferOrView()
instead passes 'utf8' to Buffer.from(). Because the default encoding of
Buffer.from() is 'utf8', passing a falsy encoding to
getArrayBufferOrView() instead of 'buffer' results in the same behavior.

Refs: #47182
PR-URL: #49145
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@joyeecheung @UlisesGascon

This version avoids the additional access to the embedder slot
when we already have a reference to the realm.

PR-URL: #49007
Refs: #48836
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

@joyeecheung @UlisesGascon

This reduce the number of embedder slot accesses and also removes
the assumption in a few binding methods that the current realm is
the principal realm of the current environment (which is not true
for shadow realms).

PR-URL: #49007
Refs: #48836
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

@joyeecheung @UlisesGascon

These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: #49020
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@joyeecheung @UlisesGascon

..and replace the similar code added for logging.

PR-URL: #49020
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@tniessen @UlisesGascon

This (not particularly elegant) native addon tests the effect of
UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more
than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits
the number of concurrent async tasks to anything less than
UV_THREADPOOL_SIZE.

PR-URL: #49165
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

@tniessen @UlisesGascon

getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires
some careful justification but the default encoding can be eliminated
from hash.js entirely. The reasoning is almost identical with that in
#49145 so I won't repeat it here.

Refs: #47182
PR-URL: #49167
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

@tniessen @UlisesGascon

getDefaultEncoding() always returns 'buffer' in Node.js 20.

In diffiehellman.js, this value is always used as input to either
toBuf(), encode(), or getArrayBufferOrView(). All of these functions
treat any falsy encoding just like 'buffer', so we can safely remove the
calls to getDefaultEncoding().

Refs: #47182
PR-URL: #49169
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

@joyeecheung @UlisesGascon

Previously we assume that the objects are GC'ed after one
global.gc() returns, which is not necessarily always the case. Use
gcUntil() to run GC multiple times if they are not GC'ed in the
first time around.

PR-URL: #49053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

@joyeecheung @UlisesGascon

The C++ implementation can now be done entirely in JS using WeakRef.
Re-implement it in JS instead to simplify the code.

PR-URL: #49053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

@joyeecheung @UlisesGascon

PR-URL: #49053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

@MoLow @UlisesGascon

PR-URL: #49199
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

@joyeecheung @UlisesGascon

Previously the ContextifyContext holds a MicrotaskQueueWrap which in
turns holds a MicrotaskQueue in a shared pointer. The indirection is
actually unnecessary, we can directly hold the MicrotaskQueue via
a unique pointer in ContextifyContext, the lifetime would still
remain the same but the graph would be simpler, and this removes
the additional JS -> C++ to create the wrapper object.

PR-URL: #48982
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

@sapphi-red @UlisesGascon

Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: #46304
PR-URL: #46391
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

@joyeecheung @UlisesGascon

V8 now no longer supports serializing code cache in an isolate
with unfinalized read-only space. So guard the code cache regeneration
with the `is_building_snapshot()` flag. When the isolate is created
for snapshot generation, the code cache is going to be serialized
separately anyway, so there is no need to do it in the builtin loader.

PR-URL: #49108
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

@atlowChemi @UlisesGascon

Fixes: #48112
Ref: #48208
PR-URL: #49184
Refs: #48208
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

@atlowChemi @UlisesGascon

PR-URL: #49186
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

@Ceres6 @UlisesGascon

Support for a single comma separates list for allow-fs-* flags is
removed. Instead now multiple flags can be passed to allow multiple
paths.

Fixes: nodejs/security-wg#1039
PR-URL: #49047
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>