We depend on V8's version of simdutf now.
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Original commit message:
[api] add Isolate::Deinitialize() and Isolate::Free()
This allows embedders to mirror the isolate disposal routine
with an initialization routine that uses Isolate::Allocate().
```
v8::Isolate* isolate = v8::Isolate::Allocate();
// Use the isolate address as a key.
v8::Isolate::Initialize(isolate, params);
isolate->Deinitialize();
// Remove the entry keyed by isolate address.
v8::Isolate::Free(isolate);
```
Previously, the only way to dispose the isolate bundles the
de-initialization and the freeing of the address together in
v8::Isolate::Dispose(). This is inadequate for embedders like
Node.js that uses the isolate address as a key to manage the
task runner associated with it, if another thread gets an
isolate allocated at the aligned address before the other
thread finishes cleanup for the isolate previously allocated
at the same address, and locking on the entire disposal can
be too risky since it may post GC tasks that in turn requires
using the isolate address to locate the task runner. It's a
lot simpler to handle the issue if the disposal process of
the isolate can mirror the initialization of it and split
into two routines.
Refs: https://github.com/nodejs/node/pull/57753#issuecomment-2818999420
Refs: https://github.com/nodejs/node/issues/30850
Bug: 412943769
Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6480071
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/main@{#99890}
Refs: 954187bb1b
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
It introduces process hangs on some platforms because Node.js doesn't
tear down V8 correctly.
Disable it while we work on a solution.
Refs: https://github.com/nodejs/node/issues/47297
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902
PR-URL: https://github.com/nodejs/node/pull/47450
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
illumos pointers are VA48, can allocate from the top of the 64-bit range
as well.
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
GCC emits warnings because of the trailing backslashes.
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/58070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/57940
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57857
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
An `std::string_view v` is a `const char* v.data()` along with an
`std::size_t v.size()` that guarantees that `v.size()` contiguous
elements of type `char` can be accessed relative to the pointer
`v.data()`.
One of the main reasons behind the existence of `std::string_view` is
the ability to operate on `char` sequences without requiring null
termination, which otherwise often requires expensive copies of strings
to be made. As a consequence, it is generally incorrect to assume that
`v.data()` points to a null-terminated sequence of `char`, and the only
way to obtain a null-terminated string from an `std::string_view` is to
make a copy. It is not even possible to check if the sequence pointed to
by `v.data()` is null-terminated because the null character would be at
position `v.data() + v.size()`, which is outside of the range that `v`
guarantees safe access to. (A default-constructed `std::string_view`
even sets its own data pointer to a `nullptr`, which is fine because it
only needs to guarantee safe access to zero elements, i.e., to no
elements).
In `deps/ncrypto` and `src/crypto`, there are various APIs that consume
`std::string_view v` arguments but then ignore `v.size()` and treat
`v.data()` as a C-style string of type `const char*`. However, that is
not what call sites would expect from functions that explicitly ask for
`std::string_view` arguments, since it makes assumptions beyond the
guarantees provided by `std::string_view` and leads to undefined
behavior unless the given view either contains an embedded null
character or the `char` at address `v.data() + v.size()` is a null
character. This is not a reasonable assumption for `std::string_view` in
general, and it also defeats the purpose of `std::string_view` for the
most part since, when `v.size()` is being ignored, it is essentially
just a `const char*`.
Constructing an `std::string_view` from a `const char*` is also not
"free" but requires computing the length of the C-style string (unless
the length can be computed at compile time, e.g., because the value is
just a string literal). Repeated conversion between `const char*` as
used by OpenSSL and `std::string_view` as used by ncrypto thus incurs
the additional overhead of computing the length of the string whenever
an `std::string_view` is constructed from a `const char*`. (This seems
negligible compared to the safety argument though.)
Similarly, returning a `const char*` pointer to a C-style string as an
`std::string_view` has two downsides: the function must compute the
length of the string in order to construct the view, and the caller
can no longer assume that the return value is null-terminated and thus
cannot pass the returned view to functions that require their arguments
to be null terminated. (And, for the reasons explained above, the caller
also cannot check if the value is null-terminated without potentially
invoking undefined behavior.)
C++20 unfortunately does not have a type similar to Rust's `CStr` or
GSL `czstring`. Therefore, this commit changes many occurrences of
`std::string_view` back to `const char*`, which is conventional for
null-terminated C-style strings and does not require computing the
length of strings.
There are _a lot_ of occurrences of `std::string_view` in ncrypto and
for each one, we need to evaluate if it is safe and a good abstraction.
I tried to do so, but I might have changed too few or too many, so
please feel free to give feedback on individual occurrences.
PR-URL: https://github.com/nodejs/node/pull/57816
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
And ignore them for future updates.
PR-URL: https://github.com/nodejs/node/pull/57835
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The `CipherBase` class assumes that any authentication tag will fit into
`EVP_GCM_TLS_TAG_LEN` bytes, which is true because Node.js only supports
GCM with AES as the blocker cipher, and the block size of AES happens to
be 16 bytes, which coincidentally is also the output size of the
Poly1305 construction used by ChaCha20-Poly1305 as well as the maximum
size of authentication tags produced by AES in CCM or OCB mode.
This commit adds a new constant `ncrypto::Cipher::MAX_AUTH_TAG_LENGTH`
which is the maximum length of authentication tags produced by
algorithms that Node.js supports and replaces some constants in
`CipherBase` with semantically more meaningful named constants.
The OpenSSL team is debating whether a constant like
`MAX_AUTH_TAG_LENGTH` (`EVP_MAX_AEAD_TAG_LENGTH`) should exist at all
since its value necessarily depends on the set of AEAD algorithms
supported, but I do believe that, for Node.js, this is a step in the
right direction. It certainly makes more sense than to use the AES-GCM
tag size as defined by TLS.
PR-URL: https://github.com/nodejs/node/pull/57803
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57801
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57792
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/56764
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: https://github.com/nodejs/node/pull/57023
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57693
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: https://github.com/nodejs/node/pull/57704
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57685
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
This commit adds a method for retrieving column metadata from
a prepared statement.
Fixes: https://github.com/nodejs/node/issues/57457
PR-URL: https://github.com/nodejs/node/pull/57490
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57455
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Steven R Loomis <srl295@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Original commit message:
Compact retained maps array more often
When we add maps to the retained maps array, we compacted the array if
it's full. But, since we are now adding maps in a batch, it's unlikely
to meet the condition. Thus, update the condition to check whether new
size exceeds the capacity.
Bug: 398528460
Change-Id: I89caa47b69532c6397596edfe5caf7c7d24768cc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6330019
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Choongwoo Han <choongwoo.han@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#99163}
Refs: c172ffc5bf
PR-URL: https://github.com/nodejs/node/pull/57437
Fixes: https://github.com/nodejs/node/issues/57412
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Co-Authored-By: tunamagur0
<47546832+tunamagur0@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/57429
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
* Use ncrypto APIs where appropriate
* Remove obsolete no-longer used functions
* Improve error handling a bit
* move secure heap handling to ncrypto
To simplify handling of boringssl/openssl, move secure
heap impl to ncrypto. Overall the reduces the complexity
of the code in crypto_util by eliminating additional
ifdef branches.
* simplify CryptoErrorStore::ToException a bit
* simplify error handling in crypto_common
* move curve utility methods to ncrypto
* verify that released DataPointers aren't on secure heap
The ByteSource does not currently know how to free a DataPointer
allocated on the secure heap, so just verify.
DataPointers on the secure heap are not something that users can
allocate on their own. Their use is rare. Eventually ByteSource
is going to be refactored around ncrypto APIs so these additional
checks should be temporary.
* simplify some ifdefs that are covered by ncrypto
* cleanup some obsolete includes in crypto_util
PR-URL: https://github.com/nodejs/node/pull/57323
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
An eventual goal for ncrypto is to completely abstract away
details of working directly with openssl in order to make it
easier to work with multiple different openssl/boringssl versions.
As part of that we want to move away from direct reliance on
specific openssl APIs in the runtime and instead go through
the ncrypto abstractions. Not only does this help other
runtimes trying to be compatible with Node.js, but it helps
Node.js also by reducing the complexity of the crypto code
in Node.js itself.
PR-URL: https://github.com/nodejs/node/pull/57300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/57335
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57335
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57265
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
