By default, the `readableWebStream` method of `FileHandle` returns
a ReadableStream that, when finished, does not close the underlying
FileHandle. This can lead to issues if the stream is consumed
without having a reference to the FileHandle to close after use.
This commit adds an `autoClose` option to the `readableWebStream`
method, which, when set to `true`, will automatically close the
FileHandle when the stream is finished or canceled.
The test modified in this commit demonstrates one of the cases where
this is necessary in that the stream is consumed by separate code than
the FileHandle which was being left to close the underlying fd when
it is garbage collected, which is a deprecated behavior.
PR-URL: https://github.com/nodejs/node/pull/58548
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58571
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
the REPL completion logic evaluates code, this is generally ok
but it can be problematic when there are objects with nested
properties since the code evaluation would trigger their potential
getters (e.g. the `obj.foo.b` line would trigger the getter of
`foo`), the changes here disable the completion logic when proxies
and getters are involved thus making sure that code evaluation does
not take place when it can potentially (and surprisingly to the user)
trigger side effectful behaviors
PR-URL: https://github.com/nodejs/node/pull/57909
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This change simplifies the AEAD implementation. Instead of storing the
authentication tag when the user calls `setAuthTag()` and passing it to
OpenSSL later in `MaybePassAuthTagToOpenSSL()`, the modified code
forwards it to OpenSSL from within `setAuthTag()` already, removing the
need to store it.
For clarity, I have also renamed the possible `AuthTagState` values to
better reflect the actual state of the authentication tag.
I assume that we did not originally do this due to issues with some
old versions of OpenSSL when reordering certain function calls, but even
with the recent additions I made to the relevant test (namely,
1ef99237f566c89247660b935667fbf4efa606ce and
53944c44629b651425f8d18c4d33f9bc99657d37), it seems to pass in both
OpenSSL 3 and OpenSSL 1.1.1 with this simplification.
PR-URL: https://github.com/nodejs/node/pull/58547
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
- Use ESM for top-level await
- Avoid concurrent child processes in the test
- Use `events.once` instead of Promise wrapper
- Assert directly on sets
PR-URL: https://github.com/nodejs/node/pull/58546
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
There are several motivation for removing this:
1. The implementation does not align with InternalModuleStat,
most noticably it does not namespace the path or convert
it to UTF-16 before using it with std::filesystem::path
on Windows which could crash on non-English locale.
2. It needs the Environment - if not for decoding the string,
at least for env->exec_path() to resolve the path for
namespacing - and therefore needs a handle to the Context
which requires a handle scope which actually makes the
fast API version slower than the normal binding.
For simplicity this just removes the fast API to fix the bug and
improve the performance.
PR-URL: https://github.com/nodejs/node/pull/58489
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Refs: https://datatracker.ietf.org/doc/html/rfc9113#section-5.3.1
PR-URL: https://github.com/nodejs/node/pull/58293
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Has been deprecated for six years. It's time to remove it.
PR-URL: https://github.com/nodejs/node/pull/58533
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Dario Piotrowicz <dario.piotrowicz@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Passing a callback to worker.terminate() has been deprecated
for about six years now. It's time to remove it.
PR-URL: https://github.com/nodejs/node/pull/58528
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Additional accessors on PerformanceEntry objects
have been deprecated in favor of the detail property
for a number of years now. This removes them.
PR-URL: https://github.com/nodejs/node/pull/58531
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
The `open()` method on fs read and write streams has been
deprecated for many years. It's time to remove it while
still allowing the open method to be monkeypatched since
that's still apparently a thing.
PR-URL: https://github.com/nodejs/node/pull/58529
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jason Zhang <xzha4350@gmail.com>
The `._channel` property has been deprecated for many years
now. It's time to remove it.
PR-URL: https://github.com/nodejs/node/pull/58527
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58524
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Deleting a null pointer has no effect. The check is not needed.
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58514
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58518
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Dario Piotrowicz <dario.piotrowicz@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58500
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Since priority signaling covers more than `http2Stream.priority`,
it makes more sense to rename the section.
There were also a few missing mentions in `http2.md`.
PR-URL: https://github.com/nodejs/node/pull/58504
Refs: https://github.com/nodejs/node/pull/58313
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Seven years is long enough to be deprecated.
PR-URL: https://github.com/nodejs/node/pull/58474
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Update sqlite Session to support Symbol.dispose and
move the definition of the dispose methods to c++ to
close the open TODO
PR-URL: https://github.com/nodejs/node/pull/58378
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58432
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If `--enable-fips` or `--force-fips` fails to be applied during
`ProcessFipsOptions()`, the node process might exit with
`ExitCode::kNoFailure` because `ERR_GET_REASON(ERR_peek_error())` can
return `0` since `ncrypto::testFipsEnabled()` does not populate the
OpenSSL error queue.
You can likely test this locally by running
node --enable-fips && echo $?
with the current `node` binary from the main branch if compiled without
support for FIPS.
As confirmed by the `XXX` comment here, which was added three years ago
in commit b6971602564fc93c536ad469947536b487c968ea, even if the error
queue was populated properly, the OpenSSL reason codes are not really
related to the Node.js `ExitCode` enumeration.
PR-URL: https://github.com/nodejs/node/pull/58379
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Obtain sourceURL magic comments via V8 API to avoid a second round of
extraction of magic comments.
Updates source map snapshot normalization to allow testing full path
names inside the test outputs.
PR-URL: https://github.com/nodejs/node/pull/58389
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
There is a CHECK_NOT_NULL check before dereferencing node_env on
line 710 in the "if" block, but there is no CHECK_NOT_NULL check before
dereferencing node_env on line 721. Maybe it makes sense to put
CHECK_NOT_NULL right after calling the Environment::GetCurrent function.
PR-URL: https://github.com/nodejs/node/pull/58459
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reduce some of the boilerplate code needed to access the
cppgc::AllocationHandle from the Node.js Environment.
PR-URL: https://github.com/nodejs/node/pull/58483
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58449
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
PR-URL: https://github.com/nodejs/node/pull/58469
Refs: https://github.com/nodejs/node/pull/35486
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58313
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
And add additional info for security releases.
PR-URL: https://github.com/nodejs/node/pull/58475
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
