This commit does two things:
- Reverses the boolean value returned by timeLogImpl(). The new
values make more sense semantically (IMO anyway), and save a
a single NOT operation.
- Explicitly check for undefined when calling _times.get()
instead of coercing the value.
PR-URL: https://github.com/nodejs/node/pull/29100
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
nghttp2 has updated its limit for outstanding Ping/Settings ACKs
to 1000. This commit allows reverting to the old default of 10000.
The associated CVEs are CVE-2019-9512/CVE-2019-9515.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If we are waiting for the ability to send more output, we should not
process more input. This commit a) makes us send output earlier,
during processing of input, if we accumulate a lot and b) allows
interrupting the call into nghttp2 that processes input data
and resuming it at a later time, if we do find ourselves in a position
where we are waiting to be able to send more output.
This is part of mitigating CVE-2019-9511/CVE-2019-9517.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If a write to the underlying socket finishes asynchronously, that
means that we cannot write any more data at that point without waiting
for it to finish. If this happens, we should also not be producing any
more input.
This is part of mitigating CVE-2019-9511/CVE-2019-9517.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is intended to mitigate CVE-2019-9518.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Allocating memory upfront comes with overhead, and in particular,
`std::vector` implementations do not necessarily return memory
to the system when one might expect that (e.g. after shrinking the
vector).
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Ignore headers with 0-length names and track memory for headers
the way we track it for other HTTP/2 session memory too.
This is intended to mitigate CVE-2019-9516.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Limit the number of invalid input frames, as they may be pointing
towards a misbehaving peer. The limit is currently set to 1000 but
could be changed or made configurable.
This is intended to mitigate CVE-2019-9514.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Limit the number of streams that are rejected upon creation. Since
each such rejection is associated with an `NGHTTP2_ENHANCE_YOUR_CALM`
error that should tell the peer to not open any more streams,
continuing to open streams should be read as a sign of a misbehaving
peer. The limit is currently set to 100 but could be changed or made
configurable.
This is intended to mitigate CVE-2019-9514.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Lazily allocate `ArrayBuffer`s for the contents of DATA frames.
Creating `ArrayBuffer`s is, sadly, not a cheap operation with V8.
This is part of performance improvements to mitigate CVE-2019-9513.
Together with the previous commit, these changes improve throughput
in the adversarial case by about 100 %, and there is little more
that we can do besides artificially limiting the rate of incoming
metadata frames (i.e. after this patch, CPU usage is virtually
exclusively in libnghttp2).
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
For some JS events, it only makes sense to call into JS when there
are listeners for the event in question.
The overhead is noticeable if a lot of these events are emitted during
the lifetime of a session. To reduce this overhead, keep track of
whether any/how many JS listeners are present, and if there are none,
skip calls into JS altogether.
This is part of performance improvements to mitigate CVE-2019-9513.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
DRY up the `debug()` calls, and in particular, avoid building template
strings before we know whether we need to.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This includes mitigations for CVE-2019-9512/CVE-2019-9515.
PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/29104
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/29091
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Currently, if this test is run as the root user the following
failure will occur:
=== release test-fs-access ===
Path: parallel/test-fs-access
(node:46733) internal/test/binding: These APIs are for internal testing
only. Do not use them.
Can't clean tmpdir: /root/node/test/.tmp.522
Files blocking: [ 'read_only_file', 'read_write_file' ]
/root/node/test/common/tmpdir.js:136
throw e;
^
Error: EACCES: permission denied, rmdir '/root/node/test/.tmp.522'
at Object.rmdirSync (fs.js:693:3)
at rmdirSync (/root/node/test/common/tmpdir.js:72:8)
at rimrafSync (/root/node/test/common/tmpdir.js:41:7)
at process.onexit (/root/node/test/common/tmpdir.js:121:5)
at process.emit (events.js:214:15) {
errno: -13,
syscall: 'rmdir',
code: 'EACCES',
path: '/root/node/test/.tmp.522'
}
Command: ./node --expose-internals test/parallel/test-fs-access.js
This commit adds a root user check and skips this test if running as the
user root.
PR-URL: https://github.com/nodejs/node/pull/29092
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
This commit changes the default message used by
ERR_BUFFER_OUT_OF_BOUNDS. Previously, the default
message implied that the problematic was always a
write, which is not accurate.
PR-URL: https://github.com/nodejs/node/pull/29098
Fixes: https://github.com/nodejs/node/issues/29097
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This will allow users to know how to change their project to support
ES modules.
PR-URL: https://github.com/nodejs/node/pull/28950
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Clarifies that creating multiple async iterators from the same stream
can lead to event listener leak.
PR-URL: https://github.com/nodejs/node/pull/28997
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
`conn.destroyed` is guaranteed to be `false` because a previous `if`
statement already handles the case where `conn && conn.destroyed`
evaluates to `true` returning `false` in that case.
PR-URL: https://github.com/nodejs/node/pull/29078
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
The description for the --link-module configuration option is as
follows:
$ ./configure --help | grep -A 5 'link-module'
--link-module=LINKED_MODULE
Path to a JS file to be bundled in the binary as a
builtin. This module will be referenced by path
without extension; e.g. /root/x/y.js will be
referenced via require('root/x/y'). Can be used
multiple times
This lead me to think that it was possible to specify a file like this:
$ ./configure --link-module=something.js
$ NODE_DEBUG=mkcodecache make -j8
This will lead to a compilation error as an entry in the source_ map in
node_javascript.cc will end up having an empty string as its key:
source_.emplace("", UnionBytes{_raw, 105});
This will then be used by CodeCacheBuilder when it iterates over the
module ids, which will lead to the following compilation errors:
/node/out/Release/obj/gen/node_code_cache.cc:12:23: warning:
ISO C++17 does not allow a decomposition group to be
empty [-Wempty-decomposition]
static const uint8_t [] = {
^
/node/out/Release/obj/gen/node_code_cache.cc:12:22: warning:
decomposition declarations are a C++17 extension [-Wc++17-extensions]
static const uint8_t [] = {
^~
/node/out/Release/obj/gen/node_code_cache.cc:12:1: error:
decomposition declaration cannot be declared 'static'
static const uint8_t [] = {
^~~~~~
/node/out/Release/obj/gen/node_code_cache.cc:12:22: error:
decomposition declaration cannot be declared with type 'const uint8_t'
(aka 'const unsigned char'); declared type must be 'auto' or
reference to 'auto'
static const uint8_t [] = {
^
/node/out/Release/obj/gen/node_code_cache.cc:12:22: error:
excess elements in scalar initializer
static const uint8_t [] = {
^
/node/out/Release/obj/gen/node_code_cache.cc:660:7: error:
expected expression
,
^
/node/out/Release/obj/gen/node_code_cache.cc:661:24: error:
no matching function for call to 'arraysize'
static_cast<int>(arraysize()), policy
^~~~~~~~~
../src/util.h:667:18: note: candidate function template not viable:
requires 1 argument, but 0 were provided
constexpr size_t arraysize(const T (&)[N]) {
^
2 warnings and 5 errors generated.
This commit suggests that passing a single file be allowed by modifying
tools/js2c.py.
PR-URL: https://github.com/nodejs/node/pull/28443
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
In the other .cc files in the project, includes are in alphabetical
order, with local files first, and libraries after. However,
inspector_profiler.cc has a library declared in the middle of the import
order, and v8 is the second to last being imported, instead of the last.
So I reordered the imports and testing showed no side effects;
everything passed.
PR-URL: https://github.com/nodejs/node/pull/29073
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
This commit simplifies the diagnostic report's code for listing
environment variables by using uv_os_environ() instead of
platform specific code.
PR-URL: https://github.com/nodejs/node/pull/28963
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Notable changes:
- UV_FS_O_FILEMAP has been added for faster access to memory
mapped files on Windows.
- uv_fs_mkdir() now returns UV_EINVAL for invalid filenames
on Windows. It previously returned UV_ENOENT.
- The uv_fs_statfs() API has been added.
- The uv_os_environ() and uv_os_free_environ() APIs have
been added.
Fixes: https://github.com/nodejs/node/issues/28599
Fixes: https://github.com/nodejs/node/issues/28945
Fixes: https://github.com/nodejs/node/issues/29008
PR-URL: https://github.com/nodejs/node/pull/29070
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Many benchmarks use test/common/tmpdir. This changes 3 benchmarks that
use NODE_TMPDIR to also use test/common/tmpdir.
This is necessary in preparation for the next commit that changes tmpdir
to delete tmpdir.path when the Node.js process exits. Thus, if multiple
benchmarks are run sequentially, the ones that use tmpdir will remove
the directory and the ones changed here would fail because it does not
exist. This happens when running test/benchmark.
Note: to explicitly select a directory for tmpdir, use NODE_TEST_DIR.
PR-URL: https://github.com/nodejs/node/pull/28858
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Tests can leave processes running blocking the tmpdir. This does not
yet prevent tests from doing that, but prevents failures on
subsequent tests.
PR-URL: https://github.com/nodejs/node/pull/28858
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The first argument to lookupService() should be an IP address,
and is named "address" in the documentation. This commit updates
the code to match the documentation and provide less confusing
errors.
PR-URL: https://github.com/nodejs/node/pull/29040
Fixes: https://github.com/nodejs/node/issues/29039
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
One skipped test remains, it creates very large Buffer objects,
triggering the AIX OOM to kill node and its parent processes.
See: https://github.com/nodejs/build/issues/1849#issuecomment-514414165
PR-URL: https://github.com/nodejs/node/pull/29054
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Inline and simplify onwritedrain. Also remove comment that seems to be
outdated/invalid.
PR-URL: https://github.com/nodejs/node/pull/29037
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
* Use `console.error()` for error or stderr output.
* Unify comment style.
* Unify link format.
* Correct link URL.
* Fix some typos.
PR-URL: https://github.com/nodejs/node/pull/29024
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
... before trying to valueOf them
PR-URL: https://github.com/nodejs/node/pull/29029
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
