From bf7b94f0b3f9f0abddf410d82e494e73bae87302 Mon Sep 17 00:00:00 2001 From: Mohammed Keyvanzadeh Date: Sun, 15 Oct 2023 18:44:57 +0330 Subject: [PATCH 01/87] tools: refactor checkimports.py - Use f-strings for formatting. - Use raw strings for regexes alongside f-strings. - Use a generator. - Remove unnecessary `else` clause. PR-URL: https://github.com/nodejs/node/pull/50011 Reviewed-By: Richard Lau Reviewed-By: Christian Clauss --- tools/checkimports.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tools/checkimports.py b/tools/checkimports.py index b94919e3cc47e4..ed003d8b6b98cd 100755 --- a/tools/checkimports.py +++ b/tools/checkimports.py @@ -8,9 +8,9 @@ import itertools def do_exist(file_name, lines, imported): - if not any(not re.match('using \w+::{0};'.format(imported), line) and - re.search('\\b{0}\\b'.format(imported), line) for line in lines): - print('File "{0}" does not use "{1}"'.format(file_name, imported)) + if not any(not re.match(fr'using \w+::{imported};', line) and + re.search(fr'\b{imported}\b', line) for line in lines): + print(f'File "{file_name}" does not use "{imported}"') return False return True @@ -27,18 +27,16 @@ def is_valid(file_name): usings.append(matches.group(1)) importeds.append(matches.group(2)) - valid = all([do_exist(file_name, lines, imported) for imported in importeds]) + valid = all(do_exist(file_name, lines, imported) for imported in importeds) sorted_usings = sorted(usings, key=lambda x: x.lower()) if sorted_usings != usings: - print("using statements aren't sorted in '{0}'.".format(file_name)) + print(f"using statements aren't sorted in '{file_name}'.") for num, actual, expected in zip(line_numbers, usings, sorted_usings): if actual != expected: - print('\tLine {0}: Actual: {1}, Expected: {2}' - .format(num, actual, expected)) + print(f'\tLine {num}: Actual: {actual}, Expected: {expected}') return False - else: - return valid + return valid if __name__ == '__main__': if len(sys.argv) > 1: From 943047e80015e2594965d5c3c49cc87953df0c06 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 16 Oct 2023 00:30:13 +0800 Subject: [PATCH 02/87] deps: V8: cherry-pick 25902244ad1a Original commit message: [api] add line breaks to the output of Message::PrintCurrentStackTrace Previously this prints the stack trace without line breaks and it can be difficult to read. This also affects --abort-on-uncaught-exception. This patch adds line breaks to the output to improve readability. Change-Id: I4c44b529f8c829329f784b0859b1d13c9ec56838 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4925009 Reviewed-by: Leszek Swirski Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#90360} Refs: https://github.com/v8/v8/commit/25902244ad1aa5ad7c7e5c85d6b0afae4e878536 PR-URL: https://github.com/nodejs/node/pull/50156 Reviewed-By: Jiawen Geng Reviewed-By: Debadree Chatterjee --- common.gypi | 2 +- deps/v8/src/execution/isolate.cc | 1 + deps/v8/test/cctest/test-api.cc | 46 ++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/common.gypi b/common.gypi index 071a27afc2fd59..4c62386f4e8fce 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.12', + 'v8_embedder_string': '-node.13', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/execution/isolate.cc b/deps/v8/src/execution/isolate.cc index e7232e5c1fd50d..cbe353e95f5584 100644 --- a/deps/v8/src/execution/isolate.cc +++ b/deps/v8/src/execution/isolate.cc @@ -2503,6 +2503,7 @@ void Isolate::PrintCurrentStackTrace(std::ostream& out) { for (int i = 0; i < frames->length(); ++i) { Handle frame(CallSiteInfo::cast(frames->get(i)), this); SerializeCallSiteInfo(this, frame, &builder); + if (i != frames->length() - 1) builder.AppendCharacter('\n'); } Handle stack_trace = builder.Finish().ToHandleChecked(); diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 6c80599b571793..fa6901159a2bd5 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -31,6 +31,7 @@ #include #include #include +#include #include #include "test/cctest/cctest.h" @@ -4926,6 +4927,51 @@ TEST(MessageGetSourceLine) { }); } +void GetCurrentStackTrace(const v8::FunctionCallbackInfo& args) { + std::stringstream ss; + v8::Message::PrintCurrentStackTrace(args.GetIsolate(), ss); + std::string str = ss.str(); + args.GetReturnValue().Set(v8_str(str.c_str())); +} + +THREADED_TEST(MessagePrintCurrentStackTrace) { + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + Local templ = ObjectTemplate::New(isolate); + templ->Set(isolate, "getCurrentStackTrace", + v8::FunctionTemplate::New(isolate, GetCurrentStackTrace)); + LocalContext context(nullptr, templ); + + v8::ScriptOrigin origin = v8::ScriptOrigin(isolate, v8_str("test"), 0, 0); + v8::Local script = v8_str( + "function c() {\n" + " return getCurrentStackTrace();\n" + "}\n" + "function b() {\n" + " return c();\n" + "}\n" + "function a() {\n" + " return b();\n" + "}\n" + "a();"); + v8::Local stack_trace = + v8::Script::Compile(context.local(), script, &origin) + .ToLocalChecked() + ->Run(context.local()) + .ToLocalChecked(); + + CHECK(stack_trace->IsString()); + v8::String::Utf8Value stack_trace_value(isolate, + stack_trace.As()); + std::string stack_trace_string(*stack_trace_value); + std::string expected( + "c (test:2:10)\n" + "b (test:5:10)\n" + "a (test:8:10)\n" + "test:10:1"); + CHECK_EQ(stack_trace_string, expected); +} + THREADED_TEST(GetSetProperty) { LocalContext context; v8::Isolate* isolate = context->GetIsolate(); From f3a9ea0bc4d3f502d9b22625d09fffe6d42e79f0 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 15 Oct 2023 21:31:08 +0300 Subject: [PATCH 03/87] stream: call helper function from push and unshift PR-URL: https://github.com/nodejs/node/pull/50173 Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy Reviewed-By: Benjamin Gruenbaum --- lib/internal/streams/readable.js | 193 ++++++++++++++++++++++--------- 1 file changed, 140 insertions(+), 53 deletions(-) diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js index f551053bf7b79c..7ceb83d3f20523 100644 --- a/lib/internal/streams/readable.js +++ b/lib/internal/streams/readable.js @@ -284,77 +284,164 @@ Readable.prototype[SymbolAsyncDispose] = function() { // similar to how Writable.write() returns true if you should // write() some more. Readable.prototype.push = function(chunk, encoding) { - return readableAddChunk(this, chunk, encoding, false); + debug('push', chunk); + + const state = this._readableState; + return (state[kState] & kObjectMode) === 0 ? + readableAddChunkPushByteMode(this, state, chunk, encoding) : + readableAddChunkPushObjectMode(this, state, chunk, encoding); }; // Unshift should *always* be something directly out of read(). Readable.prototype.unshift = function(chunk, encoding) { - return readableAddChunk(this, chunk, encoding, true); + debug('unshift', chunk); + const state = this._readableState; + return (state[kState] & kObjectMode) === 0 ? + readableAddChunkUnshiftByteMode(this, state, chunk, encoding) : + readableAddChunkUnshiftObjectMode(this, state, chunk); }; -function readableAddChunk(stream, chunk, encoding, addToFront) { - debug('readableAddChunk', chunk); - const state = stream._readableState; - let err; - if ((state[kState] & kObjectMode) === 0) { - if (typeof chunk === 'string') { - encoding = encoding || state.defaultEncoding; - if (state.encoding !== encoding) { - if (addToFront && state.encoding) { - // When unshifting, if state.encoding is set, we have to save - // the string in the BufferList with the state encoding. - chunk = Buffer.from(chunk, encoding).toString(state.encoding); - } else { - chunk = Buffer.from(chunk, encoding); - encoding = ''; - } +function readableAddChunkUnshiftByteMode(stream, state, chunk, encoding) { + if (chunk === null) { + state[kState] &= ~kReading; + onEofChunk(stream, state); + + return false; + } + + if (typeof chunk === 'string') { + encoding = encoding || state.defaultEncoding; + if (state.encoding !== encoding) { + if (state.encoding) { + // When unshifting, if state.encoding is set, we have to save + // the string in the BufferList with the state encoding. + chunk = Buffer.from(chunk, encoding).toString(state.encoding); + } else { + chunk = Buffer.from(chunk, encoding); } - } else if (chunk instanceof Buffer) { - encoding = ''; - } else if (Stream._isUint8Array(chunk)) { - chunk = Stream._uint8ArrayToBuffer(chunk); - encoding = ''; - } else if (chunk != null) { - err = new ERR_INVALID_ARG_TYPE( - 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk); } + } else if (Stream._isUint8Array(chunk)) { + chunk = Stream._uint8ArrayToBuffer(chunk); + } else if (chunk !== undefined && !(chunk instanceof Buffer)) { + errorOrDestroy(stream, new ERR_INVALID_ARG_TYPE( + 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk)); + return false; } - if (err) { - errorOrDestroy(stream, err); - } else if (chunk === null) { + + if (!(chunk && chunk.length > 0)) { + return canPushMore(state); + } + + return readableAddChunkUnshiftValue(stream, state, chunk); +} + +function readableAddChunkUnshiftObjectMode(stream, state, chunk) { + if (chunk === null) { state[kState] &= ~kReading; onEofChunk(stream, state); - } else if (((state[kState] & kObjectMode) !== 0) || (chunk && chunk.length > 0)) { - if (addToFront) { - if ((state[kState] & kEndEmitted) !== 0) - errorOrDestroy(stream, new ERR_STREAM_UNSHIFT_AFTER_END_EVENT()); - else if (state.destroyed || state.errored) - return false; - else - addChunk(stream, state, chunk, true); - } else if (state.ended) { - errorOrDestroy(stream, new ERR_STREAM_PUSH_AFTER_EOF()); - } else if (state.destroyed || state.errored) { - return false; - } else { - state[kState] &= ~kReading; - if (state.decoder && !encoding) { - chunk = state.decoder.write(chunk); - if (state.objectMode || chunk.length !== 0) - addChunk(stream, state, chunk, false); - else - maybeReadMore(stream, state); - } else { - addChunk(stream, state, chunk, false); - } + + return false; + } + + return readableAddChunkUnshiftValue(stream, state, chunk); +} + +function readableAddChunkUnshiftValue(stream, state, chunk) { + if ((state[kState] & kEndEmitted) !== 0) + errorOrDestroy(stream, new ERR_STREAM_UNSHIFT_AFTER_END_EVENT()); + else if (state.destroyed || state.errored) + return false; + else + addChunk(stream, state, chunk, true); + + return canPushMore(state); +} + +function readableAddChunkPushByteMode(stream, state, chunk, encoding) { + if (chunk === null) { + state[kState] &= ~kReading; + onEofChunk(stream, state); + + return false; + } + + if (typeof chunk === 'string') { + encoding = encoding || state.defaultEncoding; + if (state.encoding !== encoding) { + chunk = Buffer.from(chunk, encoding); + encoding = ''; } - } else if (!addToFront) { + } else if (chunk instanceof Buffer) { + encoding = ''; + } else if (Stream._isUint8Array(chunk)) { + chunk = Stream._uint8ArrayToBuffer(chunk); + encoding = ''; + } else if (chunk !== undefined) { + errorOrDestroy(stream, new ERR_INVALID_ARG_TYPE( + 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk)); + return false; + } + + if (!chunk || chunk.length <= 0) { state[kState] &= ~kReading; maybeReadMore(stream, state); + + return canPushMore(state); + } + + if (state.ended) { + errorOrDestroy(stream, new ERR_STREAM_PUSH_AFTER_EOF()); + + return false; + } + + if (state.destroyed || state.errored) { + return false; + } + + state[kState] &= ~kReading; + if (state.decoder && !encoding) { + chunk = state.decoder.write(chunk); + if (chunk.length === 0) { + maybeReadMore(stream, state); + + return canPushMore(state); + } } + addChunk(stream, state, chunk, false); + return canPushMore(state); +} + +function readableAddChunkPushObjectMode(stream, state, chunk, encoding) { + if (chunk === null) { + state[kState] &= ~kReading; + onEofChunk(stream, state); + + return false; + } + + if (state.ended) { + errorOrDestroy(stream, new ERR_STREAM_PUSH_AFTER_EOF()); + return false; + } + + if (state.destroyed || state.errored) { + return false; + } + + state[kState] &= ~kReading; + if (state.decoder && !encoding) { + chunk = state.decoder.write(chunk); + } + + addChunk(stream, state, chunk, false); + return canPushMore(state); +} + +function canPushMore(state) { // We can push more data if we are below the highWaterMark. // Also, if we have no data yet, we can stand some more bytes. // This is to work around cases where hwm=0, such as the repl. From 5df3d5abcc8cc1d7ae23daeea5435da75ec77181 Mon Sep 17 00:00:00 2001 From: Jungku Lee Date: Mon, 16 Oct 2023 05:28:23 +0900 Subject: [PATCH 04/87] tools: update comment in `update-uncidi.sh` and `acorn_version.h` PR-URL: https://github.com/nodejs/node/pull/50175 Fixes: https://github.com/nodejs/node/issues/50159 Refs: https://github.com/nodejs/node/pull/50165 Reviewed-By: Luigi Pinca Reviewed-By: Richard Lau Reviewed-By: Rafael Gonzaga --- src/acorn_version.h | 2 +- tools/dep_updaters/update-undici.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/acorn_version.h b/src/acorn_version.h index 8c71b2ece5aa80..7e0add93c19bf8 100644 --- a/src/acorn_version.h +++ b/src/acorn_version.h @@ -1,5 +1,5 @@ // This is an auto generated file, please do not edit. -// Refer to tools/update-acorn.sh +// Refer to tools/dep_updaters/update-acorn.sh #ifndef SRC_ACORN_VERSION_H_ #define SRC_ACORN_VERSION_H_ #define ACORN_VERSION "8.10.0" diff --git a/tools/dep_updaters/update-undici.sh b/tools/dep_updaters/update-undici.sh index 962557f7a0d185..e50a3b909b8adf 100755 --- a/tools/dep_updaters/update-undici.sh +++ b/tools/dep_updaters/update-undici.sh @@ -43,7 +43,7 @@ rm -f deps/undici/undici.js # update version information in src/undici_version.h cat > "$ROOT/src/undici_version.h" < Date: Mon, 16 Oct 2023 17:50:28 +0200 Subject: [PATCH 05/87] test: fix defect path traversal tests The test never actually tested what it claims to test because it did not properly insert separators before `..`. PR-URL: https://github.com/nodejs/node/pull/50124 Reviewed-By: Yagiz Nizipli Reviewed-By: Rafael Gonzaga --- test/fixtures/permission/fs-traversal.js | 6 +++--- test/parallel/test-permission-fs-traversal-path.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js index 18243edfeadf7e..87132177643c45 100644 --- a/test/fixtures/permission/fs-traversal.js +++ b/test/fixtures/permission/fs-traversal.js @@ -12,9 +12,9 @@ path.resolve = (s) => s; const blockedFolder = process.env.BLOCKEDFOLDER; const allowedFolder = process.env.ALLOWEDFOLDER; -const traversalPath = allowedFolder + '../file.md'; -const traversalFolderPath = allowedFolder + '../folder'; -const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md'); +const traversalPath = allowedFolder + '/../file.md'; +const traversalFolderPath = allowedFolder + '/../folder'; +const bufferTraversalPath = Buffer.from(traversalPath); const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath); { diff --git a/test/parallel/test-permission-fs-traversal-path.js b/test/parallel/test-permission-fs-traversal-path.js index 547cd81c77cf18..d618c3e4f79879 100644 --- a/test/parallel/test-permission-fs-traversal-path.js +++ b/test/parallel/test-permission-fs-traversal-path.js @@ -18,7 +18,7 @@ const tmpdir = require('../common/tmpdir'); const file = fixtures.path('permission', 'fs-traversal.js'); const blockedFolder = tmpdir.path; -const allowedFolder = tmpdir.resolve('subdirectory/'); +const allowedFolder = tmpdir.resolve('subdirectory'); const commonPathWildcard = path.join(__filename, '../../common*'); { From 1f40ca1b918b625c0dc70097b2aa84704bbb2361 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 12 Oct 2023 16:44:55 +0000 Subject: [PATCH 06/87] doc: update security release process - update security release process to reflect current way to ask for tweet to amplify security release blog posts. Signed-off-by: Michael Dawson PR-URL: https://github.com/nodejs/node/pull/50166 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Rafael Gonzaga --- doc/contributing/security-release-process.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/contributing/security-release-process.md b/doc/contributing/security-release-process.md index fd33f3ccbb5afd..a3a4224de95fea 100644 --- a/doc/contributing/security-release-process.md +++ b/doc/contributing/security-release-process.md @@ -120,7 +120,8 @@ The google groups UI does not support adding a CC, until we figure out a better way, forward the email you receive to `oss-security@lists.openwall.com` as a CC. -* [ ] Send a message to `#nodejs-social` in OpenJS Foundation slack +* [ ] Post in the [nodejs-social channel][] + in the OpenJS slack asking for amplication of the blog post. ```text Security release pre-alert: @@ -179,7 +180,8 @@ out a better way, forward the email you receive to For more information see: https://nodejs.org/en/blog/vulnerability/month-year-security-releases/ ``` -* [ ] Create a new issue in [nodejs/tweet][] +* [ ] Post in the [nodejs-social channel][] + in the OpenJS slack asking for amplication of the blog post. ```text Security release: @@ -238,5 +240,5 @@ The steps to correct CVE information are: [H1 CVE requests]: https://hackerone.com/nodejs/cve_requests [docker-node]: https://github.com/nodejs/docker-node/issues [email]: https://groups.google.com/forum/#!forum/nodejs-sec +[nodejs-social channel]: https://openjs-foundation.slack.com/archives/C0142A39BNE [nodejs/build]: https://github.com/nodejs/build/issues -[nodejs/tweet]: https://github.com/nodejs/tweet/issues From 47633ab086d2314750f57060da8e41a756191ee4 Mon Sep 17 00:00:00 2001 From: Shi Pujin Date: Tue, 17 Oct 2023 04:19:31 +0800 Subject: [PATCH 07/87] doc: add loong64 to list of architectures PR-URL: https://github.com/nodejs/node/pull/50172 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Michael Dawson --- doc/api/os.md | 5 +++-- doc/api/process.md | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/doc/api/os.md b/doc/api/os.md index a02726fb7c2eb1..550ae7659de5f5 100644 --- a/doc/api/os.md +++ b/doc/api/os.md @@ -50,8 +50,9 @@ added: v0.5.0 * Returns: {string} Returns the operating system CPU architecture for which the Node.js binary was -compiled. Possible values are `'arm'`, `'arm64'`, `'ia32'`, `'mips'`, -`'mipsel'`, `'ppc'`, `'ppc64'`, `'riscv64'`, `'s390'`, `'s390x'`, and `'x64'`. +compiled. Possible values are `'arm'`, `'arm64'`, `'ia32'`, `'loong64'`, +`'mips'`, `'mipsel'`, `'ppc'`, `'ppc64'`, `'riscv64'`, `'s390'`, `'s390x'`, +and `'x64'`. The return value is equivalent to [`process.arch`][]. diff --git a/doc/api/process.md b/doc/api/process.md index 5718c83e4fbcf4..f0ce10fa9f071a 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -873,8 +873,8 @@ added: v0.5.0 * {string} The operating system CPU architecture for which the Node.js binary was compiled. -Possible values are: `'arm'`, `'arm64'`, `'ia32'`, `'mips'`,`'mipsel'`, `'ppc'`, -`'ppc64'`, `'riscv64'`, `'s390'`, `'s390x'`, and `'x64'`. +Possible values are: `'arm'`, `'arm64'`, `'ia32'`, `'loong64'`, `'mips'`, +`'mipsel'`, `'ppc'`, `'ppc64'`, `'riscv64'`, `'s390'`, `'s390x'`, and `'x64'`. ```mjs import { arch } from 'node:process'; From 789372a0721ec0086c3292ad558b3e06124d272d Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Sat, 14 Oct 2023 17:04:43 -0500 Subject: [PATCH 08/87] stream: allow pass stream class to `stream.compose` PR-URL: https://github.com/nodejs/node/pull/50187 Fixes: https://github.com/nodejs/node/issues/50176 Reviewed-By: Moshe Atlow Reviewed-By: Robert Nagy Reviewed-By: Benjamin Gruenbaum --- lib/internal/streams/duplexify.js | 5 +++++ test/parallel/test-runner-run.mjs | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/internal/streams/duplexify.js b/lib/internal/streams/duplexify.js index 788bcb63242c38..2503b531519066 100644 --- a/lib/internal/streams/duplexify.js +++ b/lib/internal/streams/duplexify.js @@ -85,6 +85,11 @@ module.exports = function duplexify(body, name) { if (typeof body === 'function') { const { value, write, final, destroy } = fromAsyncGen(body); + // Body might be a constructor function instead of an async generator function. + if (isDuplexNodeStream(value)) { + return value; + } + if (isIterable(value)) { return from(Duplexify, value, { // TODO (ronag): highWaterMark? diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 3201da25f87a1d..0d7aa346409647 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -96,6 +96,16 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { assert.match(stringResults[1], /tests 1/); assert.match(stringResults[1], /pass 1/); }); + + it('spec', async () => { + const result = await run({ + files: [join(testFixtures, 'default-behavior/test/random.cjs')] + }).compose(spec).toArray(); + const stringResults = result.map((bfr) => bfr.toString()); + assert.match(stringResults[0], /this should pass/); + assert.match(stringResults[1], /tests 1/); + assert.match(stringResults[1], /pass 1/); + }); }); it('should be piped with tap', async () => { From 06b7724f1466745e85646bcd7731d8e25d1dfb30 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Mon, 16 Oct 2023 20:01:19 -0300 Subject: [PATCH 09/87] doc: add command to keep major branch sync PR-URL: https://github.com/nodejs/node/pull/50102 Reviewed-By: Richard Lau --- doc/contributing/releases.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/contributing/releases.md b/doc/contributing/releases.md index 04d005ad8258a8..1d3bd71194caf4 100644 --- a/doc/contributing/releases.md +++ b/doc/contributing/releases.md @@ -1207,6 +1207,20 @@ Notify the `@nodejs/npm` team in the release proposal PR to inform them of the upcoming release. `npm` maintains a list of [supported versions](https://github.com/npm/cli/blob/latest/lib/utils/unsupported.js#L3) that will need updating to include the new major release. +To keep the branch in sync until the release date, it can be as simple as +doing the following: + +> Make sure to check that there are no PRs with the label `dont-land-on-vX.x`. + +```bash +git checkout vN.x +git reset --hard upstream/main +git checkout vN.x-staging +git reset --hard upstream/main +git push upstream vN.x +git push upstream vN.x-staging +``` + ### Update `NODE_MODULE_VERSION` This macro in `src/node_version.h` is used to signal an ABI version for native From bac872cbd06adf5d9d4a43729686c1139c34f9a1 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Mon, 16 Oct 2023 20:01:31 -0300 Subject: [PATCH 10/87] doc: update release-stewards with last sec-release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50179 Refs: https://github.com/nodejs-private/node-private/issues/485 Reviewed-By: Richard Lau Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Michael Dawson --- doc/contributing/security-release-process.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/contributing/security-release-process.md b/doc/contributing/security-release-process.md index a3a4224de95fea..4cd9835a953e94 100644 --- a/doc/contributing/security-release-process.md +++ b/doc/contributing/security-release-process.md @@ -30,6 +30,7 @@ The current security stewards are documented in the main Node.js | RH and IBM | Michael | 2023-Feb-16 | | NearForm | Rafael | 2023-Jun-20 | | NearForm | Rafael | 2023-Aug-09 | +| NearForm | Rafael | 2023-Oct-13 | | Datadog | Bryan | | | IBM | Joe | | | Platformatic | Matteo | | From b6021ab8f6709f80fba35576acbbb7438861e621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vinicius=20Louren=C3=A7o?= <12551007+H4ad@users.noreply.github.com> Date: Tue, 17 Oct 2023 03:23:40 -0300 Subject: [PATCH 11/87] lib: reduce overhead of blob clone PR-URL: https://github.com/nodejs/node/pull/50110 Reviewed-By: Yagiz Nizipli Reviewed-By: Stephen Belanger Reviewed-By: Rafael Gonzaga Reviewed-By: Chengzhong Wu --- benchmark/blob/clone.js | 7 +++++-- lib/internal/blob.js | 19 +++++-------------- test/parallel/test-blob.js | 10 ++++++++++ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/benchmark/blob/clone.js b/benchmark/blob/clone.js index 7f66255ad9da0c..39890672fc66f8 100644 --- a/benchmark/blob/clone.js +++ b/benchmark/blob/clone.js @@ -7,14 +7,17 @@ const assert = require('assert'); const bench = common.createBenchmark(main, { n: [50e3], + bytes: [128, 1024, 1024 ** 2], }); let _cloneResult; -function main({ n }) { +function main({ n, bytes }) { + const buff = Buffer.allocUnsafe(bytes); + const blob = new Blob(buff); bench.start(); for (let i = 0; i < n; ++i) - _cloneResult = structuredClone(new Blob(['hello'])); + _cloneResult = structuredClone(blob); bench.end(n); // Avoid V8 deadcode (elimination) diff --git a/lib/internal/blob.js b/lib/internal/blob.js index a6801f3d7fba3b..3ebb1b8a84b50a 100644 --- a/lib/internal/blob.js +++ b/lib/internal/blob.js @@ -8,7 +8,6 @@ const { ObjectDefineProperty, ObjectSetPrototypeOf, PromiseReject, - ReflectConstruct, RegExpPrototypeExec, RegExpPrototypeSymbolReplace, StringPrototypeToLowerCase, @@ -200,7 +199,7 @@ class Blob { const length = this[kLength]; return { data: { handle, type, length }, - deserializeInfo: 'internal/blob:ClonedBlob', + deserializeInfo: 'internal/blob:Blob', }; } @@ -397,25 +396,18 @@ class Blob { } } -function ClonedBlob() { - return ReflectConstruct(function() { - markTransferMode(this, true, false); - }, [], Blob); -} -ClonedBlob.prototype[kDeserialize] = () => {}; - -function TransferrableBlob(handle, length, type = '') { +function TransferableBlob(handle, length, type = '') { markTransferMode(this, true, false); this[kHandle] = handle; this[kType] = type; this[kLength] = length; } -ObjectSetPrototypeOf(TransferrableBlob.prototype, Blob.prototype); -ObjectSetPrototypeOf(TransferrableBlob, Blob); +ObjectSetPrototypeOf(TransferableBlob.prototype, Blob.prototype); +ObjectSetPrototypeOf(TransferableBlob, Blob); function createBlob(handle, length, type = '') { - const transferredBlob = new TransferrableBlob(handle, length, type); + const transferredBlob = new TransferableBlob(handle, length, type); // Fix issues like: https://github.com/nodejs/node/pull/49730#discussion_r1331720053 transferredBlob.constructor = Blob; @@ -489,7 +481,6 @@ function createBlobFromFilePath(path, options) { module.exports = { Blob, - ClonedBlob, createBlob, createBlobFromFilePath, isBlob, diff --git a/test/parallel/test-blob.js b/test/parallel/test-blob.js index 04b0580202c171..cb9e2f239547f0 100644 --- a/test/parallel/test-blob.js +++ b/test/parallel/test-blob.js @@ -480,3 +480,13 @@ assert.throws(() => new Blob({}), { assert.ok(blob.slice(0, 1).constructor === Blob); assert.ok(blob.slice(0, 1) instanceof Blob); } + +(async () => { + const blob = new Blob(['hello']); + + assert.ok(structuredClone(blob).constructor === Blob); + assert.ok(structuredClone(blob) instanceof Blob); + assert.ok(structuredClone(blob).size === blob.size); + assert.ok(structuredClone(blob).size === blob.size); + assert.ok((await structuredClone(blob).text()) === (await blob.text())); +})().then(common.mustCall()); From a54179f0e0a1206bf9888cfcf262cb3888191e5a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 5 Oct 2023 03:27:11 +0200 Subject: [PATCH 12/87] vm: unify host-defined option generation in vm.compileFunction Set a default host-defined option for vm.compileFunction so that it's consistent with vm.Script. PR-URL: https://github.com/nodejs/node/pull/50137 Refs: https://github.com/nodejs/node/issues/35375 Reviewed-By: Geoffrey Booth Reviewed-By: Yagiz Nizipli Reviewed-By: Chengzhong Wu Reviewed-By: Antoine du Hamel --- lib/internal/vm.js | 25 ++++++++++++++++++- lib/vm.js | 12 +++------ src/node_contextify.cc | 20 ++++++--------- .../test-vm-no-dynamic-import-callback.js | 13 +++++++--- 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/lib/internal/vm.js b/lib/internal/vm.js index f348ef6d2d612f..a463bdec88bdd4 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -2,12 +2,16 @@ const { ArrayPrototypeForEach, + Symbol, } = primordials; const { compileFunction, isContext: _isContext, } = internalBinding('contextify'); +const { + default_host_defined_options, +} = internalBinding('symbols'); const { validateArray, validateBoolean, @@ -30,12 +34,27 @@ function isContext(object) { return _isContext(object); } +function getHostDefinedOptionId(importModuleDynamically, filename) { + if (importModuleDynamically !== undefined) { + // Check that it's either undefined or a function before we pass + // it into the native constructor. + validateFunction(importModuleDynamically, + 'options.importModuleDynamically'); + } + if (importModuleDynamically === undefined) { + // We need a default host defined options that are the same for all + // scripts not needing custom module callbacks so that the isolate + // compilation cache can be hit. + return default_host_defined_options; + } + return Symbol(filename); +} + function internalCompileFunction(code, params, options) { validateString(code, 'code'); if (params !== undefined) { validateStringArray(params, 'params'); } - const { filename = '', columnOffset = 0, @@ -72,6 +91,8 @@ function internalCompileFunction(code, params, options) { validateObject(extension, name, kValidateObjectAllowNullable); }); + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); const result = compileFunction( code, filename, @@ -82,6 +103,7 @@ function internalCompileFunction(code, params, options) { parsingContext, contextExtensions, params, + hostDefinedOptionId, ); if (produceCachedData) { @@ -113,6 +135,7 @@ function internalCompileFunction(code, params, options) { } module.exports = { + getHostDefinedOptionId, internalCompileFunction, isContext, }; diff --git a/lib/vm.js b/lib/vm.js index f134cdc983db6d..fe2981019a6170 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -41,7 +41,6 @@ const { const { validateBoolean, validateBuffer, - validateFunction, validateInt32, validateObject, validateOneOf, @@ -54,6 +53,7 @@ const { kVmBreakFirstLineSymbol, } = require('internal/util'); const { + getHostDefinedOptionId, internalCompileFunction, isContext, } = require('internal/vm'); @@ -86,12 +86,8 @@ class Script extends ContextifyScript { } validateBoolean(produceCachedData, 'options.produceCachedData'); - if (importModuleDynamically !== undefined) { - // Check that it's either undefined or a function before we pass - // it into the native constructor. - validateFunction(importModuleDynamically, - 'options.importModuleDynamically'); - } + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); // Calling `ReThrow()` on a native TryCatch does not generate a new // abort-on-uncaught-exception check. A dummy try/catch in JS land // protects against that. @@ -103,7 +99,7 @@ class Script extends ContextifyScript { cachedData, produceCachedData, parsingContext, - importModuleDynamically !== undefined); + hostDefinedOptionId); } catch (e) { throw e; /* node-do-not-add-exception-line */ } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 466c2d81c7e7a9..4c7988eaed7c9e 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -771,11 +771,11 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { bool produce_cached_data = false; Local parsing_context = context; - bool needs_custom_host_defined_options = false; + Local id_symbol; if (argc > 2) { // new ContextifyScript(code, filename, lineOffset, columnOffset, // cachedData, produceCachedData, parsingContext, - // needsCustomHostDefinedOptions) + // hostDefinedOptionId) CHECK_EQ(argc, 8); CHECK(args[2]->IsNumber()); line_offset = args[2].As()->Value(); @@ -795,9 +795,8 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(sandbox); parsing_context = sandbox->context(); } - if (args[7]->IsTrue()) { - needs_custom_host_defined_options = true; - } + CHECK(args[7]->IsSymbol()); + id_symbol = args[7].As(); } ContextifyScript* contextify_script = @@ -821,12 +820,6 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - // We need a default host defined options that's the same for all scripts - // not needing custom module callbacks for so that the isolate compilation - // cache can be hit. - Local id_symbol = needs_custom_host_defined_options - ? Symbol::New(isolate, filename) - : env->default_host_defined_options(); host_defined_options->Set( isolate, loader::HostDefinedOptions::kID, id_symbol); @@ -1199,6 +1192,10 @@ void ContextifyContext::CompileFunction( params_buf = args[8].As(); } + // Argument 10: host-defined option symbol + CHECK(args[9]->IsSymbol()); + Local id_symbol = args[9].As(); + // Read cache from cached data buffer ScriptCompiler::CachedData* cached_data = nullptr; if (!cached_data_buf.IsEmpty()) { @@ -1210,7 +1207,6 @@ void ContextifyContext::CompileFunction( // Set host_defined_options Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - Local id_symbol = Symbol::New(isolate, filename); host_defined_options->Set( isolate, loader::HostDefinedOptions::kID, id_symbol); diff --git a/test/parallel/test-vm-no-dynamic-import-callback.js b/test/parallel/test-vm-no-dynamic-import-callback.js index 3651f997598b21..35b553d587c6e4 100644 --- a/test/parallel/test-vm-no-dynamic-import-callback.js +++ b/test/parallel/test-vm-no-dynamic-import-callback.js @@ -1,7 +1,7 @@ 'use strict'; -require('../common'); -const { Script } = require('vm'); +const common = require('../common'); +const { Script, compileFunction } = require('vm'); const assert = require('assert'); assert.rejects(async () => { @@ -10,4 +10,11 @@ assert.rejects(async () => { await imported; }, { code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING' -}); +}).then(common.mustCall()); + +assert.rejects(async () => { + const imported = compileFunction('return import("fs")')(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING' +}).then(common.mustCall()); From 3999362c59d17dbc18fbe28ea9e1fe918310190a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 11 Oct 2023 04:50:50 +0200 Subject: [PATCH 13/87] vm: use internal versions of compileFunction and Script Instead of using the public versions of the vm APIs internally, use the internal versions so that we can skip unnecessary argument validation. The public versions would need special care to the generation of host-defined options to hit the isolate compilation cache when imporModuleDynamically isn't used, while internally it's almost always used, so this allows us to handle the host-defined options separately. PR-URL: https://github.com/nodejs/node/pull/50137 Refs: https://github.com/nodejs/node/issues/35375 Reviewed-By: Geoffrey Booth Reviewed-By: Yagiz Nizipli Reviewed-By: Chengzhong Wu Reviewed-By: Antoine du Hamel --- lib/internal/modules/cjs/loader.js | 73 +++++++------ lib/internal/modules/esm/translators.js | 35 ++++--- lib/internal/process/execution.js | 35 ++++--- lib/internal/vm.js | 131 +++++++++++++----------- lib/repl.js | 85 +++++++-------- lib/vm.js | 63 ++++++++++-- test/message/eval_messages.out | 14 +-- test/message/stdin_messages.out | 18 ++-- 8 files changed, 268 insertions(+), 186 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8be147c8e233ca..b3b438372fe9ed 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -52,6 +52,7 @@ const { SafeMap, SafeWeakMap, String, + Symbol, StringPrototypeCharAt, StringPrototypeCharCodeAt, StringPrototypeEndsWith, @@ -84,7 +85,12 @@ const { setOwnProperty, getLazy, } = require('internal/util'); -const { internalCompileFunction } = require('internal/vm'); +const { + internalCompileFunction, + makeContextifyScript, + runScriptInThisContext, +} = require('internal/vm'); + const assert = require('internal/assert'); const fs = require('fs'); const path = require('path'); @@ -1240,7 +1246,6 @@ Module.prototype.require = function(id) { let resolvedArgv; let hasPausedEntry = false; /** @type {import('vm').Script} */ -let Script; /** * Wraps the given content in a script and runs it in a new context. @@ -1250,47 +1255,49 @@ let Script; * @param {object} codeCache The SEA code cache */ function wrapSafe(filename, content, cjsModuleInstance, codeCache) { + const hostDefinedOptionId = Symbol(`cjs:${filename}`); + async function importModuleDynamically(specifier, _, importAttributes) { + const cascadedLoader = getCascadedLoader(); + return cascadedLoader.import(specifier, normalizeReferrerURL(filename), + importAttributes); + } if (patched) { - const wrapper = Module.wrap(content); - if (Script === undefined) { - ({ Script } = require('vm')); - } - const script = new Script(wrapper, { - filename, - lineOffset: 0, - importModuleDynamically: async (specifier, _, importAttributes) => { - const cascadedLoader = getCascadedLoader(); - return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAttributes); - }, - }); + const wrapped = Module.wrap(content); + const script = makeContextifyScript( + wrapped, // code + filename, // filename + 0, // lineOffset + 0, // columnOffset + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); // Cache the source map for the module if present. if (script.sourceMapURL) { maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL); } - return script.runInThisContext({ - displayErrors: true, - }); + return runScriptInThisContext(script, true, false); } + const params = [ 'exports', 'require', 'module', '__filename', '__dirname' ]; try { - const result = internalCompileFunction(content, [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ], { - filename, - cachedData: codeCache, - importModuleDynamically(specifier, _, importAttributes) { - const cascadedLoader = getCascadedLoader(); - return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAttributes); - }, - }); + const result = internalCompileFunction( + content, // code, + filename, // filename + 0, // lineOffset + 0, // columnOffset, + codeCache, // cachedData + false, // produceCachedData + undefined, // parsingContext + undefined, // contextExtensions + params, // params + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); // The code cache is used for SEAs only. if (codeCache && diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 5627d98cf294d0..c36fe07a9503ae 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -15,6 +15,7 @@ const { StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeStartsWith, + Symbol, SyntaxErrorPrototype, globalThis: { WebAssembly }, } = primordials; @@ -192,19 +193,29 @@ function enrichCJSError(err, content, filename) { */ function loadCJSModule(module, source, url, filename) { let compiledWrapper; + async function importModuleDynamically(specifier, _, importAttributes) { + return asyncESM.esmLoader.import(specifier, url, importAttributes); + } try { - compiledWrapper = internalCompileFunction(source, [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ], { - filename, - importModuleDynamically(specifier, _, importAttributes) { - return asyncESM.esmLoader.import(specifier, url, importAttributes); - }, - }).function; + compiledWrapper = internalCompileFunction( + source, // code, + filename, // filename + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + undefined, // contextExtensions + [ // params + 'exports', + 'require', + 'module', + '__filename', + '__dirname', + ], + Symbol(`cjs:${filename}`), // hostDefinedOptionsId + importModuleDynamically, // importModuleDynamically + ).function; } catch (err) { enrichCJSError(err, source, url); throw err; diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 8ae6a1678af1b5..b8c507c798182e 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -1,6 +1,7 @@ 'use strict'; const { + Symbol, RegExpPrototypeExec, globalThis, } = primordials; @@ -25,7 +26,9 @@ const { emitAfter, popAsyncContext, } = require('internal/async_hooks'); - +const { + makeContextifyScript, runScriptInThisContext, +} = require('internal/vm'); // shouldAbortOnUncaughtToggle is a typed array for faster // communication with JS. const { shouldAbortOnUncaughtToggle } = internalBinding('util'); @@ -53,7 +56,6 @@ function evalModule(source, print) { function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { const CJSModule = require('internal/modules/cjs/loader').Module; - const { kVmBreakFirstLineSymbol } = require('internal/util'); const { pathToFileURL } = require('internal/url'); const cwd = tryGetCwd(); @@ -79,16 +81,25 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { `; globalThis.__filename = name; RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. - const result = module._compile(script, `${name}-wrapper`)(() => - require('vm').runInThisContext(body, { - filename: name, - displayErrors: true, - [kVmBreakFirstLineSymbol]: !!breakFirstLine, - importModuleDynamically(specifier, _, importAttributes) { - const loader = asyncESM.esmLoader; - return loader.import(specifier, baseUrl, importAttributes); - }, - })); + const result = module._compile(script, `${name}-wrapper`)(() => { + const hostDefinedOptionId = Symbol(name); + async function importModuleDynamically(specifier, _, importAttributes) { + const loader = asyncESM.esmLoader; + return loader.import(specifier, baseUrl, importAttributes); + } + const script = makeContextifyScript( + body, // code + name, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); + return runScriptInThisContext(script, true, !!breakFirstLine); + }); if (print) { const { log } = require('internal/console/global'); log(result); diff --git a/lib/internal/vm.js b/lib/internal/vm.js index a463bdec88bdd4..4f7b8c652f3c26 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -1,32 +1,26 @@ 'use strict'; const { - ArrayPrototypeForEach, + ReflectApply, Symbol, } = primordials; const { + ContextifyScript, compileFunction, isContext: _isContext, } = internalBinding('contextify'); +const { + runInContext, +} = ContextifyScript.prototype; const { default_host_defined_options, } = internalBinding('symbols'); const { - validateArray, - validateBoolean, - validateBuffer, validateFunction, validateObject, - validateString, - validateStringArray, kValidateObjectAllowArray, - kValidateObjectAllowNullable, - validateInt32, } = require('internal/validators'); -const { - ERR_INVALID_ARG_TYPE, -} = require('internal/errors').codes; function isContext(object) { validateObject(object, 'object', kValidateObjectAllowArray); @@ -50,49 +44,20 @@ function getHostDefinedOptionId(importModuleDynamically, filename) { return Symbol(filename); } -function internalCompileFunction(code, params, options) { - validateString(code, 'code'); - if (params !== undefined) { - validateStringArray(params, 'params'); - } - const { - filename = '', - columnOffset = 0, - lineOffset = 0, - cachedData = undefined, - produceCachedData = false, - parsingContext = undefined, - contextExtensions = [], - importModuleDynamically, - } = options; - - validateString(filename, 'options.filename'); - validateInt32(columnOffset, 'options.columnOffset'); - validateInt32(lineOffset, 'options.lineOffset'); - if (cachedData !== undefined) - validateBuffer(cachedData, 'options.cachedData'); - validateBoolean(produceCachedData, 'options.produceCachedData'); - if (parsingContext !== undefined) { - if ( - typeof parsingContext !== 'object' || - parsingContext === null || - !isContext(parsingContext) - ) { - throw new ERR_INVALID_ARG_TYPE( - 'options.parsingContext', - 'Context', - parsingContext, - ); - } - } - validateArray(contextExtensions, 'options.contextExtensions'); - ArrayPrototypeForEach(contextExtensions, (extension, i) => { - const name = `options.contextExtensions[${i}]`; - validateObject(extension, name, kValidateObjectAllowNullable); +function registerImportModuleDynamically(referrer, importModuleDynamically) { + const { importModuleDynamicallyWrap } = require('internal/vm/module'); + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(referrer, { + __proto__: null, + importModuleDynamically: + importModuleDynamicallyWrap(importModuleDynamically), }); +} - const hostDefinedOptionId = - getHostDefinedOptionId(importModuleDynamically, filename); +function internalCompileFunction( + code, filename, lineOffset, columnOffset, + cachedData, produceCachedData, parsingContext, contextExtensions, + params, hostDefinedOptionId, importModuleDynamically) { const result = compileFunction( code, filename, @@ -119,23 +84,65 @@ function internalCompileFunction(code, params, options) { } if (importModuleDynamically !== undefined) { - validateFunction(importModuleDynamically, - 'options.importModuleDynamically'); - const { importModuleDynamicallyWrap } = require('internal/vm/module'); - const wrapped = importModuleDynamicallyWrap(importModuleDynamically); - const func = result.function; - const { registerModule } = require('internal/modules/esm/utils'); - registerModule(func, { - __proto__: null, - importModuleDynamically: wrapped, - }); + registerImportModuleDynamically(result.function, importModuleDynamically); } return result; } +function makeContextifyScript(code, + filename, + lineOffset, + columnOffset, + cachedData, + produceCachedData, + parsingContext, + hostDefinedOptionId, + importModuleDynamically) { + let script; + // Calling `ReThrow()` on a native TryCatch does not generate a new + // abort-on-uncaught-exception check. A dummy try/catch in JS land + // protects against that. + try { // eslint-disable-line no-useless-catch + script = new ContextifyScript(code, + filename, + lineOffset, + columnOffset, + cachedData, + produceCachedData, + parsingContext, + hostDefinedOptionId); + } catch (e) { + throw e; /* node-do-not-add-exception-line */ + } + + if (importModuleDynamically !== undefined) { + registerImportModuleDynamically(script, importModuleDynamically); + } + return script; +} + +// Internal version of vm.Script.prototype.runInThisContext() which skips +// argument validation. +function runScriptInThisContext(script, displayErrors, breakOnFirstLine) { + return ReflectApply( + runInContext, + script, + [ + null, // sandbox - use current context + -1, // timeout + displayErrors, // displayErrors + false, // breakOnSigint + breakOnFirstLine, // breakOnFirstLine + ], + ); +} + module.exports = { getHostDefinedOptionId, internalCompileFunction, isContext, + makeContextifyScript, + registerImportModuleDynamically, + runScriptInThisContext, }; diff --git a/lib/repl.js b/lib/repl.js index 52f3026414d72d..d8021215b3f425 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -118,6 +118,9 @@ const { } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const vm = require('vm'); + +const { runInThisContext, runInContext } = vm.Script.prototype; + const path = require('path'); const fs = require('fs'); const { Interface } = require('readline'); @@ -185,7 +188,9 @@ const history = require('internal/repl/history'); const { extensionFormatMap, } = require('internal/modules/esm/formats'); - +const { + makeContextifyScript, +} = require('internal/vm'); let nextREPLResourceNumber = 1; // This prevents v8 code cache from getting confused and using a different // cache from a resource of the same name @@ -430,8 +435,6 @@ function REPLServer(prompt, } function defaultEval(code, context, file, cb) { - const asyncESM = require('internal/process/esm_loader'); - let result, script, wrappedErr; let err = null; let wrappedCmd = false; @@ -449,6 +452,21 @@ function REPLServer(prompt, wrappedCmd = true; } + const hostDefinedOptionId = Symbol(`eval:${file}`); + let parentURL; + try { + const { pathToFileURL } = require('internal/url'); + // Adding `/repl` prevents dynamic imports from loading relative + // to the parent of `process.cwd()`. + parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; + } catch { + // Continue regardless of error. + } + async function importModuleDynamically(specifier, _, importAttributes) { + const asyncESM = require('internal/process/esm_loader'); + return asyncESM.esmLoader.import(specifier, parentURL, + importAttributes); + } // `experimentalREPLAwait` is set to true by default. // Shall be false in case `--no-experimental-repl-await` flag is used. if (experimentalREPLAwait && StringPrototypeIncludes(code, 'await')) { @@ -466,28 +484,21 @@ function REPLServer(prompt, } catch (e) { let recoverableError = false; if (e.name === 'SyntaxError') { - let parentURL; - try { - const { pathToFileURL } = require('internal/url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - // Continue regardless of error. - } - // Remove all "await"s and attempt running the script // in order to detect if error is truly non recoverable const fallbackCode = SideEffectFreeRegExpPrototypeSymbolReplace(/\bawait\b/g, code, ''); try { - vm.createScript(fallbackCode, { - filename: file, - displayErrors: true, - importModuleDynamically: (specifier, _, importAttributes) => { - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); - }, - }); + makeContextifyScript( + fallbackCode, // code + file, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); } catch (fallbackError) { if (isRecoverableError(fallbackError, fallbackCode)) { recoverableError = true; @@ -507,15 +518,6 @@ function REPLServer(prompt, return cb(null); if (err === null) { - let parentURL; - try { - const { pathToFileURL } = require('internal/url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - // Continue regardless of error. - } while (true) { try { if (self.replMode === module.exports.REPL_MODE_STRICT && @@ -524,14 +526,17 @@ function REPLServer(prompt, // value for statements and declarations that don't return a value. code = `'use strict'; void 0;\n${code}`; } - script = vm.createScript(code, { - filename: file, - displayErrors: true, - importModuleDynamically: (specifier, _, importAttributes) => { - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); - }, - }); + script = makeContextifyScript( + code, // code + file, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); } catch (e) { debug('parse error %j', code, e); if (wrappedCmd) { @@ -591,9 +596,9 @@ function REPLServer(prompt, }; if (self.useGlobal) { - result = script.runInThisContext(scriptOptions); + result = ReflectApply(runInThisContext, script, [scriptOptions]); } else { - result = script.runInContext(context, scriptOptions); + result = ReflectApply(runInContext, script, [context, scriptOptions]); } } finally { if (self.breakEvalOnSigint) { diff --git a/lib/vm.js b/lib/vm.js index fe2981019a6170..d396a3df8c0e13 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -39,13 +39,16 @@ const { ERR_INVALID_ARG_TYPE, } = require('internal/errors').codes; const { + validateArray, validateBoolean, validateBuffer, validateInt32, - validateObject, validateOneOf, + validateObject, validateString, + validateStringArray, validateUint32, + kValidateObjectAllowNullable, } = require('internal/validators'); const { emitExperimentalWarning, @@ -56,6 +59,7 @@ const { getHostDefinedOptionId, internalCompileFunction, isContext, + registerImportModuleDynamically, } = require('internal/vm'); const kParsingContext = Symbol('script parsing context'); @@ -105,13 +109,7 @@ class Script extends ContextifyScript { } if (importModuleDynamically !== undefined) { - const { importModuleDynamicallyWrap } = require('internal/vm/module'); - const { registerModule } = require('internal/modules/esm/utils'); - registerModule(this, { - __proto__: null, - importModuleDynamically: - importModuleDynamicallyWrap(importModuleDynamically), - }); + registerImportModuleDynamically(this, importModuleDynamically); } } @@ -298,7 +296,54 @@ function runInThisContext(code, options) { } function compileFunction(code, params, options = kEmptyObject) { - return internalCompileFunction(code, params, options).function; + validateString(code, 'code'); + if (params !== undefined) { + validateStringArray(params, 'params'); + } + const { + filename = '', + columnOffset = 0, + lineOffset = 0, + cachedData = undefined, + produceCachedData = false, + parsingContext = undefined, + contextExtensions = [], + importModuleDynamically, + } = options; + + validateString(filename, 'options.filename'); + validateInt32(columnOffset, 'options.columnOffset'); + validateInt32(lineOffset, 'options.lineOffset'); + if (cachedData !== undefined) + validateBuffer(cachedData, 'options.cachedData'); + validateBoolean(produceCachedData, 'options.produceCachedData'); + if (parsingContext !== undefined) { + if ( + typeof parsingContext !== 'object' || + parsingContext === null || + !isContext(parsingContext) + ) { + throw new ERR_INVALID_ARG_TYPE( + 'options.parsingContext', + 'Context', + parsingContext, + ); + } + } + validateArray(contextExtensions, 'options.contextExtensions'); + ArrayPrototypeForEach(contextExtensions, (extension, i) => { + const name = `options.contextExtensions[${i}]`; + validateObject(extension, name, kValidateObjectAllowNullable); + }); + + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); + + return internalCompileFunction( + code, filename, lineOffset, columnOffset, + cachedData, produceCachedData, parsingContext, contextExtensions, + params, hostDefinedOptionId, importModuleDynamically, + ).function; } const measureMemoryModes = { diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out index 3f44332c03a470..e07bbe4d6acd3c 100644 --- a/test/message/eval_messages.out +++ b/test/message/eval_messages.out @@ -2,10 +2,9 @@ [eval]:1 with(this){__filename} ^^^^ + SyntaxError: Strict mode code may not include a with statement - at new Script (node:vm:*:*) - at createScript (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at makeContextifyScript (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -21,8 +20,7 @@ throw new Error("hello") Error: hello at [eval]:1:7 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -37,8 +35,7 @@ throw new Error("hello") Error: hello at [eval]:1:7 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -53,8 +50,7 @@ var x = 100; y = x; ReferenceError: y is not defined at [eval]:1:16 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index 3fd047aacb11a2..6afc8a62d7fcd9 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -4,9 +4,7 @@ with(this){__filename} ^^^^ SyntaxError: Strict mode code may not include a with statement - at new Script (node:vm:*) - at createScript (node:vm:*) - at Object.runInThisContext (node:vm:*) + at makeContextifyScript (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -14,6 +12,8 @@ SyntaxError: Strict mode code may not include a with statement at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) + at process.processTicksAndRejections (node:internal/process/task_queues:*:*) Node.js * 42 @@ -24,8 +24,7 @@ throw new Error("hello") Error: hello at [stdin]:1:7 - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -33,6 +32,7 @@ Error: hello at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * [stdin]:1 @@ -41,8 +41,7 @@ throw new Error("hello") Error: hello at [stdin]:1:* - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -50,6 +49,7 @@ Error: hello at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * 100 @@ -59,8 +59,7 @@ let x = 100; y = x; ReferenceError: y is not defined at [stdin]:1:16 - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -68,6 +67,7 @@ ReferenceError: y is not defined at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * From dda33c2bf13a39b504b60859c00b0556e73060eb Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 5 Oct 2023 23:08:44 +0200 Subject: [PATCH 14/87] vm: reject in importModuleDynamically without --experimental-vm-modules Users cannot access any API that can be used to return a module or module namespace in this callback without --experimental-vm-modules anyway, so this would eventually lead to a rejection. This patch rejects in this case with our own error message and use a constant host-defined option for the rejection, so that scripts with the same source can still be compiled using the compilation cache if no `import()` is actually called in the script. PR-URL: https://github.com/nodejs/node/pull/50137 Refs: https://github.com/nodejs/node/issues/35375 Reviewed-By: Geoffrey Booth Reviewed-By: Yagiz Nizipli Reviewed-By: Chengzhong Wu Reviewed-By: Antoine du Hamel --- doc/api/errors.md | 6 ++++ doc/api/vm.md | 24 ++++++++++++---- lib/internal/errors.js | 3 ++ lib/internal/modules/esm/utils.js | 8 +++++- lib/internal/vm.js | 16 +++++++++++ src/env_properties.h | 3 +- ...vm-dynamic-import-callback-missing-flag.js | 28 +++++++++++++++++++ 7 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-vm-dynamic-import-callback-missing-flag.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 24b1d0afeede9a..8070fffc388b9f 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2981,6 +2981,12 @@ An attempt was made to use something that was already closed. While using the Performance Timing API (`perf_hooks`), no valid performance entry types are found. + + +### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG` + +A dynamic import callback was invoked without `--experimental-vm-modules`. + ### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING` diff --git a/doc/api/vm.md b/doc/api/vm.md index e5c116d54af0bf..2043a39f527249 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -98,7 +98,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -760,6 +762,9 @@ changes: * `importModuleDynamically` {Function} Called during evaluation of this module when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. + If `--experimental-vm-modules` isn't set, this callback will be ignored + and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `module` {vm.Module} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1018,7 +1023,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API, and should not be - considered stable. + considered stable. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `function` {Function} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1242,7 +1249,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1341,7 +1350,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1421,7 +1432,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1585,6 +1598,7 @@ are not controllable through the timeout either. [Source Text Module Record]: https://tc39.es/ecma262/#sec-source-text-module-records [Synthetic Module Record]: https://heycam.github.io/webidl/#synthetic-module-records [V8 Embedder's Guide]: https://v8.dev/docs/embed#contexts +[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing [`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status [`Error`]: errors.md#class-error diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 76ce7867043c7d..a72236063b47fd 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1821,6 +1821,9 @@ E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', 'At least one valid performance entry type is required', Error); E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING', 'A dynamic import callback was not specified.', TypeError); +E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG', + 'A dynamic import callback was invoked without --experimental-vm-modules', + TypeError); E('ERR_VM_MODULE_ALREADY_LINKED', 'Module has already been linked', Error); E('ERR_VM_MODULE_CANNOT_CREATE_CACHED_DATA', 'Cached data cannot be created for a module which has been evaluated', Error); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index cbb583ede526b5..9d3e1aefbe3a31 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -14,9 +14,11 @@ const { } = internalBinding('util'); const { default_host_defined_options, + vm_dynamic_import_missing_flag, } = internalBinding('symbols'); const { + ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG, ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, ERR_INVALID_ARG_VALUE, } = require('internal/errors').codes; @@ -132,7 +134,8 @@ const moduleRegistries = new SafeWeakMap(); */ function registerModule(referrer, registry) { const idSymbol = referrer[host_defined_option_symbol]; - if (idSymbol === default_host_defined_options) { + if (idSymbol === default_host_defined_options || + idSymbol === vm_dynamic_import_missing_flag) { // The referrer is compiled without custom callbacks, so there is // no registry to hold on to. We'll throw // ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING when a callback is @@ -173,6 +176,9 @@ async function importModuleDynamicallyCallback(symbol, specifier, attributes) { return importModuleDynamically(specifier, callbackReferrer, attributes); } } + if (symbol === vm_dynamic_import_missing_flag) { + throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG(); + } throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); } diff --git a/lib/internal/vm.js b/lib/internal/vm.js index 4f7b8c652f3c26..b67eb177b35f07 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -15,6 +15,7 @@ const { } = ContextifyScript.prototype; const { default_host_defined_options, + vm_dynamic_import_missing_flag, } = internalBinding('symbols'); const { validateFunction, @@ -22,6 +23,11 @@ const { kValidateObjectAllowArray, } = require('internal/validators'); +const { + getOptionValue, +} = require('internal/options'); + + function isContext(object) { validateObject(object, 'object', kValidateObjectAllowArray); @@ -41,6 +47,16 @@ function getHostDefinedOptionId(importModuleDynamically, filename) { // compilation cache can be hit. return default_host_defined_options; } + // We should've thrown here immediately when we introduced + // --experimental-vm-modules and importModuleDynamically, but since + // users are already using this callback to throw a similar error, + // we also defer the error to the time when an actual import() is called + // to avoid breaking them. To ensure that the isolate compilation + // cache can still be hit, use a constant sentinel symbol here. + if (!getOptionValue('--experimental-vm-modules')) { + return vm_dynamic_import_missing_flag; + } + return Symbol(filename); } diff --git a/src/env_properties.h b/src/env_properties.h index 93fa8afdca39d3..7356846d6e7085 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -46,7 +46,8 @@ V(owner_symbol, "owner_symbol") \ V(onpskexchange_symbol, "onpskexchange") \ V(resource_symbol, "resource_symbol") \ - V(trigger_async_id_symbol, "trigger_async_id_symbol") + V(trigger_async_id_symbol, "trigger_async_id_symbol") \ + V(vm_dynamic_import_missing_flag, "vm_dynamic_import_missing_flag") // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. diff --git a/test/parallel/test-vm-dynamic-import-callback-missing-flag.js b/test/parallel/test-vm-dynamic-import-callback-missing-flag.js new file mode 100644 index 00000000000000..4b0d09ca3674a7 --- /dev/null +++ b/test/parallel/test-vm-dynamic-import-callback-missing-flag.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common'); +const { Script, compileFunction } = require('vm'); +const assert = require('assert'); + +assert( + !process.execArgv.includes('--experimental-vm-modules'), + 'This test must be run without --experimental-vm-modules'); + +assert.rejects(async () => { + const script = new Script('import("fs")', { + importModuleDynamically: common.mustNotCall(), + }); + const imported = script.runInThisContext(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG' +}).then(common.mustCall()); + +assert.rejects(async () => { + const imported = compileFunction('return import("fs")', [], { + importModuleDynamically: common.mustNotCall(), + })(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG' +}).then(common.mustCall()); From 571f7ef1fa214c820ec1765e9e31f36a2731a255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Tue, 17 Oct 2023 17:16:23 +0200 Subject: [PATCH 15/87] deps: patch V8 to 11.8.172.15 Refs: https://github.com/v8/v8/compare/11.8.172.13...11.8.172.15 PR-URL: https://github.com/nodejs/node/pull/50114 Reviewed-By: Jiawen Geng Reviewed-By: Richard Lau Reviewed-By: Yagiz Nizipli --- deps/v8/include/v8-version.h | 2 +- deps/v8/src/compiler/js-call-reducer.cc | 5 +++- deps/v8/src/ic/ic.cc | 14 +++++++---- .../mjsunit/compiler/regress-crbug-1486342.js | 25 +++++++++++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 deps/v8/test/mjsunit/compiler/regress-crbug-1486342.js diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index 79ab6bb5753c6b..e34be9b283f45c 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -11,7 +11,7 @@ #define V8_MAJOR_VERSION 11 #define V8_MINOR_VERSION 8 #define V8_BUILD_NUMBER 172 -#define V8_PATCH_LEVEL 13 +#define V8_PATCH_LEVEL 15 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) diff --git a/deps/v8/src/compiler/js-call-reducer.cc b/deps/v8/src/compiler/js-call-reducer.cc index 65fd9b61753b39..caec49b87c5282 100644 --- a/deps/v8/src/compiler/js-call-reducer.cc +++ b/deps/v8/src/compiler/js-call-reducer.cc @@ -6381,8 +6381,11 @@ Reduction JSCallReducer::ReduceArrayIterator(Node* node, } } + // JSCreateArrayIterator doesn't have control output, so we bypass the old + // JSCall node on the control chain. + ReplaceWithValue(node, node, node, control); + // Morph the {node} into a JSCreateArrayIterator with the given {kind}. - RelaxControls(node); node->ReplaceInput(0, receiver); node->ReplaceInput(1, context); node->ReplaceInput(2, effect); diff --git a/deps/v8/src/ic/ic.cc b/deps/v8/src/ic/ic.cc index 630f4db9c82ae2..98af9536effafa 100644 --- a/deps/v8/src/ic/ic.cc +++ b/deps/v8/src/ic/ic.cc @@ -3185,18 +3185,22 @@ bool CanFastCloneObjectWithDifferentMaps(Handle source_map, Handle target_map, Isolate* isolate) { DisallowGarbageCollection no_gc; - // TODO(olivf): Add support for non JS_OBJECT_TYPE source maps. The reason for - // this restriction is that the IC does not initialize the target object and - // instead relies on copying the source objects bytes. Thus they need to have - // the same binary layout. + // Ensure source and target have identical binary represenation of properties + // and elements as the IC relies on copying the raw bytes. This also excludes + // cases with non-enumerable properties or accessors on the source object. if (source_map->instance_type() != JS_OBJECT_TYPE || target_map->instance_type() != JS_OBJECT_TYPE || !source_map->OnlyHasSimpleProperties() || - !target_map->OnlyHasSimpleProperties()) { + !target_map->OnlyHasSimpleProperties() || + source_map->elements_kind() != target_map->elements_kind() || + !source_map->has_fast_elements()) { return false; } // Check that the source inobject properties are big enough to initialize all // target slots, but not too big to fit. + // TODO(olivf): This restriction (and the same restriction on the backing + // store) could be lifted by properly initializing the target object instead + // of relying on copying empty slots. int source_inobj_properties = source_map->GetInObjectProperties(); int target_inobj_properties = target_map->GetInObjectProperties(); int source_used_inobj_properties = diff --git a/deps/v8/test/mjsunit/compiler/regress-crbug-1486342.js b/deps/v8/test/mjsunit/compiler/regress-crbug-1486342.js new file mode 100644 index 00000000000000..c35a7339801d03 --- /dev/null +++ b/deps/v8/test/mjsunit/compiler/regress-crbug-1486342.js @@ -0,0 +1,25 @@ +// Copyright 2023 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --jit-fuzzing + +const o13 = { + "maxByteLength": 5368789, +}; +const v14 = new ArrayBuffer(129, o13); +const v16 = new Uint16Array(v14); + +function f3(param) { + for (let i = 0; i < 5; i++) { + try {"resize".includes(v14); } catch (e) {} + v14.resize(3.0, ..."resize", ...v16); + } + + let f = function() { return param; } +} + +%PrepareFunctionForOptimization(f3); +f3(); +%OptimizeFunctionOnNextCall(f3); +f3(); From 514ac86579aaa0215913a0067c4c80dae3a94f99 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 17 Oct 2023 21:09:34 +0200 Subject: [PATCH 16/87] stream: reduce scope of readable bitmap details PR-URL: https://github.com/nodejs/node/pull/49963 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Raz Luvaton Reviewed-By: Luigi Pinca Reviewed-By: Rich Trott --- lib/internal/streams/readable.js | 289 ++++++++++++++++++++----------- 1 file changed, 191 insertions(+), 98 deletions(-) diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js index 7ceb83d3f20523..a129b1b6f4b75d 100644 --- a/lib/internal/streams/readable.js +++ b/lib/internal/streams/readable.js @@ -72,7 +72,6 @@ const { } = require('internal/errors'); const { validateObject } = require('internal/validators'); -const kPaused = Symbol('kPaused'); const kState = Symbol('kState'); const { StringDecoder } = require('string_decoder'); @@ -84,6 +83,11 @@ const nop = () => {}; const { errorOrDestroy } = destroyImpl; +const kErroredValue = Symbol('kErroredValue'); +const kDefaultEncodingValue = Symbol('kDefaultEncodingValue'); +const kDecoderValue = Symbol('kDecoderValue'); +const kEncodingValue = Symbol('kEncodingValue'); + const kObjectMode = 1 << 0; const kEnded = 1 << 1; const kEndEmitted = 1 << 2; @@ -103,6 +107,14 @@ const kCloseEmitted = 1 << 15; const kMultiAwaitDrain = 1 << 16; const kReadingMore = 1 << 17; const kDataEmitted = 1 << 18; +const kErrored = 1 << 19; +const kDefaultUTF8Encoding = 1 << 20; +const kDecoder = 1 << 21; +const kEncoding = 1 << 22; +const kHasFlowing = 1 << 23; +const kFlowing = 1 << 24; +const kHasPaused = 1 << 25; +const kPaused = 1 << 26; // TODO(benjamingr) it is likely slower to do it this way than with free functions function makeBitMapDescriptor(bit) { @@ -151,8 +163,93 @@ ObjectDefineProperties(ReadableState.prototype, { // If true, a maybeReadMore has been scheduled. readingMore: makeBitMapDescriptor(kReadingMore), dataEmitted: makeBitMapDescriptor(kDataEmitted), + + // Indicates whether the stream has errored. When true no further + // _read calls, 'data' or 'readable' events should occur. This is needed + // since when autoDestroy is disabled we need a way to tell whether the + // stream has failed. + errored: { + __proto__: null, + enumerable: false, + get() { + return (this[kState] & kErrored) !== 0 ? this[kErroredValue] : null; + }, + set(value) { + if (value) { + this[kErroredValue] = value; + this[kState] |= kErrored; + } else { + this[kState] &= ~kErrored; + } + }, + }, + + defaultEncoding: { + __proto__: null, + enumerable: false, + get() { return (this[kState] & kDefaultUTF8Encoding) !== 0 ? 'utf8' : this[kDefaultEncodingValue]; }, + set(value) { + if (value === 'utf8' || value === 'utf-8') { + this[kState] |= kDefaultUTF8Encoding; + } else { + this[kState] &= ~kDefaultUTF8Encoding; + this[kDefaultEncodingValue] = value; + } + }, + }, + + decoder: { + __proto__: null, + enumerable: false, + get() { + return (this[kState] & kDecoder) !== 0 ? this[kDecoderValue] : null; + }, + set(value) { + if (value) { + this[kDecoderValue] = value; + this[kState] |= kDecoder; + } else { + this[kState] &= ~kDecoder; + } + }, + }, + + encoding: { + __proto__: null, + enumerable: false, + get() { + return (this[kState] & kEncoding) !== 0 ? this[kEncodingValue] : null; + }, + set(value) { + if (value) { + this[kEncodingValue] = value; + this[kState] |= kEncoding; + } else { + this[kState] &= ~kEncoding; + } + }, + }, + + flowing: { + __proto__: null, + enumerable: false, + get() { + return (this[kState] & kHasFlowing) !== 0 ? (this[kState] & kFlowing) !== 0 : null; + }, + set(value) { + if (value == null) { + this[kState] &= ~(kHasFlowing | kFlowing); + } else if (value) { + this[kState] |= (kHasFlowing | kFlowing); + } else { + this[kState] |= kHasFlowing; + this[kState] &= ~kFlowing; + } + }, + }, }); + function ReadableState(options, stream, isDuplex) { // Duplex streams are both readable and writable, but share // the same options object. @@ -184,9 +281,6 @@ function ReadableState(options, stream, isDuplex) { this.buffer = new BufferList(); this.length = 0; this.pipes = []; - this.flowing = null; - - this[kPaused] = null; // Should close be emitted on destroy. Defaults to true. if (options && options.emitClose === false) this[kState] &= ~kEmitClose; @@ -194,20 +288,12 @@ function ReadableState(options, stream, isDuplex) { // Should .destroy() be called after 'end' (and potentially 'finish'). if (options && options.autoDestroy === false) this[kState] &= ~kAutoDestroy; - - // Indicates whether the stream has errored. When true no further - // _read calls, 'data' or 'readable' events should occur. This is needed - // since when autoDestroy is disabled we need a way to tell whether the - // stream has failed. - this.errored = null; - - // Crypto is kind of old and crusty. Historically, its default string // encoding is 'binary' so we have to make this configurable. // Everything else in the universe uses 'utf8', though. const defaultEncoding = options?.defaultEncoding; - if (defaultEncoding == null) { - this.defaultEncoding = 'utf8'; + if (defaultEncoding == null || defaultEncoding === 'utf8' || defaultEncoding === 'utf-8') { + this[kState] |= kDefaultUTF8Encoding; } else if (Buffer.isEncoding(defaultEncoding)) { this.defaultEncoding = defaultEncoding; } else { @@ -218,8 +304,6 @@ function ReadableState(options, stream, isDuplex) { // type: null | Writable | Set. this.awaitDrainWriters = null; - this.decoder = null; - this.encoding = null; if (options && options.encoding) { this.decoder = new StringDecoder(options.encoding); this.encoding = options.encoding; @@ -363,7 +447,6 @@ function readableAddChunkPushByteMode(stream, state, chunk, encoding) { if (chunk === null) { state[kState] &= ~kReading; onEofChunk(stream, state); - return false; } @@ -391,22 +474,20 @@ function readableAddChunkPushByteMode(stream, state, chunk, encoding) { return canPushMore(state); } - if (state.ended) { + if ((state[kState] & kEnded) !== 0) { errorOrDestroy(stream, new ERR_STREAM_PUSH_AFTER_EOF()); - return false; } - if (state.destroyed || state.errored) { + if ((state[kState] & (kDestroyed | kErrored)) !== 0) { return false; } state[kState] &= ~kReading; - if (state.decoder && !encoding) { - chunk = state.decoder.write(chunk); + if ((state[kState] & kDecoder) !== 0 && !encoding) { + chunk = state[kDecoderValue].write(chunk); if (chunk.length === 0) { maybeReadMore(stream, state); - return canPushMore(state); } } @@ -419,22 +500,22 @@ function readableAddChunkPushObjectMode(stream, state, chunk, encoding) { if (chunk === null) { state[kState] &= ~kReading; onEofChunk(stream, state); - return false; } - if (state.ended) { + if ((state[kState] & kEnded) !== 0) { errorOrDestroy(stream, new ERR_STREAM_PUSH_AFTER_EOF()); return false; } - if (state.destroyed || state.errored) { + if ((state[kState] & (kDestroyed | kErrored)) !== 0) { return false; } state[kState] &= ~kReading; - if (state.decoder && !encoding) { - chunk = state.decoder.write(chunk); + + if ((state[kState] & kDecoder) !== 0 && !encoding) { + chunk = state[kDecoderValue].write(chunk); } addChunk(stream, state, chunk, false); @@ -445,12 +526,12 @@ function canPushMore(state) { // We can push more data if we are below the highWaterMark. // Also, if we have no data yet, we can stand some more bytes. // This is to work around cases where hwm=0, such as the repl. - return !state.ended && + return (state[kState] & kEnded) === 0 && (state.length < state.highWaterMark || state.length === 0); } function addChunk(stream, state, chunk, addToFront) { - if (state.flowing && state.length === 0 && !state.sync && + if ((state[kState] & (kFlowing | kSync)) === kFlowing && state.length === 0 && stream.listenerCount('data') > 0) { // Use the guard to avoid creating `Set()` repeatedly // when we have multiple pipes. @@ -460,11 +541,11 @@ function addChunk(stream, state, chunk, addToFront) { state.awaitDrainWriters = null; } - state.dataEmitted = true; + state[kState] |= kDataEmitted; stream.emit('data', chunk); } else { // Update the buffer info. - state.length += state.objectMode ? 1 : chunk.length; + state.length += (state[kState] & kObjectMode) !== 0 ? 1 : chunk.length; if (addToFront) state.buffer.unshift(chunk); else @@ -478,7 +559,7 @@ function addChunk(stream, state, chunk, addToFront) { Readable.prototype.isPaused = function() { const state = this._readableState; - return state[kPaused] === true || state.flowing === false; + return (state[kState] & kPaused) !== 0 || (state[kState] & (kHasFlowing | kFlowing)) === kHasFlowing; }; // Backwards compatibility. @@ -529,13 +610,13 @@ function howMuchToRead(n, state) { return 1; if (NumberIsNaN(n)) { // Only flow one buffer at a time. - if (state.flowing && state.length) + if ((state[kState] & kFlowing) !== 0 && state.length) return state.buffer.first().length; return state.length; } if (n <= state.length) return n; - return state.ended ? state.length : 0; + return (state[kState] & kEnded) !== 0 ? state.length : 0; } // You can override either this method, or the async _read(n) below. @@ -562,13 +643,13 @@ Readable.prototype.read = function(n) { // already have a bunch of data in the buffer, then just trigger // the 'readable' event and move on. if (n === 0 && - state.needReadable && + (state[kState] & kNeedReadable) !== 0 && ((state.highWaterMark !== 0 ? state.length >= state.highWaterMark : state.length > 0) || - state.ended)) { - debug('read: emitReadable', state.length, state.ended); - if (state.length === 0 && state.ended) + (state[kState] & kEnded) !== 0)) { + debug('read: emitReadable', state.length, (state[kState] & kEnded) !== 0); + if (state.length === 0 && (state[kState] & kEnded) !== 0) endReadable(this); else emitReadable(this); @@ -578,7 +659,7 @@ Readable.prototype.read = function(n) { n = howMuchToRead(n, state); // If we've ended, and we're now clear, then finish it up. - if (n === 0 && state.ended) { + if (n === 0 && (state[kState] & kEnded) !== 0) { if (state.length === 0) endReadable(this); return null; @@ -619,8 +700,7 @@ Readable.prototype.read = function(n) { // However, if we've ended, then there's no point, if we're already // reading, then it's unnecessary, if we're constructing we have to wait, // and if we're destroyed or errored, then it's not allowed, - if (state.ended || state.reading || state.destroyed || state.errored || - !state.constructed) { + if ((state[kState] & (kReading | kEnded | kDestroyed | kErrored | kConstructed)) !== kConstructed) { doRead = false; debug('reading, ended or constructing', doRead); } else if (doRead) { @@ -640,7 +720,7 @@ Readable.prototype.read = function(n) { // If _read pushed data synchronously, then `reading` will be false, // and we need to re-evaluate how much data we can return to the user. - if (!state.reading) + if ((state[kState] & kReading) === 0) n = howMuchToRead(nOrig, state); } @@ -651,11 +731,11 @@ Readable.prototype.read = function(n) { ret = null; if (ret === null) { - state.needReadable = state.length <= state.highWaterMark; + state[kState] |= state.length <= state.highWaterMark ? kNeedReadable : 0; n = 0; } else { state.length -= n; - if (state.multiAwaitDrain) { + if ((state[kState] & kMultiAwaitDrain) !== 0) { state.awaitDrainWriters.clear(); } else { state.awaitDrainWriters = null; @@ -665,16 +745,16 @@ Readable.prototype.read = function(n) { if (state.length === 0) { // If we have nothing in the buffer, then we want to know // as soon as we *do* get something into the buffer. - if (!state.ended) - state.needReadable = true; + if ((state[kState] & kEnded) === 0) + state[kState] |= kNeedReadable; // If we tried to read() past the EOF, then emit end on the next tick. - if (nOrig !== n && state.ended) + if (nOrig !== n && (state[kState] & kEnded) !== 0) endReadable(this); } - if (ret !== null && !state.errorEmitted && !state.closeEmitted) { - state.dataEmitted = true; + if (ret !== null && (state[kState] & (kErrorEmitted | kCloseEmitted)) === 0) { + state[kState] |= kDataEmitted; this.emit('data', ret); } @@ -683,25 +763,26 @@ Readable.prototype.read = function(n) { function onEofChunk(stream, state) { debug('onEofChunk'); - if (state.ended) return; - if (state.decoder) { - const chunk = state.decoder.end(); + if ((state[kState] & kEnded) !== 0) return; + const decoder = (state[kState] & kDecoder) !== 0 ? state[kDecoderValue] : null; + if (decoder) { + const chunk = decoder.end(); if (chunk && chunk.length) { state.buffer.push(chunk); - state.length += state.objectMode ? 1 : chunk.length; + state.length += (state[kState] & kObjectMode) !== 0 ? 1 : chunk.length; } } - state.ended = true; + state[kState] |= kEnded; - if (state.sync) { + if ((state[kState] & kSync) !== 0) { // If we are sync, wait until next tick to emit the data. // Otherwise we risk emitting data in the flow() // the readable code triggers during a read() call. emitReadable(stream); } else { // Emit 'readable' now to make sure it gets picked up. - state.needReadable = false; - state.emittedReadable = true; + state[kState] &= ~kNeedReadable; + state[kState] |= kEmittedReadable; // We have to emit readable now that we are EOF. Modules // in the ecosystem (e.g. dicer) rely on this event being sync. emitReadable_(stream); @@ -713,21 +794,21 @@ function onEofChunk(stream, state) { // a nextTick recursion warning, but that's not so bad. function emitReadable(stream) { const state = stream._readableState; - debug('emitReadable', state.needReadable, state.emittedReadable); - state.needReadable = false; - if (!state.emittedReadable) { - debug('emitReadable', state.flowing); - state.emittedReadable = true; + debug('emitReadable'); + state[kState] &= ~kNeedReadable; + if ((state[kState] & kEmittedReadable) === 0) { + debug('emitReadable', (state[kState] & kFlowing) !== 0); + state[kState] |= kEmittedReadable; process.nextTick(emitReadable_, stream); } } function emitReadable_(stream) { const state = stream._readableState; - debug('emitReadable_', state.destroyed, state.length, state.ended); - if (!state.destroyed && !state.errored && (state.length || state.ended)) { + debug('emitReadable_'); + if ((state[kState] & (kDestroyed | kErrored)) === 0 && (state.length || state.ended)) { stream.emit('readable'); - state.emittedReadable = false; + state[kState] &= ~kEmittedReadable; } // The stream needs another readable event if: @@ -736,10 +817,9 @@ function emitReadable_(stream) { // 2. It is not ended. // 3. It is below the highWaterMark, so we can schedule // another readable later. - state.needReadable = - !state.flowing && - !state.ended && - state.length <= state.highWaterMark; + state[kState] |= + (state[kState] & (kFlowing | kEnded)) === 0 && + state.length <= state.highWaterMark ? kNeedReadable : 0; flow(stream); } @@ -751,8 +831,8 @@ function emitReadable_(stream) { // However, if we're not ended, or reading, and the length < hwm, // then go ahead and try to read some more preemptively. function maybeReadMore(stream, state) { - if (!state.readingMore && state.constructed) { - state.readingMore = true; + if ((state[kState] & (kReadingMore | kConstructed)) === kConstructed) { + state[kState] |= kReadingMore; process.nextTick(maybeReadMore_, stream, state); } } @@ -781,9 +861,9 @@ function maybeReadMore_(stream, state) { // called push() with new data. In this case we skip performing more // read()s. The execution ends in this method again after the _read() ends // up calling push() with more data. - while (!state.reading && !state.ended && + while ((state[kState] & (kReading | kEnded)) === 0 && (state.length < state.highWaterMark || - (state.flowing && state.length === 0))) { + ((state[kState] & kFlowing) !== 0 && state.length === 0))) { const len = state.length; debug('maybeReadMore read 0'); stream.read(0); @@ -791,7 +871,7 @@ function maybeReadMore_(stream, state) { // Didn't get any data, stop spinning. break; } - state.readingMore = false; + state[kState] &= ~kReadingMore; } // Abstract method. to be overridden in specific implementation classes. @@ -808,7 +888,7 @@ Readable.prototype.pipe = function(dest, pipeOpts) { if (state.pipes.length === 1) { if (!state.multiAwaitDrain) { - state.multiAwaitDrain = true; + state[kState] |= kMultiAwaitDrain; state.awaitDrainWriters = new SafeSet( state.awaitDrainWriters ? [state.awaitDrainWriters] : [], ); @@ -1089,16 +1169,16 @@ function updateReadableListening(self) { const state = self._readableState; state.readableListening = self.listenerCount('readable') > 0; - if (state.resumeScheduled && state[kPaused] === false) { + if ((state[kState] & (kHasPaused | kPaused | kResumeScheduled)) === (kHasPaused | kResumeScheduled)) { // Flowing needs to be set to true now, otherwise // the upcoming resume will not flow. - state.flowing = true; + state[kState] |= kHasFlowing | kFlowing; // Crude way to check if we should resume. } else if (self.listenerCount('data') > 0) { self.resume(); - } else if (!state.readableListening) { - state.flowing = null; + } else if ((state[kState] & kReadableListening) === 0) { + state[kState] &= ~(kHasFlowing | kFlowing); } } @@ -1111,15 +1191,21 @@ function nReadingNextTick(self) { // If the user uses them, then switch into old mode. Readable.prototype.resume = function() { const state = this._readableState; - if (!state.flowing) { + if ((state[kState] & kFlowing) === 0) { debug('resume'); // We flow only if there is no one listening // for readable, but we still have to call // resume(). - state.flowing = !state.readableListening; + state[kState] |= kHasFlowing; + if (!state.readableListening) { + state[kState] |= kFlowing; + } else { + state[kState] &= ~kFlowing; + } resume(this, state); } - state[kPaused] = false; + state[kState] |= kHasPaused; + state[kState] &= ~kPaused; return this; }; @@ -1131,33 +1217,35 @@ function resume(stream, state) { } function resume_(stream, state) { - debug('resume', state.reading); - if (!state.reading) { + debug('resume', (state[kState] & kReading) !== 0); + if ((state[kState] & kReading) === 0) { stream.read(0); } - state.resumeScheduled = false; + state[kState] &= ~kResumeScheduled; stream.emit('resume'); flow(stream); - if (state.flowing && !state.reading) + if ((state[kState] & (kFlowing | kReading)) === kFlowing) stream.read(0); } Readable.prototype.pause = function() { - debug('call pause flowing=%j', this._readableState.flowing); - if (this._readableState.flowing !== false) { + const state = this._readableState; + debug('call pause'); + if (state.flowing !== false) { debug('pause'); - this._readableState.flowing = false; + state[kState] |= kHasFlowing; + state[kState] &= ~kFlowing; this.emit('pause'); } - this._readableState[kPaused] = true; + state[kState] |= kHasPaused | kPaused; return this; }; function flow(stream) { const state = stream._readableState; - debug('flow', state.flowing); - while (state.flowing && stream.read() !== null); + debug('flow'); + while ((state[kState] & kFlowing) !== 0 && stream.read() !== null); } // Wrap an old-style stream as the async data source. @@ -1436,10 +1524,15 @@ ObjectDefineProperties(ReadableState.prototype, { paused: { __proto__: null, get() { - return this[kPaused] !== false; + return (this[kState] & kPaused) !== 0; }, set(value) { - this[kPaused] = !!value; + this[kState] |= kHasPaused; + if (value) { + this[kState] |= kPaused; + } else { + this[kState] &= ~kPaused; + } }, }, }); @@ -1479,9 +1572,9 @@ function fromList(n, state) { function endReadable(stream) { const state = stream._readableState; - debug('endReadable', state.endEmitted); - if (!state.endEmitted) { - state.ended = true; + debug('endReadable', (state[kState] & kEndEmitted) !== 0); + if ((state[kState] & kEndEmitted) === 0) { + state[kState] |= kEnded; process.nextTick(endReadableNT, state, stream); } } From 3729e3335860d9ae1e7bde74ec49578fcc91d98b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Louren=C3=A7o?= Date: Tue, 17 Oct 2023 15:59:29 -0300 Subject: [PATCH 17/87] doc: add H4ad to collaborators Fixes: https://github.com/nodejs/node/issues/50103 PR-URL: https://github.com/nodejs/node/pull/50217 Reviewed-By: Yagiz Nizipli Reviewed-By: Raz Luvaton --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 46f118d31c4cfa..d96132374644e0 100644 --- a/README.md +++ b/README.md @@ -359,6 +359,8 @@ For information about the governance of the Node.js project, see **Guy Bedford** <> (he/him) * [HarshithaKP](https://github.com/HarshithaKP) - **Harshitha K P** <> (she/her) +* [H4ad](https://github.com/himself65) - + **Vinícius Lourenço Claro Cardoso** <> (he/him) * [himself65](https://github.com/himself65) - **Zeyu "Alex" Yang** <> (he/him) * [iansu](https://github.com/iansu) - From 8c1a46c751cee94859c332014f9f41495efe30b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vinicius=20Louren=C3=A7o?= <12551007+H4ad@users.noreply.github.com> Date: Tue, 17 Oct 2023 16:31:49 -0300 Subject: [PATCH 18/87] doc: fix H4ad collaborator sort PR-URL: https://github.com/nodejs/node/pull/50218 Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow Reviewed-By: Matthew Aitken Reviewed-By: Richard Lau --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d96132374644e0..1f1027e1b57ecf 100644 --- a/README.md +++ b/README.md @@ -357,10 +357,10 @@ For information about the governance of the Node.js project, see **Gireesh Punathil** <> (he/him) * [guybedford](https://github.com/guybedford) - **Guy Bedford** <> (he/him) +* [H4ad](https://github.com/H4ad) - + **Vinícius Lourenço Claro Cardoso** <> (he/him) * [HarshithaKP](https://github.com/HarshithaKP) - **Harshitha K P** <> (she/her) -* [H4ad](https://github.com/himself65) - - **Vinícius Lourenço Claro Cardoso** <> (he/him) * [himself65](https://github.com/himself65) - **Zeyu "Alex" Yang** <> (he/him) * [iansu](https://github.com/iansu) - From 82363be2ac1ae5bf39d3eb5fbcfcca2ba0cc954b Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 18 Oct 2023 02:46:38 +0200 Subject: [PATCH 19/87] doc: fix typo in dgram docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50211 Reviewed-By: Marco Ippolito Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen Reviewed-By: Vinícius Lourenço Claro Cardoso --- doc/api/dgram.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/dgram.md b/doc/api/dgram.md index f19e62bc7736bd..65d4a9f678cbd2 100644 --- a/doc/api/dgram.md +++ b/doc/api/dgram.md @@ -904,7 +904,7 @@ to exclude the socket from the reference counting that keeps the Node.js process active, allowing the process to exit even if the socket is still listening. -Calling `socket.unref()` multiple times will have no addition effect. +Calling `socket.unref()` multiple times will have no additional effect. The `socket.unref()` method returns a reference to the socket so calls can be chained. From 18862e4d5df0e24844180202d999e6bb80463e59 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Tue, 17 Oct 2023 21:51:41 -0400 Subject: [PATCH 20/87] fs: add flush option to appendFile() functions This commit adds documentation and tests for the 'flush' option of the fs.appendFile family of functions. Technically, support was indirectly added in #50009, but this makes it more official. Refs: https://github.com/nodejs/node/issues/49886 Refs: https://github.com/nodejs/node/pull/50009 PR-URL: https://github.com/nodejs/node/pull/50095 Reviewed-By: Matteo Collina --- doc/api/fs.md | 21 ++++ test/parallel/test-fs-append-file-flush.js | 114 +++++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 test/parallel/test-fs-append-file-flush.js diff --git a/doc/api/fs.md b/doc/api/fs.md index fef0e13ab4a76e..fabbb60eeed73b 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -180,6 +180,9 @@ longer be used. * `path` {string|Buffer|URL|FileHandle} filename or {FileHandle} @@ -904,6 +913,8 @@ added: v10.0.0 * `encoding` {string|null} **Default:** `'utf8'` * `mode` {integer} **Default:** `0o666` * `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`. + * `flush` {boolean} If `true`, the underlying file descriptor is flushed + prior to closing it. **Default:** `false`. * Returns: {Promise} Fulfills with `undefined` upon success. Asynchronously append data to a file, creating the file if it does not yet @@ -2052,6 +2063,9 @@ the user from reading or writing to it. An import `type` attribute was provided, but the specified module is of a different type. - + -### `ERR_IMPORT_ASSERTION_TYPE_MISSING` +### `ERR_IMPORT_ATTRIBUTE_MISSING` An import attribute is missing, preventing the specified module to be imported. - - -### `ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED` - - - -An import attribute is not supported by this version of Node.js. - ### `ERR_IMPORT_ATTRIBUTE_UNSUPPORTED` @@ -3308,6 +3294,45 @@ changes: An invalid transfer object was passed to `postMessage()`. + + +### `ERR_IMPORT_ASSERTION_TYPE_FAILED` + + + +An import assertion has failed, preventing the specified module to be imported. + + + +### `ERR_IMPORT_ASSERTION_TYPE_MISSING` + + + +An import assertion is missing, preventing the specified module to be imported. + + + +### `ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED` + + + +An import attribute is not supported by this version of Node.js. + ### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST` diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a72236063b47fd..363b9d3bb8fe8b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1280,15 +1280,10 @@ E('ERR_HTTP_SOCKET_ENCODING', E('ERR_HTTP_TRAILER_INVALID', 'Trailers are invalid with this transfer encoding', Error); E('ERR_ILLEGAL_CONSTRUCTOR', 'Illegal constructor', TypeError); -// TODO(aduh95): change the error to mention import attributes instead of import assertions. -E('ERR_IMPORT_ASSERTION_TYPE_FAILED', +E('ERR_IMPORT_ATTRIBUTE_MISSING', + 'Module "%s" needs an import attribute of "%s: %s"', TypeError); +E('ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE', 'Module "%s" is not of type "%s"', TypeError); -// TODO(aduh95): change the error to mention import attributes instead of import assertions. -E('ERR_IMPORT_ASSERTION_TYPE_MISSING', - 'Module "%s" needs an import attribute of type "%s"', TypeError); -// TODO(aduh95): change the error to mention import attributes instead of import assertions. -E('ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', - 'Import attribute type "%s" is unsupported', TypeError); E('ERR_IMPORT_ATTRIBUTE_UNSUPPORTED', 'Import attribute "%s" with value "%s" is not supported', TypeError); E('ERR_INCOMPATIBLE_OPTION_PAIR', diff --git a/lib/internal/modules/esm/assert.js b/lib/internal/modules/esm/assert.js index ce3280de84bf4d..5672f8c8f9959d 100644 --- a/lib/internal/modules/esm/assert.js +++ b/lib/internal/modules/esm/assert.js @@ -10,9 +10,8 @@ const { const { validateString } = require('internal/validators'); const { - ERR_IMPORT_ASSERTION_TYPE_FAILED, - ERR_IMPORT_ASSERTION_TYPE_MISSING, - ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED, + ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE, + ERR_IMPORT_ATTRIBUTE_MISSING, ERR_IMPORT_ATTRIBUTE_UNSUPPORTED, } = require('internal/errors').codes; @@ -86,7 +85,7 @@ function validateAttributes(url, format, // `importAttributes.type` might not have been it. if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { // `type` wasn't specified at all. - throw new ERR_IMPORT_ASSERTION_TYPE_MISSING(url, validType); + throw new ERR_IMPORT_ATTRIBUTE_MISSING(url, 'type', validType); } return handleInvalidType(url, importAttributes.type); } @@ -103,11 +102,11 @@ function handleInvalidType(url, type) { // `type` might not have been one of the types we understand. if (!ArrayPrototypeIncludes(supportedAssertionTypes, type)) { - throw new ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED(type); + throw new ERR_IMPORT_ATTRIBUTE_UNSUPPORTED('type', type); } // `type` was the wrong value for this format. - throw new ERR_IMPORT_ASSERTION_TYPE_FAILED(url, type); + throw new ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE(url, type); } diff --git a/test/es-module/test-esm-import-attributes-errors.js b/test/es-module/test-esm-import-attributes-errors.js index 4521248db176e5..243277fdcd7d29 100644 --- a/test/es-module/test-esm-import-attributes-errors.js +++ b/test/es-module/test-esm-import-attributes-errors.js @@ -18,37 +18,37 @@ async function test() { await rejects( import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}with{type:"json"}`), - { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } + { code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE' } ); await rejects( import(jsModuleDataUrl, { with: { type: 'json' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } + { code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE' } ); await rejects( import(jsModuleDataUrl, { with: { type: 'unsupported' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } + { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' } ); await rejects( import(jsonModuleDataUrl), - { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } + { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' } ); await rejects( import(jsonModuleDataUrl, { with: {} }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } + { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' } ); await rejects( import(jsonModuleDataUrl, { with: { foo: 'bar' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } + { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' } ); await rejects( import(jsonModuleDataUrl, { with: { type: 'unsupported' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } + { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' } ); } diff --git a/test/es-module/test-esm-import-attributes-errors.mjs b/test/es-module/test-esm-import-attributes-errors.mjs index ff932636e39a5f..c29cc459326c4b 100644 --- a/test/es-module/test-esm-import-attributes-errors.mjs +++ b/test/es-module/test-esm-import-attributes-errors.mjs @@ -13,35 +13,35 @@ await rejects( await rejects( import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}with{type:"json"}`), - { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } + { code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE' } ); await rejects( import(jsModuleDataUrl, { with: { type: 'json' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } + { code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE' } ); await rejects( import(import.meta.url, { with: { type: 'unsupported' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } + { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' } ); await rejects( import(jsonModuleDataUrl), - { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } + { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' } ); await rejects( import(jsonModuleDataUrl, { with: {} }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } + { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' } ); await rejects( import(jsonModuleDataUrl, { with: { foo: 'bar' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } + { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' } ); await rejects( import(jsonModuleDataUrl, { with: { type: 'unsupported' } }), - { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } + { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' } ); diff --git a/test/es-module/test-esm-import-attributes-validation.js b/test/es-module/test-esm-import-attributes-validation.js index f436f7073126d7..0d1c4bb57d92cf 100644 --- a/test/es-module/test-esm-import-attributes-validation.js +++ b/test/es-module/test-esm-import-attributes-validation.js @@ -15,7 +15,7 @@ assert.ok(validateAttributes(url, 'module', {})); assert.ok(validateAttributes(url, 'wasm', {})); assert.throws(() => validateAttributes(url, 'json', {}), { - code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING', + code: 'ERR_IMPORT_ATTRIBUTE_MISSING', }); assert.throws(() => validateAttributes(url, 'json', { type: 'json', unsupportedAttribute: 'value' }), { @@ -27,17 +27,17 @@ assert.throws(() => validateAttributes(url, 'module', { unsupportedAttribute: 'v }); assert.throws(() => validateAttributes(url, 'module', { type: 'json' }), { - code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED', + code: 'ERR_IMPORT_ATTRIBUTE_TYPE_INCOMPATIBLE', }); // The HTML spec specifically disallows this for now, while Wasm module import // and whether it will require a type assertion is still an open question. assert.throws(() => validateAttributes(url, 'module', { type: 'javascript' }), { - code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', + code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED', }); assert.throws(() => validateAttributes(url, 'module', { type: 'css' }), { - code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', + code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED', }); assert.throws(() => validateAttributes(url, 'module', { type: false }), { From c76eb279712457d0ee6273195a04df2e879decae Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 18 Oct 2023 07:32:34 -0700 Subject: [PATCH 23/87] esm: improve check for ESM syntax PR-URL: https://github.com/nodejs/node/pull/50127 Reviewed-By: Guy Bedford Reviewed-By: Yagiz Nizipli Reviewed-By: Jacob Smith Reviewed-By: Antoine du Hamel --- lib/internal/modules/cjs/loader.js | 7 +- lib/internal/modules/esm/translators.js | 10 +- lib/internal/modules/helpers.js | 23 --- src/node_contextify.cc | 208 ++++++++++++++++++++---- src/node_contextify.h | 24 +++ 5 files changed, 207 insertions(+), 65 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index b3b438372fe9ed..b077ee386bb40e 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -90,6 +90,7 @@ const { makeContextifyScript, runScriptInThisContext, } = require('internal/vm'); +const { containsModuleSyntax } = internalBinding('contextify'); const assert = require('internal/assert'); const fs = require('fs'); @@ -104,7 +105,6 @@ const { const { getCjsConditions, initializeCjsConditions, - hasEsmSyntax, loadBuiltinModule, makeRequireFunction, normalizeReferrerURL, @@ -1315,7 +1315,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) { } catch (err) { if (process.mainModule === cjsModuleInstance) { const { enrichCJSError } = require('internal/modules/esm/translators'); - enrichCJSError(err, content); + enrichCJSError(err, content, filename); } throw err; } @@ -1400,10 +1400,11 @@ Module._extensions['.js'] = function(module, filename) { const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null }; // Function require shouldn't be used in ES modules. if (pkg.data?.type === 'module') { + // This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed. const parent = moduleParentCache.get(module); const parentPath = parent?.filename; const packageJsonPath = path.resolve(pkg.path, 'package.json'); - const usesEsm = hasEsmSyntax(content); + const usesEsm = containsModuleSyntax(content, filename); const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath, packageJsonPath); // Attempt to reconstruct the parent require frame. diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index c36fe07a9503ae..7a62615cfe4210 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -30,11 +30,11 @@ function lazyTypes() { return _TYPES = require('internal/util/types'); } +const { containsModuleSyntax } = internalBinding('contextify'); const assert = require('internal/assert'); const { readFileSync } = require('fs'); const { dirname, extname, isAbsolute } = require('path'); const { - hasEsmSyntax, loadBuiltinModule, stripBOM, } = require('internal/modules/helpers'); @@ -166,11 +166,11 @@ translators.set('module', async function moduleStrategy(url, source, isMain) { * Provide a more informative error for CommonJS imports. * @param {Error | any} err * @param {string} [content] Content of the file, if known. - * @param {string} [filename] Useful only if `content` is unknown. + * @param {string} [filename] The filename of the erroring module. */ function enrichCJSError(err, content, filename) { if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype && - hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) { + containsModuleSyntax(content, filename)) { // Emit the warning synchronously because we are in the middle of handling // a SyntaxError that will throw and likely terminate the process before an // asynchronous warning would be emitted. @@ -217,7 +217,7 @@ function loadCJSModule(module, source, url, filename) { importModuleDynamically, // importModuleDynamically ).function; } catch (err) { - enrichCJSError(err, source, url); + enrichCJSError(err, source, filename); throw err; } @@ -344,7 +344,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source, assert(module === CJSModule._cache[filename]); CJSModule._load(filename); } catch (err) { - enrichCJSError(err, source, url); + enrichCJSError(err, source, filename); throw err; } } : loadCJSModule; diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 7f2959cc469dc1..6b30a1d8c76d4b 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -3,7 +3,6 @@ const { ArrayPrototypeForEach, ArrayPrototypeJoin, - ArrayPrototypeSome, ObjectDefineProperty, ObjectPrototypeHasOwnProperty, SafeMap, @@ -299,32 +298,10 @@ function normalizeReferrerURL(referrer) { return new URL(referrer).href; } -/** - * For error messages only, check if ESM syntax is in use. - * @param {string} code - */ -function hasEsmSyntax(code) { - debug('Checking for ESM syntax'); - const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser; - let root; - try { - root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' }); - } catch { - return false; - } - - return ArrayPrototypeSome(root.body, (stmt) => - stmt.type === 'ExportDefaultDeclaration' || - stmt.type === 'ExportNamedDeclaration' || - stmt.type === 'ImportDeclaration' || - stmt.type === 'ExportAllDeclaration'); -} - module.exports = { addBuiltinLibsToObject, getCjsConditions, initializeCjsConditions, - hasEsmSyntax, loadBuiltinModule, makeRequireFunction, normalizeReferrerURL, diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 4c7988eaed7c9e..35c628f4210d3a 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -318,6 +318,7 @@ void ContextifyContext::CreatePerIsolateProperties( SetMethod(isolate, target, "makeContext", MakeContext); SetMethod(isolate, target, "isContext", IsContext); SetMethod(isolate, target, "compileFunction", CompileFunction); + SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax); } void ContextifyContext::RegisterExternalReferences( @@ -325,6 +326,7 @@ void ContextifyContext::RegisterExternalReferences( registry->Register(MakeContext); registry->Register(IsContext); registry->Register(CompileFunction); + registry->Register(ContainsModuleSyntax); registry->Register(PropertyGetterCallback); registry->Register(PropertySetterCallback); registry->Register(PropertyDescriptorCallback); @@ -1204,33 +1206,18 @@ void ContextifyContext::CompileFunction( data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength()); } - // Set host_defined_options Local host_defined_options = - PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - host_defined_options->Set( - isolate, loader::HostDefinedOptions::kID, id_symbol); - - ScriptOrigin origin(isolate, - filename, - line_offset, // line offset - column_offset, // column offset - true, // is cross origin - -1, // script id - Local(), // source map URL - false, // is opaque (?) - false, // is WASM - false, // is ES Module - host_defined_options); - - ScriptCompiler::Source source(code, origin, cached_data); - ScriptCompiler::CompileOptions options; - if (source.GetCachedData() == nullptr) { - options = ScriptCompiler::kNoCompileOptions; - } else { - options = ScriptCompiler::kConsumeCodeCache; - } + GetHostDefinedOptions(isolate, id_symbol); + ScriptCompiler::Source source = + GetCommonJSSourceInstance(isolate, + code, + filename, + line_offset, + column_offset, + host_defined_options, + cached_data); + ScriptCompiler::CompileOptions options = GetCompileOptions(source); - TryCatchScope try_catch(env); Context::Scope scope(parsing_context); // Read context extensions from buffer @@ -1255,9 +1242,83 @@ void ContextifyContext::CompileFunction( } } + TryCatchScope try_catch(env); + Local result = CompileFunctionAndCacheResult(env, + parsing_context, + &source, + params, + context_extensions, + options, + produce_cached_data, + id_symbol, + try_catch); + + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + try_catch.ReThrow(); + return; + } + + if (result.IsEmpty()) { + return; + } + args.GetReturnValue().Set(result); +} + +Local ContextifyContext::GetHostDefinedOptions( + Isolate* isolate, Local id_symbol) { + Local host_defined_options = + PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); + host_defined_options->Set( + isolate, loader::HostDefinedOptions::kID, id_symbol); + return host_defined_options; +} + +ScriptCompiler::Source ContextifyContext::GetCommonJSSourceInstance( + Isolate* isolate, + Local code, + Local filename, + int line_offset, + int column_offset, + Local host_defined_options, + ScriptCompiler::CachedData* cached_data) { + ScriptOrigin origin(isolate, + filename, + line_offset, // line offset + column_offset, // column offset + true, // is cross origin + -1, // script id + Local(), // source map URL + false, // is opaque (?) + false, // is WASM + false, // is ES Module + host_defined_options); + return ScriptCompiler::Source(code, origin, cached_data); +} + +ScriptCompiler::CompileOptions ContextifyContext::GetCompileOptions( + const ScriptCompiler::Source& source) { + ScriptCompiler::CompileOptions options; + if (source.GetCachedData() != nullptr) { + options = ScriptCompiler::kConsumeCodeCache; + } else { + options = ScriptCompiler::kNoCompileOptions; + } + return options; +} + +Local ContextifyContext::CompileFunctionAndCacheResult( + Environment* env, + Local parsing_context, + ScriptCompiler::Source* source, + std::vector> params, + std::vector> context_extensions, + ScriptCompiler::CompileOptions options, + bool produce_cached_data, + Local id_symbol, + const TryCatchScope& try_catch) { MaybeLocal maybe_fn = ScriptCompiler::CompileFunction( parsing_context, - &source, + source, params.size(), params.data(), context_extensions.size(), @@ -1269,24 +1330,26 @@ void ContextifyContext::CompileFunction( if (!maybe_fn.ToLocal(&fn)) { if (try_catch.HasCaught() && !try_catch.HasTerminated()) { errors::DecorateErrorStack(env, try_catch); - try_catch.ReThrow(); + return Object::New(env->isolate()); } - return; } + + Local context = env->context(); if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) .IsNothing()) { - return; + return Object::New(env->isolate()); } + Isolate* isolate = env->isolate(); Local result = Object::New(isolate); if (result->Set(parsing_context, env->function_string(), fn).IsNothing()) - return; + return Object::New(env->isolate()); if (result ->Set(parsing_context, env->source_map_url_string(), fn->GetScriptOrigin().SourceMapUrl()) .IsNothing()) - return; + return Object::New(env->isolate()); std::unique_ptr new_cached_data; if (produce_cached_data) { @@ -1295,14 +1358,91 @@ void ContextifyContext::CompileFunction( if (StoreCodeCacheResult(env, result, options, - source, + *source, produce_cached_data, std::move(new_cached_data)) .IsNothing()) { - return; + return Object::New(env->isolate()); } - args.GetReturnValue().Set(result); + return result; +} + +// When compiling as CommonJS source code that contains ESM syntax, the +// following error messages are returned: +// - `import` statements: "Cannot use import statement outside a module" +// - `export` statements: "Unexpected token 'export'" +// - `import.meta` references: "Cannot use 'import.meta' outside a module" +// Dynamic `import()` is permitted in CommonJS, so it does not error. +// While top-level `await` is not permitted in CommonJS, it returns the same +// error message as when `await` is used in a sync function, so we don't use it +// as a disambiguation. +constexpr std::array esm_syntax_error_messages = { + "Cannot use import statement outside a module", // `import` statements + "Unexpected token 'export'", // `export` statements + "Cannot use 'import.meta' outside a module"}; // `import.meta` references + +void ContextifyContext::ContainsModuleSyntax( + const FunctionCallbackInfo& args) { + // Argument 1: source code + CHECK(args[0]->IsString()); + Local code = args[0].As(); + + // Argument 2: filename + Local filename = String::Empty(args.GetIsolate()); + if (!args[1]->IsUndefined()) { + CHECK(args[1]->IsString()); + filename = args[1].As(); + } + + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + Local context = env->context(); + + // TODO(geoffreybooth): Centralize this rather than matching the logic in + // cjs/loader.js and translators.js + Local script_id = String::Concat( + isolate, String::NewFromUtf8(isolate, "cjs:").ToLocalChecked(), filename); + Local id_symbol = Symbol::New(isolate, script_id); + + Local host_defined_options = + GetHostDefinedOptions(isolate, id_symbol); + ScriptCompiler::Source source = GetCommonJSSourceInstance( + isolate, code, filename, 0, 0, host_defined_options, nullptr); + ScriptCompiler::CompileOptions options = GetCompileOptions(source); + + std::vector> params = { + String::NewFromUtf8(isolate, "exports").ToLocalChecked(), + String::NewFromUtf8(isolate, "require").ToLocalChecked(), + String::NewFromUtf8(isolate, "module").ToLocalChecked(), + String::NewFromUtf8(isolate, "__filename").ToLocalChecked(), + String::NewFromUtf8(isolate, "__dirname").ToLocalChecked()}; + + TryCatchScope try_catch(env); + + ContextifyContext::CompileFunctionAndCacheResult(env, + context, + &source, + params, + std::vector>(), + options, + true, + id_symbol, + try_catch); + + bool found_error_message_caused_by_module_syntax = false; + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + Utf8Value message_value(env->isolate(), try_catch.Message()->Get()); + auto message = message_value.ToStringView(); + + for (const auto& error_message : esm_syntax_error_messages) { + if (message.find(error_message) != std::string_view::npos) { + found_error_message_caused_by_module_syntax = true; + break; + } + } + } + args.GetReturnValue().Set(found_error_message_caused_by_module_syntax); } static void StartSigintWatchdog(const FunctionCallbackInfo& args) { diff --git a/src/node_contextify.h b/src/node_contextify.h index d1dddbf374d563..721c146ff88c35 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -83,6 +83,30 @@ class ContextifyContext : public BaseObject { static void IsContext(const v8::FunctionCallbackInfo& args); static void CompileFunction( const v8::FunctionCallbackInfo& args); + static v8::Local CompileFunctionAndCacheResult( + Environment* env, + v8::Local parsing_context, + v8::ScriptCompiler::Source* source, + std::vector> params, + std::vector> context_extensions, + v8::ScriptCompiler::CompileOptions options, + bool produce_cached_data, + v8::Local id_symbol, + const errors::TryCatchScope& try_catch); + static v8::Local GetHostDefinedOptions( + v8::Isolate* isolate, v8::Local id_symbol); + static v8::ScriptCompiler::Source GetCommonJSSourceInstance( + v8::Isolate* isolate, + v8::Local code, + v8::Local filename, + int line_offset, + int column_offset, + v8::Local host_defined_options, + v8::ScriptCompiler::CachedData* cached_data); + static v8::ScriptCompiler::CompileOptions GetCompileOptions( + const v8::ScriptCompiler::Source& source); + static void ContainsModuleSyntax( + const v8::FunctionCallbackInfo& args); static void WeakCallback( const v8::WeakCallbackInfo& data); static void PropertyGetterCallback( From 8efb75fd803a914fd7d77f31399f6ed718287da8 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 18 Oct 2023 11:01:51 -0400 Subject: [PATCH 24/87] test: set `test-runner-watch-mode` as flaky PR-URL: https://github.com/nodejs/node/pull/50221 Refs: https://github.com/nodejs/node/issues/49985 Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: Robert Nagy --- test/parallel/parallel.status | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index ffe3505021b522..dd25e712512b23 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -11,6 +11,8 @@ prefix parallel test-crypto-keygen: PASS,FLAKY # https://github.com/nodejs/node/issues/41201 test-fs-rmdir-recursive: PASS, FLAKY +# https://github.com/nodejs/node/issues/49985 +test-runner-watch-mode: PASS, FLAKY # Windows on ARM [$system==win32 && $arch==arm64] From a38d1311bf034d4bfd7ce1d5f15c78443dd5e5d4 Mon Sep 17 00:00:00 2001 From: Stefan Stojanovic Date: Wed, 18 Oct 2023 17:23:16 +0200 Subject: [PATCH 25/87] test: set test-worker-nearheaplimit-deadlock flaky MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test is only flaky on x86 Windows. Fixes: https://github.com/nodejs/node/issues/50220 PR-URL: https://github.com/nodejs/node/pull/50238 Refs: https://github.com/nodejs/node/pull/49962 Reviewed-By: Richard Lau Reviewed-By: Yagiz Nizipli Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Michael Dawson --- test/parallel/parallel.status | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index dd25e712512b23..a0ff5d69361cb5 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -14,6 +14,10 @@ test-fs-rmdir-recursive: PASS, FLAKY # https://github.com/nodejs/node/issues/49985 test-runner-watch-mode: PASS, FLAKY +# Windows on x86 +[$system==win32 && $arch==x86] +test-worker-nearheaplimit-deadlock: PASS, FLAKY + # Windows on ARM [$system==win32 && $arch==arm64] From 610036c67d7865415664f357332ad9bd3f8f65ac Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:08:40 -0400 Subject: [PATCH 26/87] fs: improve error performance of `renameSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- benchmark/fs/bench-renameSync.js | 49 ++++++++++++++++++++++++++++++++ lib/fs.js | 8 +++--- src/node_file.cc | 15 +++++----- typings/internalBinding/fs.d.ts | 1 + 4 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 benchmark/fs/bench-renameSync.js diff --git a/benchmark/fs/bench-renameSync.js b/benchmark/fs/bench-renameSync.js new file mode 100644 index 00000000000000..9f9f5e4e84cda0 --- /dev/null +++ b/benchmark/fs/bench-renameSync.js @@ -0,0 +1,49 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const tmpdir = require('../../test/common/tmpdir'); + +const bench = common.createBenchmark(main, { + type: ['invalid', 'valid'], + n: [2e3], +}); + +function main({ n, type }) { + switch (type) { + case 'invalid': { + let hasError = false; + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs.renameSync(tmpdir.resolve(`.non-existing-file-${i}`), tmpdir.resolve(`.new-file-${i}`)); + } catch { + hasError = true; + } + } + bench.end(n); + assert(hasError); + break; + } + case 'valid': { + tmpdir.refresh(); + for (let i = 0; i < n; i++) { + fs.writeFileSync(tmpdir.resolve(`.existing-file-${i}`), 'bench', 'utf8'); + } + + bench.start(); + for (let i = 0; i < n; i++) { + fs.renameSync( + tmpdir.resolve(`.existing-file-${i}`), + tmpdir.resolve(`.new-existing-file-${i}`), + ); + } + + bench.end(n); + break; + } + default: + throw new Error('Invalid type'); + } +} diff --git a/lib/fs.js b/lib/fs.js index 2d5170cd2f44b0..ae1b831775d707 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1034,10 +1034,10 @@ function rename(oldPath, newPath, callback) { function renameSync(oldPath, newPath) { oldPath = getValidatedPath(oldPath, 'oldPath'); newPath = getValidatedPath(newPath, 'newPath'); - const ctx = { path: oldPath, dest: newPath }; - binding.rename(pathModule.toNamespacedPath(oldPath), - pathModule.toNamespacedPath(newPath), undefined, ctx); - handleErrorFromBinding(ctx); + binding.rename( + pathModule.toNamespacedPath(oldPath), + pathModule.toNamespacedPath(newPath), + ); } /** diff --git a/src/node_file.cc b/src/node_file.cc index afaa028784e710..1c24f1614b46f2 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1432,7 +1432,7 @@ static void Rename(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); const int argc = args.Length(); - CHECK_GE(argc, 3); + CHECK_GE(argc, 2); BufferValue old_path(isolate, args[0]); CHECK_NOT_NULL(*old_path); @@ -1449,8 +1449,8 @@ static void Rename(const FunctionCallbackInfo& args) { permission::PermissionScope::kFileSystemWrite, new_path.ToStringView()); - FSReqBase* req_wrap_async = GetReqWrap(args, 2); - if (req_wrap_async != nullptr) { + if (argc > 2) { // rename(old_path, new_path, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 2); FS_ASYNC_TRACE_BEGIN2(UV_FS_RENAME, req_wrap_async, "old_path", @@ -1460,12 +1460,11 @@ static void Rename(const FunctionCallbackInfo& args) { AsyncDestCall(env, req_wrap_async, args, "rename", *new_path, new_path.length(), UTF8, AfterNoArgs, uv_fs_rename, *old_path, *new_path); - } else { - CHECK_EQ(argc, 4); - FSReqWrapSync req_wrap_sync; + } else { // rename(old_path, new_path) + FSReqWrapSync req_wrap_sync("rename", *old_path, *new_path); FS_SYNC_TRACE_BEGIN(rename); - SyncCall(env, args[3], &req_wrap_sync, "rename", uv_fs_rename, - *old_path, *new_path); + SyncCallAndThrowOnError( + env, &req_wrap_sync, uv_fs_rename, *old_path, *new_path); FS_SYNC_TRACE_END(rename); } } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index c4dd973293ab22..bdab2e80be7f26 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -183,6 +183,7 @@ declare namespace InternalFSBinding { function rename(oldPath: string, newPath: string, req: FSReqCallback): void; function rename(oldPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void; function rename(oldPath: string, newPath: string, usePromises: typeof kUsePromises): Promise; + function rename(oldPath: string, newPath: string): void; function rmdir(path: string, req: FSReqCallback): void; function rmdir(path: string, req: undefined, ctx: FSSyncContext): void; From f766c04856cdfbe5546aae67ac7c6b0f6d42cc1d Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:10:04 -0400 Subject: [PATCH 27/87] fs: improve error performance of `chownSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- benchmark/fs/bench-chownSync.js | 54 +++++++++++++++++++++++++++++++++ lib/fs.js | 8 +++-- src/node_file.cc | 12 +++----- typings/internalBinding/fs.d.ts | 1 + 4 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 benchmark/fs/bench-chownSync.js diff --git a/benchmark/fs/bench-chownSync.js b/benchmark/fs/bench-chownSync.js new file mode 100644 index 00000000000000..d07f3b65949979 --- /dev/null +++ b/benchmark/fs/bench-chownSync.js @@ -0,0 +1,54 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const tmpdir = require('../../test/common/tmpdir'); + +if (process.platform === 'win32') { + console.log('Skipping: Windows does not have `getuid` or `getgid`'); + process.exit(0); +} + +const bench = common.createBenchmark(main, { + type: ['existing', 'non-existing'], + method: ['chownSync', 'lchownSync'], + n: [1e4], +}); + +function main({ n, type, method }) { + const uid = process.getuid(); + const gid = process.getgid(); + const fsMethod = fs[method]; + + switch (type) { + case 'existing': { + tmpdir.refresh(); + const tmpfile = tmpdir.resolve(`.existing-file-${process.pid}`); + fs.writeFileSync(tmpfile, 'this-is-for-a-benchmark', 'utf8'); + bench.start(); + for (let i = 0; i < n; i++) { + fsMethod(tmpfile, uid, gid); + } + bench.end(n); + break; + } + case 'non-existing': { + const path = tmpdir.resolve(`.non-existing-file-${Date.now()}`); + let hasError = false; + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs[method](path, uid, gid); + } catch { + hasError = true; + } + } + bench.end(n); + assert(hasError); + break; + } + default: + new Error('Invalid type'); + } +} diff --git a/lib/fs.js b/lib/fs.js index ae1b831775d707..dc733d3ce66b86 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2089,9 +2089,11 @@ function chownSync(path, uid, gid) { path = getValidatedPath(path); validateInteger(uid, 'uid', -1, kMaxUserId); validateInteger(gid, 'gid', -1, kMaxUserId); - const ctx = { path }; - binding.chown(pathModule.toNamespacedPath(path), uid, gid, undefined, ctx); - handleErrorFromBinding(ctx); + binding.chown( + pathModule.toNamespacedPath(path), + uid, + gid, + ); } /** diff --git a/src/node_file.cc b/src/node_file.cc index 1c24f1614b46f2..926fb1e65b9ae9 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2572,18 +2572,16 @@ static void Chown(const FunctionCallbackInfo& args) { CHECK(IsSafeJsInt(args[2])); const uv_gid_t gid = static_cast(args[2].As()->Value()); - FSReqBase* req_wrap_async = GetReqWrap(args, 3); - if (req_wrap_async != nullptr) { // chown(path, uid, gid, req) + if (argc > 3) { // chown(path, uid, gid, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 3); FS_ASYNC_TRACE_BEGIN1( UV_FS_CHOWN, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, req_wrap_async, args, "chown", UTF8, AfterNoArgs, uv_fs_chown, *path, uid, gid); - } else { // chown(path, uid, gid, undefined, ctx) - CHECK_EQ(argc, 5); - FSReqWrapSync req_wrap_sync; + } else { // chown(path, uid, gid) + FSReqWrapSync req_wrap_sync("chown", *path); FS_SYNC_TRACE_BEGIN(chown); - SyncCall(env, args[4], &req_wrap_sync, "chown", - uv_fs_chown, *path, uid, gid); + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_chown, *path, uid, gid); FS_SYNC_TRACE_END(chown); } } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index bdab2e80be7f26..0f5c29a13025b5 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -67,6 +67,7 @@ declare namespace InternalFSBinding { function chown(path: string, uid: number, gid: number, req: FSReqCallback): void; function chown(path: string, uid: number, gid: number, req: undefined, ctx: FSSyncContext): void; function chown(path: string, uid: number, gid: number, usePromises: typeof kUsePromises): Promise; + function chown(path: string, uid: number, gid: number): void; function close(fd: number, req: FSReqCallback): void; function close(fd: number, req: undefined, ctx: FSSyncContext): void; From 81f15274e2d5b98eb1046f086f8983bd8737e8e2 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:10:49 -0400 Subject: [PATCH 28/87] fs: improve error performance of `linkSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- benchmark/fs/bench-linkSync.js | 50 +++++++++++++++++++++++++++++++++ lib/fs.js | 10 +++---- src/node_file.cc | 12 ++++---- typings/internalBinding/fs.d.ts | 1 + 4 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 benchmark/fs/bench-linkSync.js diff --git a/benchmark/fs/bench-linkSync.js b/benchmark/fs/bench-linkSync.js new file mode 100644 index 00000000000000..7c3ef29291913e --- /dev/null +++ b/benchmark/fs/bench-linkSync.js @@ -0,0 +1,50 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const tmpdir = require('../../test/common/tmpdir'); + +tmpdir.refresh(); +const tmpfile = tmpdir.resolve(`.bench-file-data-${Date.now()}`); +fs.writeFileSync(tmpfile, 'bench-file', 'utf-8'); + +const bench = common.createBenchmark(main, { + type: ['valid', 'invalid'], + n: [1e3], +}); + +function main({ n, type }) { + switch (type) { + case 'valid': { + bench.start(); + for (let i = 0; i < n; i++) { + fs.linkSync(tmpfile, tmpdir.resolve(`.valid-${i}`), 'file'); + } + bench.end(n); + + break; + } + + case 'invalid': { + let hasError = false; + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs.linkSync( + tmpdir.resolve(`.non-existing-file-for-linkSync-${i}`), + __filename, + 'file', + ); + } catch { + hasError = true; + } + } + bench.end(n); + assert(hasError); + break; + } + default: + new Error('Invalid type'); + } +} diff --git a/lib/fs.js b/lib/fs.js index dc733d3ce66b86..16a61253dc7311 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1847,12 +1847,10 @@ function linkSync(existingPath, newPath) { existingPath = getValidatedPath(existingPath, 'existingPath'); newPath = getValidatedPath(newPath, 'newPath'); - const ctx = { path: existingPath, dest: newPath }; - const result = binding.link(pathModule.toNamespacedPath(existingPath), - pathModule.toNamespacedPath(newPath), - undefined, ctx); - handleErrorFromBinding(ctx); - return result; + binding.link( + pathModule.toNamespacedPath(existingPath), + pathModule.toNamespacedPath(newPath), + ); } /** diff --git a/src/node_file.cc b/src/node_file.cc index 926fb1e65b9ae9..b98a3eac111b31 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1342,7 +1342,7 @@ static void Link(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); const int argc = args.Length(); - CHECK_GE(argc, 3); + CHECK_GE(argc, 2); BufferValue src(isolate, args[0]); CHECK_NOT_NULL(*src); @@ -1360,8 +1360,8 @@ static void Link(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemWrite, dest_view); - FSReqBase* req_wrap_async = GetReqWrap(args, 2); - if (req_wrap_async != nullptr) { // link(src, dest, req) + if (argc > 2) { // link(src, dest, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 2); FS_ASYNC_TRACE_BEGIN2(UV_FS_LINK, req_wrap_async, "src", @@ -1371,11 +1371,9 @@ static void Link(const FunctionCallbackInfo& args) { AsyncDestCall(env, req_wrap_async, args, "link", *dest, dest.length(), UTF8, AfterNoArgs, uv_fs_link, *src, *dest); } else { // link(src, dest) - CHECK_EQ(argc, 4); - FSReqWrapSync req_wrap_sync; + FSReqWrapSync req_wrap_sync("link", *src, *dest); FS_SYNC_TRACE_BEGIN(link); - SyncCall(env, args[3], &req_wrap_sync, "link", - uv_fs_link, *src, *dest); + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_link, *src, *dest); FS_SYNC_TRACE_END(link); } } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 0f5c29a13025b5..23592809f2c570 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -121,6 +121,7 @@ declare namespace InternalFSBinding { function link(existingPath: string, newPath: string, req: FSReqCallback): void; function link(existingPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void; function link(existingPath: string, newPath: string, usePromises: typeof kUsePromises): Promise; + function link(existingPath: string, newPath: string): void; function lstat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback): void; function lstat(path: StringOrBuffer, useBigint: true, req: FSReqCallback): void; From 275987841ed0ee17201b3c1d32e8d80ea1cf98e8 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:11:44 -0400 Subject: [PATCH 29/87] fs: improve error performance of `mkdtempSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- lib/fs.js | 6 +----- src/node_file.cc | 23 +++++++++++------------ typings/internalBinding/fs.d.ts | 1 + 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 16a61253dc7311..36d55e6e4989ca 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2966,11 +2966,7 @@ function mkdtempSync(prefix, options) { path = Buffer.concat([prefix, Buffer.from('XXXXXX')]); } - const ctx = { path }; - const result = binding.mkdtemp(path, options.encoding, - undefined, ctx); - handleErrorFromBinding(ctx); - return result; + return binding.mkdtemp(path, options.encoding); } /** diff --git a/src/node_file.cc b/src/node_file.cc index b98a3eac111b31..93af6f46e0ba79 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2760,27 +2760,26 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(args, 2); - if (req_wrap_async != nullptr) { // mkdtemp(tmpl, encoding, req) + if (argc > 2) { // mkdtemp(tmpl, encoding, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 2); FS_ASYNC_TRACE_BEGIN1( UV_FS_MKDTEMP, req_wrap_async, "path", TRACE_STR_COPY(*tmpl)) AsyncCall(env, req_wrap_async, args, "mkdtemp", encoding, AfterStringPath, uv_fs_mkdtemp, *tmpl); - } else { // mkdtemp(tmpl, encoding, undefined, ctx) - CHECK_EQ(argc, 4); - FSReqWrapSync req_wrap_sync; + } else { // mkdtemp(tmpl, encoding) + FSReqWrapSync req_wrap_sync("mkdtemp", *tmpl); FS_SYNC_TRACE_BEGIN(mkdtemp); - SyncCall(env, args[3], &req_wrap_sync, "mkdtemp", - uv_fs_mkdtemp, *tmpl); + int result = + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_mkdtemp, *tmpl); FS_SYNC_TRACE_END(mkdtemp); - const char* path = req_wrap_sync.req.path; - + if (is_uv_error(result)) { + return; + } Local error; MaybeLocal rc = - StringBytes::Encode(isolate, path, encoding, &error); + StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding, &error); if (rc.IsEmpty()) { - Local ctx = args[3].As(); - ctx->Set(env->context(), env->error_string(), error).Check(); + env->isolate()->ThrowException(error); return; } args.GetReturnValue().Set(rc.ToLocalChecked()); diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 23592809f2c570..2c03c9934d2775 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -140,6 +140,7 @@ declare namespace InternalFSBinding { function mkdtemp(prefix: string, encoding: unknown, req: FSReqCallback): void; function mkdtemp(prefix: string, encoding: unknown, req: undefined, ctx: FSSyncContext): string; function mkdtemp(prefix: string, encoding: unknown, usePromises: typeof kUsePromises): Promise; + function mkdtemp(prefix: string, encoding: unknown): string; function mkdir(path: string, mode: number, recursive: boolean, req: FSReqCallback): void; function mkdir(path: string, mode: number, recursive: true, req: FSReqCallback): void; From bc6f279261d6b07cd01bdeacd1d095566e3a895f Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:12:17 -0400 Subject: [PATCH 30/87] fs: improve error performance of `readlinkSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- benchmark/fs/bench-readlinkSync.js | 54 ++++++++++++++++++++++++++++++ lib/fs.js | 13 +++---- src/node_file.cc | 20 +++++------ typings/internalBinding/fs.d.ts | 1 + 4 files changed, 69 insertions(+), 19 deletions(-) create mode 100644 benchmark/fs/bench-readlinkSync.js diff --git a/benchmark/fs/bench-readlinkSync.js b/benchmark/fs/bench-readlinkSync.js new file mode 100644 index 00000000000000..15c22273e557b6 --- /dev/null +++ b/benchmark/fs/bench-readlinkSync.js @@ -0,0 +1,54 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const tmpdir = require('../../test/common/tmpdir'); + +if (process.platform === 'win32') { + console.log('Skipping: Windows does not play well with `symlinkSync`'); + process.exit(0); +} + +const bench = common.createBenchmark(main, { + type: ['valid', 'invalid'], + n: [1e3], +}); + +function main({ n, type }) { + switch (type) { + case 'valid': { + tmpdir.refresh(); + const tmpfile = tmpdir.resolve(`.readlink-file-${process.pid}`); + fs.writeFileSync(tmpfile, 'data', 'utf8'); + let returnValue; + for (let i = 0; i < n; i++) { + fs.symlinkSync(tmpfile, tmpdir.resolve(`.readlink-sync-${i}`), 'file'); + } + bench.start(); + for (let i = 0; i < n; i++) { + returnValue = fs.readlinkSync(tmpdir.resolve(`.readlink-sync-${i}`), { encoding: 'utf8' }); + } + bench.end(n); + assert(returnValue); + break; + } + + case 'invalid': { + let hasError = false; + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs.readlinkSync(tmpdir.resolve('.non-existing-file-for-readlinkSync')); + } catch { + hasError = true; + } + } + bench.end(n); + assert(hasError); + break; + } + default: + new Error('Invalid type'); + } +} diff --git a/lib/fs.js b/lib/fs.js index 36d55e6e4989ca..15c9623f2cb840 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1728,11 +1728,10 @@ function readlink(path, options, callback) { function readlinkSync(path, options) { options = getOptions(options); path = getValidatedPath(path, 'oldPath'); - const ctx = { path }; - const result = binding.readlink(pathModule.toNamespacedPath(path), - options.encoding, undefined, ctx); - handleErrorFromBinding(ctx); - return result; + return binding.readlink( + pathModule.toNamespacedPath(path), + options.encoding, + ); } /** @@ -2714,10 +2713,8 @@ function realpathSync(p, options) { } } if (linkTarget === null) { - const ctx = { path: base }; binding.stat(baseLong, false, undefined, true); - linkTarget = binding.readlink(baseLong, undefined, undefined, ctx); - handleErrorFromBinding(ctx); + linkTarget = binding.readlink(baseLong, undefined); } resolvedLink = pathModule.resolve(previous, linkTarget); diff --git a/src/node_file.cc b/src/node_file.cc index 93af6f46e0ba79..8b0f46c00c8685 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1383,7 +1383,7 @@ static void ReadLink(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); const int argc = args.Length(); - CHECK_GE(argc, 3); + CHECK_GE(argc, 2); BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); @@ -1392,21 +1392,20 @@ static void ReadLink(const FunctionCallbackInfo& args) { const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(args, 2); - if (req_wrap_async != nullptr) { // readlink(path, encoding, req) + if (argc > 2) { // readlink(path, encoding, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 2); FS_ASYNC_TRACE_BEGIN1( UV_FS_READLINK, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, req_wrap_async, args, "readlink", encoding, AfterStringPtr, uv_fs_readlink, *path); - } else { - CHECK_EQ(argc, 4); - FSReqWrapSync req_wrap_sync; + } else { // readlink(path, encoding) + FSReqWrapSync req_wrap_sync("readlink", *path); FS_SYNC_TRACE_BEGIN(readlink); - int err = SyncCall(env, args[3], &req_wrap_sync, "readlink", - uv_fs_readlink, *path); + int err = + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_readlink, *path); FS_SYNC_TRACE_END(readlink); if (err < 0) { - return; // syscall failed, no need to continue, error info is in ctx + return; } const char* link_path = static_cast(req_wrap_sync.req.ptr); @@ -1416,8 +1415,7 @@ static void ReadLink(const FunctionCallbackInfo& args) { encoding, &error); if (rc.IsEmpty()) { - Local ctx = args[3].As(); - ctx->Set(env->context(), env->error_string(), error).Check(); + env->isolate()->ThrowException(error); return; } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 2c03c9934d2775..619cd255f10aee 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -178,6 +178,7 @@ declare namespace InternalFSBinding { function readlink(path: StringOrBuffer, encoding: unknown, req: FSReqCallback): void; function readlink(path: StringOrBuffer, encoding: unknown, req: undefined, ctx: FSSyncContext): string | Buffer; function readlink(path: StringOrBuffer, encoding: unknown, usePromises: typeof kUsePromises): Promise; + function readlink(path: StringOrBuffer, encoding: unknown): StringOrBuffer; function realpath(path: StringOrBuffer, encoding: unknown, req: FSReqCallback): void; function realpath(path: StringOrBuffer, encoding: unknown, req: undefined, ctx: FSSyncContext): string | Buffer; From dc9ac8d41c21ede59359d122930fda7ac5c87748 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:12:42 -0400 Subject: [PATCH 31/87] fs: improve error performance of `symlinkSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- benchmark/fs/bench-symlinkSync.js | 51 +++++++++++++++++++++++++++++++ lib/fs.js | 11 +++---- src/node_file.cc | 13 ++++---- typings/internalBinding/fs.d.ts | 1 + 4 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 benchmark/fs/bench-symlinkSync.js diff --git a/benchmark/fs/bench-symlinkSync.js b/benchmark/fs/bench-symlinkSync.js new file mode 100644 index 00000000000000..5bf4e0e50779d9 --- /dev/null +++ b/benchmark/fs/bench-symlinkSync.js @@ -0,0 +1,51 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const tmpdir = require('../../test/common/tmpdir'); + +if (process.platform === 'win32') { + console.log('Skipping: Windows does not play well with `symlink`'); + process.exit(0); +} + +const bench = common.createBenchmark(main, { + type: ['valid', 'invalid'], + n: [1e3], +}); + +function main({ n, type }) { + switch (type) { + case 'valid': { + tmpdir.refresh(); + bench.start(); + for (let i = 0; i < n; i++) { + fs.symlinkSync(tmpdir.resolve('.non-existent-symlink-file'), tmpdir.resolve(`.valid-${i}`), 'file'); + } + bench.end(n); + break; + } + + case 'invalid': { + let hasError = false; + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs.symlinkSync( + tmpdir.resolve('.non-existent-symlink-file'), + __filename, + 'file', + ); + } catch { + hasError = true; + } + } + bench.end(n); + assert(hasError); + break; + } + default: + new Error('Invalid type'); + } +} diff --git a/lib/fs.js b/lib/fs.js index 15c9623f2cb840..f04f2d69330c2e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1804,13 +1804,12 @@ function symlinkSync(target, path, type) { } target = getValidatedPath(target, 'target'); path = getValidatedPath(path); - const flags = stringToSymlinkType(type); - - const ctx = { path: target, dest: path }; - binding.symlink(preprocessSymlinkDestination(target, type, path), - pathModule.toNamespacedPath(path), flags, undefined, ctx); - handleErrorFromBinding(ctx); + binding.symlink( + preprocessSymlinkDestination(target, type, path), + pathModule.toNamespacedPath(path), + stringToSymlinkType(type), + ); } /** diff --git a/src/node_file.cc b/src/node_file.cc index 8b0f46c00c8685..50a3fd29552138 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1298,7 +1298,7 @@ static void Symlink(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); const int argc = args.Length(); - CHECK_GE(argc, 4); + CHECK_GE(argc, 3); BufferValue target(isolate, args[0]); CHECK_NOT_NULL(*target); @@ -1317,8 +1317,8 @@ static void Symlink(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); int flags = args[2].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(args, 3); - if (req_wrap_async != nullptr) { // symlink(target, path, flags, req) + if (argc > 3) { // symlink(target, path, flags, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 3); FS_ASYNC_TRACE_BEGIN2(UV_FS_SYMLINK, req_wrap_async, "target", @@ -1328,11 +1328,10 @@ static void Symlink(const FunctionCallbackInfo& args) { AsyncDestCall(env, req_wrap_async, args, "symlink", *path, path.length(), UTF8, AfterNoArgs, uv_fs_symlink, *target, *path, flags); } else { // symlink(target, path, flags, undefined, ctx) - CHECK_EQ(argc, 5); - FSReqWrapSync req_wrap_sync; + FSReqWrapSync req_wrap_sync("symlink", *target, *path); FS_SYNC_TRACE_BEGIN(symlink); - SyncCall(env, args[4], &req_wrap_sync, "symlink", - uv_fs_symlink, *target, *path, flags); + SyncCallAndThrowOnError( + env, &req_wrap_sync, uv_fs_symlink, *target, *path, flags); FS_SYNC_TRACE_END(symlink); } } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 619cd255f10aee..8aab4a3ae8d242 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -206,6 +206,7 @@ declare namespace InternalFSBinding { function symlink(target: StringOrBuffer, path: StringOrBuffer, type: number, req: FSReqCallback): void; function symlink(target: StringOrBuffer, path: StringOrBuffer, type: number, req: undefined, ctx: FSSyncContext): void; function symlink(target: StringOrBuffer, path: StringOrBuffer, type: number, usePromises: typeof kUsePromises): Promise; + function symlink(target: StringOrBuffer, path: StringOrBuffer, type: number): void; function unlink(path: string, req: FSReqCallback): void; function unlink(path: string): void; From 6eeaa02f5c641c22cc8ecb1f663d3fb1f195b50b Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:13:11 -0400 Subject: [PATCH 32/87] fs: improve error performance of `lchownSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- lib/fs.js | 8 +++++--- src/node_file.cc | 12 +++++------- typings/internalBinding/fs.d.ts | 1 + 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index f04f2d69330c2e..d4bf1d80fc5c7d 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2012,9 +2012,11 @@ function lchownSync(path, uid, gid) { path = getValidatedPath(path); validateInteger(uid, 'uid', -1, kMaxUserId); validateInteger(gid, 'gid', -1, kMaxUserId); - const ctx = { path }; - binding.lchown(pathModule.toNamespacedPath(path), uid, gid, undefined, ctx); - handleErrorFromBinding(ctx); + binding.lchown( + pathModule.toNamespacedPath(path), + uid, + gid, + ); } /** diff --git a/src/node_file.cc b/src/node_file.cc index 50a3fd29552138..e892b8a8659f9c 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2633,18 +2633,16 @@ static void LChown(const FunctionCallbackInfo& args) { CHECK(IsSafeJsInt(args[2])); const uv_gid_t gid = static_cast(args[2].As()->Value()); - FSReqBase* req_wrap_async = GetReqWrap(args, 3); - if (req_wrap_async != nullptr) { // lchown(path, uid, gid, req) + if (argc > 3) { // lchown(path, uid, gid, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 3); FS_ASYNC_TRACE_BEGIN1( UV_FS_LCHOWN, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, req_wrap_async, args, "lchown", UTF8, AfterNoArgs, uv_fs_lchown, *path, uid, gid); - } else { // lchown(path, uid, gid, undefined, ctx) - CHECK_EQ(argc, 5); - FSReqWrapSync req_wrap_sync; + } else { // lchown(path, uid, gid) + FSReqWrapSync req_wrap_sync("lchown", *path); FS_SYNC_TRACE_BEGIN(lchown); - SyncCall(env, args[4], &req_wrap_sync, "lchown", - uv_fs_lchown, *path, uid, gid); + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_lchown, *path, uid, gid); FS_SYNC_TRACE_END(lchown); } } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 8aab4a3ae8d242..f746ca582327d0 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -117,6 +117,7 @@ declare namespace InternalFSBinding { function lchown(path: string, uid: number, gid: number, req: FSReqCallback): void; function lchown(path: string, uid: number, gid: number, req: undefined, ctx: FSSyncContext): void; function lchown(path: string, uid: number, gid: number, usePromises: typeof kUsePromises): Promise; + function lchown(path: string, uid: number, gid: number): void; function link(existingPath: string, newPath: string, req: FSReqCallback): void; function link(existingPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void; From c5ff000cb1a2235d5ca64374023176c416c5800d Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sat, 30 Sep 2023 20:18:32 -0400 Subject: [PATCH 33/87] fs: improve error performance of `realpathSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- lib/fs.js | 8 ++++---- src/node_file.cc | 18 ++++++++---------- typings/internalBinding/fs.d.ts | 1 + 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index d4bf1d80fc5c7d..7324c2c2036a08 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2752,10 +2752,10 @@ function realpathSync(p, options) { realpathSync.native = (path, options) => { options = getOptions(options); path = getValidatedPath(path); - const ctx = { path }; - const result = binding.realpath(pathModule.toNamespacedPath(path), options.encoding, undefined, ctx); - handleErrorFromBinding(ctx); - return result; + return binding.realpath( + pathModule.toNamespacedPath(path), + options.encoding, + ); }; /** diff --git a/src/node_file.cc b/src/node_file.cc index e892b8a8659f9c..a9b9231009c144 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1828,28 +1828,27 @@ static void RealPath(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); const int argc = args.Length(); - CHECK_GE(argc, 3); + CHECK_GE(argc, 2); BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); - FSReqBase* req_wrap_async = GetReqWrap(args, 2); - if (req_wrap_async != nullptr) { // realpath(path, encoding, req) + if (argc > 2) { // realpath(path, encoding, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 2); FS_ASYNC_TRACE_BEGIN1( UV_FS_REALPATH, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, req_wrap_async, args, "realpath", encoding, AfterStringPtr, uv_fs_realpath, *path); } else { // realpath(path, encoding, undefined, ctx) - CHECK_EQ(argc, 4); - FSReqWrapSync req_wrap_sync; + FSReqWrapSync req_wrap_sync("realpath", *path); FS_SYNC_TRACE_BEGIN(realpath); - int err = SyncCall(env, args[3], &req_wrap_sync, "realpath", - uv_fs_realpath, *path); + int err = + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_realpath, *path); FS_SYNC_TRACE_END(realpath); if (err < 0) { - return; // syscall failed, no need to continue, error info is in ctx + return; } const char* link_path = static_cast(req_wrap_sync.req.ptr); @@ -1860,8 +1859,7 @@ static void RealPath(const FunctionCallbackInfo& args) { encoding, &error); if (rc.IsEmpty()) { - Local ctx = args[3].As(); - ctx->Set(env->context(), env->error_string(), error).Check(); + env->isolate()->ThrowException(error); return; } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index f746ca582327d0..6bed01519fa2b0 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -184,6 +184,7 @@ declare namespace InternalFSBinding { function realpath(path: StringOrBuffer, encoding: unknown, req: FSReqCallback): void; function realpath(path: StringOrBuffer, encoding: unknown, req: undefined, ctx: FSSyncContext): string | Buffer; function realpath(path: StringOrBuffer, encoding: unknown, usePromises: typeof kUsePromises): Promise; + function realpath(path: StringOrBuffer, encoding: unknown): StringOrBuffer; function rename(oldPath: string, newPath: string, req: FSReqCallback): void; function rename(oldPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void; From dd66fdfb7bed898b0103a916e2364c9cf244705d Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 18 Oct 2023 14:13:04 -0400 Subject: [PATCH 34/87] test: set parallel http server test as flaky MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ref: https://github.com/nodejs/node/issues/43465 PR-URL: https://github.com/nodejs/node/pull/50227 Refs: https://github.com/nodejs/node/issues/43465 Reviewed-By: Richard Lau Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Marco Ippolito Reviewed-By: Filip Skokan --- test/parallel/parallel.status | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index a0ff5d69361cb5..0489fba1e513fa 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -26,6 +26,8 @@ test-worker-nearheaplimit-deadlock: PASS, FLAKY test-domain-error-types: PASS,FLAKY # https://github.com/nodejs/node/issues/47420 test-file-write-stream4: PASS,FLAKY +# https://github.com/nodejs/node/issues/43465 +test-http-server-request-timeouts-mixed: PASS, FLAKY [$system==linux || $system==win32] # https://github.com/nodejs/node/issues/49605 From dacee4d9b54ce405d80945d9b8d6dc57f888a12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vinicius=20Louren=C3=A7o?= <12551007+H4ad@users.noreply.github.com> Date: Wed, 18 Oct 2023 16:28:08 -0300 Subject: [PATCH 35/87] doc: add ReflectConstruct to known perf issues PR-URL: https://github.com/nodejs/node/pull/50111 Reviewed-By: Yagiz Nizipli Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel Reviewed-By: Matteo Collina --- doc/contributing/primordials.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/contributing/primordials.md b/doc/contributing/primordials.md index 5570a17f6bc5bc..b763c88880f94c 100644 --- a/doc/contributing/primordials.md +++ b/doc/contributing/primordials.md @@ -122,6 +122,9 @@ performance of code in Node.js. * `SafePromiseAny` * `SafePromiseRace` * `SafePromisePrototypeFinally`: use `try {} finally {}` block instead. +* `ReflectConstruct`: Also affects `Reflect.construct`. + `ReflectConstruct` creates new types of classes inside functions. + Instead consider creating a shared class. See [nodejs/performance#109](https://github.com/nodejs/performance/issues/109). In general, when sending or reviewing a PR that makes changes in a hot code path, use extra caution and run extensive benchmarks. From 3e380017399072e226af071b821aca9840c1b0d7 Mon Sep 17 00:00:00 2001 From: Abdirahim Musse <33973272+abmusse@users.noreply.github.com> Date: Mon, 16 Oct 2023 20:13:44 -0500 Subject: [PATCH 36/87] test: skip test-benchmark-os.js on IBM i PR-URL: https://github.com/nodejs/node/pull/50208 Reviewed-By: Richard Lau Reviewed-By: Michael Dawson --- test/benchmark/benchmark.status | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/benchmark/benchmark.status b/test/benchmark/benchmark.status index 6a966743aab26b..a1d5fdba756546 100644 --- a/test/benchmark/benchmark.status +++ b/test/benchmark/benchmark.status @@ -19,3 +19,7 @@ prefix benchmark [$system==aix] [$arch==arm] + +[$system==ibmi] +test-benchmark-os.js: SKIP + From ce27ee701b51ff829788c03e31518bc9ca3f8db3 Mon Sep 17 00:00:00 2001 From: Tim Perry <1526883+pimterry@users.noreply.github.com> Date: Wed, 18 Oct 2023 20:44:40 +0100 Subject: [PATCH 37/87] tls: reduce TLS 'close' event listener warnings Without this, some heavy usage of TLS sockets can result in MaxListenersExceededWarning firing, from the 'this.on('close', ...)' line here. These appear to come from reinitializeHandle, which calls _wrapHandle repeatedly on the same socket instance. PR-URL: https://github.com/nodejs/node/pull/50136 Reviewed-By: Luigi Pinca Reviewed-By: Ben Noordhuis --- lib/_tls_wrap.js | 7 +++- .../test-tls-reinitialize-listeners.js | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-reinitialize-listeners.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 48108710933382..9cf3fc41485080 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -713,7 +713,12 @@ TLSSocket.prototype._wrapHandle = function(wrap, handle, wrapHasActiveWriteFromP this[kRes] = res; defineHandleReading(this, handle); - this.on('close', onSocketCloseDestroySSL); + // Guard against adding multiple listeners, as this method may be called + // repeatedly on the same socket by reinitializeHandle + if (this.listenerCount('close', onSocketCloseDestroySSL) === 0) { + this.on('close', onSocketCloseDestroySSL); + } + if (wrap) { wrap.on('close', () => this.destroy()); } diff --git a/test/parallel/test-tls-reinitialize-listeners.js b/test/parallel/test-tls-reinitialize-listeners.js new file mode 100644 index 00000000000000..ffd7c825b0f699 --- /dev/null +++ b/test/parallel/test-tls-reinitialize-listeners.js @@ -0,0 +1,42 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) { + common.skip('missing crypto'); +} + +const events = require('events'); +const fixtures = require('../common/fixtures'); +const tls = require('tls'); +const { kReinitializeHandle } = require('internal/net'); + +// Test that repeated calls to kReinitializeHandle() do not result in repeatedly +// adding new listeners on the socket (i.e. no MaxListenersExceededWarnings) + +process.on('warning', common.mustNotCall()); + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}); + +server.listen(0, common.mustCall(function() { + const socket = tls.connect({ + port: this.address().port, + rejectUnauthorized: false + }); + + socket.on('secureConnect', common.mustCall(function() { + for (let i = 0; i < events.defaultMaxListeners + 1; i++) { + socket[kReinitializeHandle](); + } + + socket.destroy(); + })); + + socket.on('close', function() { + server.close(); + }); +})); From c8744909b014c40cdb03fa1b914c48614c8592b5 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 18 Oct 2023 18:19:39 -0400 Subject: [PATCH 38/87] test: set inspector async hook test as flaky MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ref: https://github.com/nodejs/node/issues/50222 PR-URL: https://github.com/nodejs/node/pull/50252 Refs: https://github.com/nodejs/node/issues/50222 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Filip Skokan --- test/parallel/parallel.status | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 0489fba1e513fa..d21ddbda26e4b7 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -105,3 +105,7 @@ test-http-pipeline-flood: SKIP [$asan==on] # https://github.com/nodejs/node/issues/39655 test-cluster-primary-error: PASS, FLAKY + +[$arch==s390x] +# https://github.com/nodejs/node/issues/50222 +test-inspector-async-hook-setup-at-inspect-brk: PASS, FLAKY From 7c6e4d7ec3e496f5fef03f7b5aac9944f9234801 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 18 Oct 2023 20:15:28 -0400 Subject: [PATCH 39/87] test: set `test-esm-loader-resolve-type` as flaky Ref: https://github.com/nodejs/node/issues/50040 PR-URL: https://github.com/nodejs/node/pull/50226 Refs: https://github.com/nodejs/node/issues/50040 Reviewed-By: Filip Skokan Reviewed-By: Geoffrey Booth --- test/es-module/es-module.status | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/es-module/es-module.status b/test/es-module/es-module.status index 58a422dc152258..97cc23ab2d1cbd 100644 --- a/test/es-module/es-module.status +++ b/test/es-module/es-module.status @@ -5,6 +5,8 @@ prefix es-module # sample-test : PASS,FLAKY [true] # This section applies to all platforms +# https://github.com/nodejs/node/issues/50040 +test-esm-loader-resolve-type: PASS, FLAKY [$system==linux || $system==freebsd] # https://github.com/nodejs/node/issues/47836 From ce9d84eae3a5b4ff522ef313922c3232e280f649 Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Wed, 18 Oct 2023 20:00:05 -0500 Subject: [PATCH 40/87] doc: update api `stream.compose` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50206 Refs: https://github.com/nodejs/node/pull/50187/files#r1361215879 Reviewed-By: Moshe Atlow Reviewed-By: Vinícius Lourenço Claro Cardoso --- doc/api/stream.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/api/stream.md b/doc/api/stream.md index b9c4b4c2ac86f0..680bd4d260f504 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -2823,6 +2823,9 @@ const server = http.createServer((req, res) => { + +> Stability: 1.0 - Early development + +Node.js will inspect the source code of ambiguous input to determine whether it +contains ES module syntax; if such syntax is detected, the input will be treated +as an ES module. + +Ambiguous input is defined as: + +* Files with a `.js` extension or no extension; and either no controlling + `package.json` file or one that lacks a `type` field; and + `--experimental-default-type` is not specified. +* String input (`--eval` or STDIN) when neither `--input-type` nor + `--experimental-default-type` are specified. + +ES module syntax is defined as syntax that would throw when evaluated as +CommonJS. This includes `import` and `export` statements and `import.meta` +references. It does _not_ include `import()` expressions, which are valid in +CommonJS. + ### `--experimental-import-meta-resolve` @@ -1019,18 +1029,33 @@ _isImports_, _conditions_) > 1. Return _"commonjs"_. > 4. If _url_ ends in _".json"_, then > 1. Return _"json"_. -> 5. Let _packageURL_ be the result of **LOOKUP\_PACKAGE\_SCOPE**(_url_). -> 6. Let _pjson_ be the result of **READ\_PACKAGE\_JSON**(_packageURL_). -> 7. If _pjson?.type_ exists and is _"module"_, then -> 1. If _url_ ends in _".js"_ or has no file extension, then -> 1. If `--experimental-wasm-modules` is enabled and the file at _url_ -> contains the header for a WebAssembly module, then -> 1. Return _"wasm"_. -> 2. Otherwise, -> 1. Return _"module"_. -> 2. Return **undefined**. -> 8. Otherwise, -> 1. Return **undefined**. +> 5. If `--experimental-wasm-modules` is enabled and _url_ ends in +> _".wasm"_, then +> 1. Return _"wasm"_. +> 6. Let _packageURL_ be the result of **LOOKUP\_PACKAGE\_SCOPE**(_url_). +> 7. Let _pjson_ be the result of **READ\_PACKAGE\_JSON**(_packageURL_). +> 8. Let _packageType_ be **null**. +> 9. If _pjson?.type_ is _"module"_ or _"commonjs"_, then +> 1. Set _packageType_ to _pjson.type_. +> 10. If _url_ ends in _".js"_, then +> 1. If _packageType_ is not **null**, then +> 1. Return _packageType_. +> 2. If `--experimental-detect-module` is enabled and the source of +> module contains static import or export syntax, then +> 1. Return _"module"_. +> 3. Return _"commonjs"_. +> 11. If _url_ does not have any extension, then +> 1. If _packageType_ is _"module"_ and `--experimental-wasm-modules` is +> enabled and the file at _url_ contains the header for a WebAssembly +> module, then +> 1. Return _"wasm"_. +> 2. If _packageType_ is not **null**, then +> 1. Return _packageType_. +> 3. If `--experimental-detect-module` is enabled and the source of +> module contains static import or export syntax, then +> 1. Return _"module"_. +> 4. Return _"commonjs"_. +> 12. Return **undefined** (will throw during load phase). **LOOKUP\_PACKAGE\_SCOPE**(_url_) diff --git a/doc/api/modules.md b/doc/api/modules.md index 02fae47d88ea69..e38bca4f3cee54 100644 --- a/doc/api/modules.md +++ b/doc/api/modules.md @@ -80,8 +80,10 @@ By default, Node.js will treat the following as CommonJS modules: * Files with a `.js` extension when the nearest parent `package.json` file contains a top-level field [`"type"`][] with a value of `"commonjs"`. -* Files with a `.js` extension when the nearest parent `package.json` file - doesn't contain a top-level field [`"type"`][]. Package authors should include +* Files with a `.js` extension or without an extension, when the nearest parent + `package.json` file doesn't contain a top-level field [`"type"`][] or there is + no `package.json` in any parent folder; unless the file contains syntax that + errors unless it is evaluated as an ES module. Package authors should include the [`"type"`][] field, even in packages where all sources are CommonJS. Being explicit about the `type` of the package will make things easier for build tools and loaders to determine how the files in the package should be diff --git a/doc/api/packages.md b/doc/api/packages.md index 9f55cbbb15939f..9287ff71c404c9 100644 --- a/doc/api/packages.md +++ b/doc/api/packages.md @@ -69,6 +69,14 @@ expressions: * Strings passed in as an argument to `--eval`, or piped to `node` via `STDIN`, with the flag `--input-type=module`. +* Code that contains syntax that only parses successfully as [ES modules][], + such as `import` or `export` statements or `import.meta`, when the code has no + explicit marker of how it should be interpreted. Explicit markers are `.mjs` + or `.cjs` extensions, `package.json` `"type"` fields with either `"module"` or + `"commonjs"` values, or `--input-type` or `--experimental-default-type` flags. + Dynamic `import()` expressions are supported in either CommonJS or ES modules + and would not cause a file to be treated as an ES module. + Node.js will treat the following as [CommonJS][] when passed to `node` as the initial input, or when referenced by `import` statements or `import()` expressions: diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index 56d002ca0883ad..ee50bea472758d 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -18,9 +18,7 @@ const { const experimentalNetworkImports = getOptionValue('--experimental-network-imports'); -const defaultTypeFlag = getOptionValue('--experimental-default-type'); -// The next line is where we flip the default to ES modules someday. -const defaultType = defaultTypeFlag === 'module' ? 'module' : 'commonjs'; +const { containsModuleSyntax } = internalBinding('contextify'); const { getPackageType } = require('internal/modules/esm/resolve'); const { fileURLToPath } = require('internal/url'); const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; @@ -85,11 +83,12 @@ function underNodeModules(url) { /** * @param {URL} url - * @param {{parentURL: string}} context + * @param {{parentURL: string; source?: Buffer}} context * @param {boolean} ignoreErrors * @returns {string} */ -function getFileProtocolModuleFormat(url, context, ignoreErrors) { +function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreErrors) { + const { source } = context; const ext = extname(url); if (ext === '.js') { @@ -97,30 +96,53 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) { if (packageType !== 'none') { return packageType; } + // The controlling `package.json` file has no `type` field. - if (defaultType === 'module') { - // An exception to the type flag making ESM the default everywhere is that package scopes under `node_modules` - // should retain the assumption that a lack of a `type` field means CommonJS. - return underNodeModules(url) ? 'commonjs' : 'module'; + switch (getOptionValue('--experimental-default-type')) { + case 'module': { // The user explicitly passed `--experimental-default-type=module`. + // An exception to the type flag making ESM the default everywhere is that package scopes under `node_modules` + // should retain the assumption that a lack of a `type` field means CommonJS. + return underNodeModules(url) ? 'commonjs' : 'module'; + } + case 'commonjs': { // The user explicitly passed `--experimental-default-type=commonjs`. + return 'commonjs'; + } + default: { // The user did not pass `--experimental-default-type`. + // `source` is undefined when this is called from `defaultResolve`; + // but this gets called again from `defaultLoad`/`defaultLoadSync`. + if (source && getOptionValue('--experimental-detect-module')) { + return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs'; + } + return 'commonjs'; + } } - return 'commonjs'; } if (ext === '') { const packageType = getPackageType(url); - if (defaultType === 'commonjs') { // Legacy behavior - if (packageType === 'none' || packageType === 'commonjs') { - return 'commonjs'; - } // Else packageType === 'module' + if (packageType === 'module') { return getFormatOfExtensionlessFile(url); - } // Else defaultType === 'module' - if (underNodeModules(url)) { // Exception for package scopes under `node_modules` - return packageType === 'module' ? getFormatOfExtensionlessFile(url) : 'commonjs'; } - if (packageType === 'none' || packageType === 'module') { - return getFormatOfExtensionlessFile(url); - } // Else packageType === 'commonjs' - return 'commonjs'; + if (packageType !== 'none') { + return packageType; // 'commonjs' or future package types + } + + // The controlling `package.json` file has no `type` field. + switch (getOptionValue('--experimental-default-type')) { + case 'module': { // The user explicitly passed `--experimental-default-type=module`. + return underNodeModules(url) ? 'commonjs' : getFormatOfExtensionlessFile(url); + } + case 'commonjs': { // The user explicitly passed `--experimental-default-type=commonjs`. + return 'commonjs'; + } + default: { // The user did not pass `--experimental-default-type`. + if (source && getOptionValue('--experimental-detect-module') && + getFormatOfExtensionlessFile(url) === 'module') { + return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs'; + } + return 'commonjs'; + } + } } const format = extensionFormatMap[ext]; diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 1881745a6d3134..f706b5b808da27 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -33,7 +33,7 @@ const DATA_URL_PATTERN = /^[^/]+\/[^,;]+(?:[^,]*?)(;base64)?,([\s\S]*)$/; /** * @param {URL} url URL to the module * @param {ESModuleContext} context used to decorate error messages - * @returns {{ responseURL: string, source: string | BufferView }} + * @returns {Promise<{ responseURL: string, source: string | BufferView }>} */ async function getSource(url, context) { const { protocol, href } = url; @@ -105,7 +105,7 @@ function getSourceSync(url, context) { * @param {LoadContext} context * @returns {LoadReturn} */ -async function defaultLoad(url, context = kEmptyObject) { +async function defaultLoad(url, context = { __proto__: null }) { let responseURL = url; let { importAttributes, @@ -127,19 +127,24 @@ async function defaultLoad(url, context = kEmptyObject) { throwIfUnsupportedURLScheme(urlInstance, experimentalNetworkImports); - format ??= await defaultGetFormat(urlInstance, context); - - validateAttributes(url, format, importAttributes); - - if ( - format === 'builtin' || - format === 'commonjs' - ) { + if (urlInstance.protocol === 'node:') { source = null; } else if (source == null) { ({ responseURL, source } = await getSource(urlInstance, context)); + context.source = source; + } + + if (format == null || format === 'commonjs') { + // Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax. + format = await defaultGetFormat(urlInstance, context); + } + + if (format === 'commonjs') { + source = null; // Let the CommonJS loader handle it (for now) } + validateAttributes(url, format, importAttributes); + return { __proto__: null, format, @@ -178,16 +183,17 @@ function defaultLoadSync(url, context = kEmptyObject) { throwIfUnsupportedURLScheme(urlInstance, false); - format ??= defaultGetFormat(urlInstance, context); - - validateAttributes(url, format, importAttributes); - - if (format === 'builtin') { + if (urlInstance.protocol === 'node:') { source = null; } else if (source == null) { ({ responseURL, source } = getSourceSync(urlInstance, context)); + context.source = source; } + format ??= defaultGetFormat(urlInstance, context); + + validateAttributes(url, format, importAttributes); + return { __proto__: null, format, diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index a9828286a9c0e0..1f03c313121db0 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -4,6 +4,7 @@ const { StringPrototypeEndsWith, } = primordials; +const { containsModuleSyntax } = internalBinding('contextify'); const { getOptionValue } = require('internal/options'); const path = require('path'); @@ -70,7 +71,19 @@ function shouldUseESMLoader(mainPath) { const { readPackageScope } = require('internal/modules/package_json_reader'); const pkg = readPackageScope(mainPath); // No need to guard `pkg` as it can only be an object or `false`. - return pkg.data?.type === 'module' || getOptionValue('--experimental-default-type') === 'module'; + switch (pkg.data?.type) { + case 'module': + return true; + case 'commonjs': + return false; + default: { // No package.json or no `type` field. + if (getOptionValue('--experimental-detect-module')) { + // If the first argument of `containsModuleSyntax` is undefined, it will read `mainPath` from the file system. + return containsModuleSyntax(undefined, mainPath); + } + return false; + } + } } /** diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index b8c507c798182e..5de5edfb2d5524 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -26,6 +26,8 @@ const { emitAfter, popAsyncContext, } = require('internal/async_hooks'); +const { containsModuleSyntax } = internalBinding('contextify'); +const { getOptionValue } = require('internal/options'); const { makeContextifyScript, runScriptInThisContext, } = require('internal/vm'); @@ -70,6 +72,12 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { const baseUrl = pathToFileURL(module.filename).href; const { loadESM } = asyncESM; + if (getOptionValue('--experimental-detect-module') && + getOptionValue('--input-type') === '' && getOptionValue('--experimental-default-type') === '' && + containsModuleSyntax(body, name)) { + return evalModule(body, print); + } + const runScript = () => { // Create wrapper for cache entry const script = ` diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 35c628f4210d3a..a849efcc9c334d 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1384,20 +1384,43 @@ constexpr std::array esm_syntax_error_messages = { void ContextifyContext::ContainsModuleSyntax( const FunctionCallbackInfo& args) { - // Argument 1: source code - CHECK(args[0]->IsString()); - Local code = args[0].As(); + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + Local context = env->context(); - // Argument 2: filename - Local filename = String::Empty(args.GetIsolate()); + if (args.Length() == 0) { + return THROW_ERR_MISSING_ARGS( + env, "containsModuleSyntax needs at least 1 argument"); + } + + // Argument 2: filename; if undefined, use empty string + Local filename = String::Empty(isolate); if (!args[1]->IsUndefined()) { CHECK(args[1]->IsString()); filename = args[1].As(); } - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); - Local context = env->context(); + // Argument 1: source code; if undefined, read from filename in argument 2 + Local code; + if (args[0]->IsUndefined()) { + CHECK(!filename.IsEmpty()); + const char* filename_str = Utf8Value(isolate, filename).out(); + std::string contents; + int result = ReadFileSync(&contents, filename_str); + if (result != 0) { + isolate->ThrowException( + ERR_MODULE_NOT_FOUND(isolate, "Cannot read file %s", filename_str)); + return; + } + code = String::NewFromUtf8(isolate, + contents.c_str(), + v8::NewStringType::kNormal, + contents.length()) + .ToLocalChecked(); + } else { + CHECK(args[0]->IsString()); + code = args[0].As(); + } // TODO(geoffreybooth): Centralize this rather than matching the logic in // cjs/loader.js and translators.js diff --git a/src/node_options.cc b/src/node_options.cc index fc9940b64c5c33..29cb7fc6b29b89 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -347,6 +347,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::conditions, kAllowedInEnvvar); AddAlias("-C", "--conditions"); + AddOption("--experimental-detect-module", + "when ambiguous modules fail to evaluate because they contain " + "ES module syntax, try again to evaluate them as ES modules", + &EnvironmentOptions::detect_module, + kAllowedInEnvvar); AddOption("--diagnostic-dir", "set dir for all output files" " (default: current working directory)", diff --git a/src/node_options.h b/src/node_options.h index ba16f54e129aec..30955c779714ce 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -104,6 +104,7 @@ class EnvironmentOptions : public Options { public: bool abort_on_uncaught_exception = false; std::vector conditions; + bool detect_module = false; std::string dns_result_order; bool enable_source_maps = false; bool experimental_fetch = true; diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs new file mode 100644 index 00000000000000..61629965518a82 --- /dev/null +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -0,0 +1,214 @@ +import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { spawn } from 'node:child_process'; +import { describe, it } from 'node:test'; +import { strictEqual, match } from 'node:assert'; + +describe('--experimental-detect-module', { concurrency: true }, () => { + describe('string input', { concurrency: true }, () => { + it('permits ESM syntax in --eval input without requiring --input-type=module', async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--eval', + 'import { version } from "node:process"; console.log(version);', + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, `${process.version}\n`); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + // ESM is unsupported for --print via --input-type=module + + it('permits ESM syntax in STDIN input without requiring --input-type=module', async () => { + const child = spawn(process.execPath, [ + '--experimental-detect-module', + ]); + child.stdin.end('console.log(typeof import.meta.resolve)'); + + match((await child.stdout.toArray()).toString(), /^function\r?\n$/); + }); + + it('should be overridden by --input-type', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--input-type=commonjs', + '--eval', + 'import.meta.url', + ]); + + match(stderr, /SyntaxError: Cannot use 'import\.meta' outside a module/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + + it('should be overridden by --experimental-default-type', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--experimental-default-type=commonjs', + '--eval', + 'import.meta.url', + ]); + + match(stderr, /SyntaxError: Cannot use 'import\.meta' outside a module/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + + it('does not trigger detection via source code `eval()`', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--eval', + 'eval("import \'nonexistent\';");', + ]); + + match(stderr, /SyntaxError: Cannot use import statement outside a module/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + }); + + describe('.js file input in a typeless package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'permits CommonJS syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-without-type/commonjs.js'), + }, + { + testName: 'permits ESM syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-without-type/module.js'), + }, + { + testName: 'permits CommonJS syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-commonjs.cjs'), + }, + { + testName: 'permits ESM syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-esm.js'), + }, + { + testName: 'permits CommonJS syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-commonjs.mjs'), + }, + { + testName: 'permits ESM syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-esm.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'executed\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + } + }); + + describe('extensionless file input in a typeless package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'permits CommonJS syntax in an extensionless entry point', + entryPath: fixtures.path('es-modules/package-without-type/noext-cjs'), + }, + { + testName: 'permits ESM syntax in an extensionless entry point', + entryPath: fixtures.path('es-modules/package-without-type/noext-esm'), + }, + { + testName: 'permits CommonJS syntax in an extensionless file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-noext-cjs.js'), + }, + { + testName: 'permits ESM syntax in an extensionless file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-noext-esm.js'), + }, + { + testName: 'permits CommonJS syntax in an extensionless file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-noext-cjs.mjs'), + }, + { + testName: 'permits ESM syntax in an extensionless file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-noext-esm.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'executed\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + } + }); + + describe('file input in a "type": "commonjs" package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'disallows ESM syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-type-commonjs/module.js'), + }, + { + testName: 'disallows ESM syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-type-commonjs/imports-esm.js'), + }, + { + testName: 'disallows ESM syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-type-commonjs/imports-esm.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + match(stderr, /SyntaxError: Unexpected token 'export'/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + } + }); + + describe('file input in a "type": "module" package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'disallows CommonJS syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-type-module/cjs.js'), + }, + { + testName: 'disallows CommonJS syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-type-module/imports-commonjs.cjs'), + }, + { + testName: 'disallows CommonJS syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-type-module/imports-commonjs.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + match(stderr, /ReferenceError: module is not defined in ES module scope/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + } + }); +}); diff --git a/test/fixtures/es-modules/package-type-commonjs/imports-esm.js b/test/fixtures/es-modules/package-type-commonjs/imports-esm.js new file mode 100644 index 00000000000000..d2f5d5fee76ef7 --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/imports-esm.js @@ -0,0 +1 @@ +import('./module.js'); diff --git a/test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs b/test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs new file mode 100644 index 00000000000000..d3eb2fba6a8ee7 --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs @@ -0,0 +1 @@ +import './module.js'; diff --git a/test/fixtures/es-modules/package-type-commonjs/module.js b/test/fixtures/es-modules/package-type-commonjs/module.js new file mode 100644 index 00000000000000..251d6e538a1fcf --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/module.js @@ -0,0 +1,2 @@ +export default 'module'; +console.log('executed'); diff --git a/test/fixtures/es-modules/package-type-module/imports-commonjs.cjs b/test/fixtures/es-modules/package-type-module/imports-commonjs.cjs new file mode 100644 index 00000000000000..7dbbf0d97a46ba --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/imports-commonjs.cjs @@ -0,0 +1 @@ +import('./cjs.js'); diff --git a/test/fixtures/es-modules/package-type-module/imports-commonjs.mjs b/test/fixtures/es-modules/package-type-module/imports-commonjs.mjs new file mode 100644 index 00000000000000..df53dcd2bd4885 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/imports-commonjs.mjs @@ -0,0 +1 @@ +import './cjs.js'; diff --git a/test/fixtures/es-modules/package-without-type/commonjs.js b/test/fixtures/es-modules/package-without-type/commonjs.js new file mode 100644 index 00000000000000..9b4d39fa21ada2 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/commonjs.js @@ -0,0 +1,2 @@ +module.exports = 'cjs'; +console.log('executed'); diff --git a/test/fixtures/es-modules/package-without-type/imports-commonjs.cjs b/test/fixtures/es-modules/package-without-type/imports-commonjs.cjs new file mode 100644 index 00000000000000..b247f42a15ceb8 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-commonjs.cjs @@ -0,0 +1 @@ +import('./commonjs.js'); diff --git a/test/fixtures/es-modules/package-without-type/imports-commonjs.mjs b/test/fixtures/es-modules/package-without-type/imports-commonjs.mjs new file mode 100644 index 00000000000000..c2f8171fd1deda --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-commonjs.mjs @@ -0,0 +1 @@ +import './commonjs.js'; diff --git a/test/fixtures/es-modules/package-without-type/imports-esm.js b/test/fixtures/es-modules/package-without-type/imports-esm.js new file mode 100644 index 00000000000000..d2f5d5fee76ef7 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-esm.js @@ -0,0 +1 @@ +import('./module.js'); diff --git a/test/fixtures/es-modules/package-without-type/imports-esm.mjs b/test/fixtures/es-modules/package-without-type/imports-esm.mjs new file mode 100644 index 00000000000000..d3eb2fba6a8ee7 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-esm.mjs @@ -0,0 +1 @@ +import './module.js'; diff --git a/test/fixtures/es-modules/package-without-type/imports-noext-cjs.js b/test/fixtures/es-modules/package-without-type/imports-noext-cjs.js new file mode 100644 index 00000000000000..9f78ce4dd0ed7e --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-noext-cjs.js @@ -0,0 +1 @@ +import('./noext-cjs'); diff --git a/test/fixtures/es-modules/package-without-type/imports-noext-cjs.mjs b/test/fixtures/es-modules/package-without-type/imports-noext-cjs.mjs new file mode 100644 index 00000000000000..2419cba28f9318 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-noext-cjs.mjs @@ -0,0 +1 @@ +import './noext-cjs'; diff --git a/test/fixtures/es-modules/package-without-type/imports-noext-esm.js b/test/fixtures/es-modules/package-without-type/imports-noext-esm.js new file mode 100644 index 00000000000000..96eca54521b9d3 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-noext-esm.js @@ -0,0 +1 @@ +import './noext-esm'; diff --git a/test/fixtures/es-modules/package-without-type/imports-noext-esm.mjs b/test/fixtures/es-modules/package-without-type/imports-noext-esm.mjs new file mode 100644 index 00000000000000..96eca54521b9d3 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-noext-esm.mjs @@ -0,0 +1 @@ +import './noext-esm'; diff --git a/test/fixtures/es-modules/package-without-type/module.js b/test/fixtures/es-modules/package-without-type/module.js index 69147a3b8ca027..251d6e538a1fcf 100644 --- a/test/fixtures/es-modules/package-without-type/module.js +++ b/test/fixtures/es-modules/package-without-type/module.js @@ -1,3 +1,2 @@ -// This file can be run or imported only if `--experimental-default-type=module` is set. export default 'module'; console.log('executed'); diff --git a/test/fixtures/es-modules/package-without-type/noext-cjs b/test/fixtures/es-modules/package-without-type/noext-cjs new file mode 100644 index 00000000000000..37f7b87ad10561 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/noext-cjs @@ -0,0 +1,2 @@ +module.exports = 'commonjs'; +console.log('executed'); diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs index dbe486f5bb2991..c6dcb0fcbffb1b 100644 --- a/test/sequential/test-watch-mode.mjs +++ b/test/sequential/test-watch-mode.mjs @@ -277,11 +277,11 @@ console.log(values.random); it('should not load --import modules in main process', async () => { const file = createTmpFile(); - const imported = pathToFileURL(createTmpFile('setImmediate(() => process.exit(0));')); + const imported = pathToFileURL(createTmpFile('process._rawDebug("imported");')); const args = ['--import', imported, file]; const { stderr, stdout } = await runWriteSucceed({ file, watchedFile: file, args }); - assert.strictEqual(stderr, ''); + assert.strictEqual(stderr, 'imported\nimported\n'); assert.deepStrictEqual(stdout, [ 'running', `Completed running ${inspect(file)}`, From a362c276eced0b56b6710b266bdc65740255b3c9 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 20 Oct 2023 18:25:42 +0200 Subject: [PATCH 55/87] crypto: ensure valid point on elliptic curve in SubtleCrypto.importKey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50234 Reviewed-By: Tobias Nießen Reviewed-By: Antoine du Hamel --- lib/internal/crypto/ec.js | 4 + src/crypto/crypto_keys.cc | 31 +++++++ src/crypto/crypto_keys.h | 3 + .../test-webcrypto-export-import-ec.js | 86 ++++++++++++++++--- 4 files changed, 114 insertions(+), 10 deletions(-) diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index 710917af2e2783..9f30f6e1a0ba09 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -264,6 +264,10 @@ async function ecImportKey( break; } + if (!keyObject[kHandle].checkEcKeyData()) { + throw lazyDOMException('Invalid keyData', 'DataError'); + } + const { namedCurve: checkNamedCurve, } = keyObject[kHandle].keyDetail({}); diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 8d2774ff61a64c..c5dd2fb8fce40f 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -907,6 +907,8 @@ v8::Local KeyObjectHandle::Initialize(Environment* env) { isolate, templ, "getSymmetricKeySize", GetSymmetricKeySize); SetProtoMethodNoSideEffect( isolate, templ, "getAsymmetricKeyType", GetAsymmetricKeyType); + SetProtoMethodNoSideEffect( + isolate, templ, "checkEcKeyData", CheckEcKeyData); SetProtoMethod(isolate, templ, "export", Export); SetProtoMethod(isolate, templ, "exportJwk", ExportJWK); SetProtoMethod(isolate, templ, "initECRaw", InitECRaw); @@ -926,6 +928,7 @@ void KeyObjectHandle::RegisterExternalReferences( registry->Register(Init); registry->Register(GetSymmetricKeySize); registry->Register(GetAsymmetricKeyType); + registry->Register(CheckEcKeyData); registry->Register(Export); registry->Register(ExportJWK); registry->Register(InitECRaw); @@ -1237,6 +1240,34 @@ void KeyObjectHandle::GetAsymmetricKeyType( args.GetReturnValue().Set(key->GetAsymmetricKeyType()); } +bool KeyObjectHandle::CheckEcKeyData() const { + MarkPopErrorOnReturn mark_pop_error_on_return; + + const ManagedEVPPKey& key = data_->GetAsymmetricKey(); + KeyType type = data_->GetKeyType(); + CHECK_NE(type, kKeyTypeSecret); + EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(key.get(), nullptr)); + CHECK(ctx); + CHECK_EQ(EVP_PKEY_id(key.get()), EVP_PKEY_EC); + + if (type == kKeyTypePrivate) { + return EVP_PKEY_check(ctx.get()) == 1; + } + +#if OPENSSL_VERSION_MAJOR >= 3 + return EVP_PKEY_public_check_quick(ctx.get()) == 1; +#else + return EVP_PKEY_public_check(ctx.get()) == 1; +#endif +} + +void KeyObjectHandle::CheckEcKeyData(const FunctionCallbackInfo& args) { + KeyObjectHandle* key; + ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); + + args.GetReturnValue().Set(key->CheckEcKeyData()); +} + void KeyObjectHandle::GetSymmetricKeySize( const FunctionCallbackInfo& args) { KeyObjectHandle* key; diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h index eb4f5222670e89..820d26cc153177 100644 --- a/src/crypto/crypto_keys.h +++ b/src/crypto/crypto_keys.h @@ -193,6 +193,9 @@ class KeyObjectHandle : public BaseObject { const v8::FunctionCallbackInfo& args); v8::Local GetAsymmetricKeyType() const; + static void CheckEcKeyData(const v8::FunctionCallbackInfo& args); + bool CheckEcKeyData() const; + static void GetSymmetricKeySize( const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-webcrypto-export-import-ec.js b/test/parallel/test-webcrypto-export-import-ec.js index e17ff0c2d4075c..2bb9173ec857ff 100644 --- a/test/parallel/test-webcrypto-export-import-ec.js +++ b/test/parallel/test-webcrypto-export-import-ec.js @@ -400,15 +400,81 @@ async function testImportRaw({ name, publicUsages }, namedCurve) { ['ECDSA', ['verify'], ['sign']], ['ECDH', [], ['deriveBits', 'deriveBits']], ]) { - assert.rejects(subtle.importKey( - 'spki', - rsaPublic.export({ format: 'der', type: 'spki' }), - { name, hash: 'SHA-256', namedCurve: 'P-256' }, - true, publicUsages), { message: /Invalid key type/ }); - assert.rejects(subtle.importKey( - 'pkcs8', - rsaPrivate.export({ format: 'der', type: 'pkcs8' }), - { name, hash: 'SHA-256', namedCurve: 'P-256' }, - true, privateUsages), { message: /Invalid key type/ }); + assert.rejects( + subtle.importKey( + 'spki', + rsaPublic.export({ format: 'der', type: 'spki' }), + { name, hash: 'SHA-256', namedCurve: 'P-256' }, + true, publicUsages), { message: /Invalid key type/ }, + ).then(common.mustCall()); + assert.rejects( + subtle.importKey( + 'pkcs8', + rsaPrivate.export({ format: 'der', type: 'pkcs8' }), + { name, hash: 'SHA-256', namedCurve: 'P-256' }, + true, privateUsages), { message: /Invalid key type/ }, + ).then(common.mustCall()); + } +} + +// Bad private keys +{ + for (const { namedCurve, key: pkcs8 } of [ + // The private key is exactly equal to the order, and the public key is + // private key * order. + { + namedCurve: 'P-256', + key: Buffer.from( + '3066020100301306072a8648ce3d020106082a8648ce3d030107044c304a0201' + + '010420ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc' + + '632551a12303210000ffffff00000000ffffffffffffffffbce6faada7179e84' + + 'f3b9cac2fc632551', 'hex'), + }, + // The private key is exactly equal to the order, and the public key is + // omitted. + { + namedCurve: 'P-256', + key: Buffer.from( + '3041020100301306072a8648ce3d020106082a8648ce3d030107042730250201' + + '010420ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc' + + '632551', 'hex'), + }, + // The private key is exactly equal to the order + 11, and the public key is + // private key * order. + { + namedCurve: 'P-521', + key: Buffer.from( + '3081ee020100301006072a8648ce3d020106052b810400230481d63081d30201' + + '01044201ffffffffffffffffffffffffffffffffffffffffffffffffffffffff' + + 'fffffffffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb7' + + '1e91386414a181890381860004008a75841259fdedff546f1a39573b4315cfed' + + '5dc7ed7c17849543ef2c54f2991652f3dbc5332663da1bd19b1aebe319108501' + + '5c024fa4c9a902ecc0e02dda0cdb9a0096fb303fcbba2129849d0ca877054fb2' + + '293add566210bd0493ed2e95d4e0b9b82b1bc8a90e8b42a4ab3892331914a953' + + '36dcac80e3f4819b5d58874f92ce48c808', 'hex'), + }, + // The private key is exactly equal to the order + 11, and the public key is + // omitted. + { + namedCurve: 'P-521', + key: Buffer.from( + '3060020100301006072a8648ce3d020106052b81040023044930470201010442' + + '01ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' + + 'fffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb71e9138' + + '6414', 'hex'), + }, + ]) { + for (const [name, privateUsages] of [ + ['ECDSA', ['sign']], + ['ECDH', ['deriveBits', 'deriveBits']], + ]) { + assert.rejects( + subtle.importKey( + 'pkcs8', + pkcs8, + { name, hash: 'SHA-256', namedCurve }, + true, privateUsages), { name: 'DataError', message: /Invalid keyData/ }, + ).then(common.mustCall()); + } } } From 5a52c518ef6429d9162d395369acf916f3195715 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 20 Oct 2023 16:22:48 -0400 Subject: [PATCH 56/87] lib: add `navigator.userAgent` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50200 Reviewed-By: Geoffrey Booth Reviewed-By: Matthew Aitken Reviewed-By: Matteo Collina Reviewed-By: Vinícius Lourenço Claro Cardoso --- doc/api/globals.md | 15 +++++++++++++++ lib/internal/navigator.js | 10 ++++++++++ test/parallel/test-navigator.js | 2 ++ 3 files changed, 27 insertions(+) diff --git a/doc/api/globals.md b/doc/api/globals.md index 0ca27111db49e1..233f5281e0a199 100644 --- a/doc/api/globals.md +++ b/doc/api/globals.md @@ -629,6 +629,21 @@ logical processors available to the current Node.js instance. console.log(`This process is running on ${navigator.hardwareConcurrency}`); ``` +### `navigator.userAgent` + + + +* {string} + +The `navigator.userAgent` read-only property returns user agent +consisting of the runtime name and major version number. + +```js +console.log(`The user-agent is ${navigator.userAgent}`); // Prints "Node.js/21" +``` + ## `PerformanceEntry` -> Stability: 1 - Experimental +> Stability: 1.1 - Active development A partial implementation of the [Navigator API][]. @@ -610,10 +610,18 @@ A partial implementation of the [Navigator API][]. added: v21.0.0 --> -> Stability: 1 - Experimental +> Stability: 1.1 - Active development A partial implementation of [`window.navigator`][]. +If your app or a dependency uses a check for `navigator` to determine whether it +is running in a browser, the following can be used to delete the `navigator` +global before app code runs: + +```bash +node --import 'data:text/javascript,delete globalThis.navigator' app.js +``` + ### `navigator.hardwareConcurrency` + * `linker` {Function} * `specifier` {string} The specifier of the requested module: ```mjs @@ -627,15 +635,14 @@ The identifier of the current module, as set in the constructor. * `referencingModule` {vm.Module} The `Module` object `link()` is called on. * `extra` {Object} - * `assert` {Object} The data from the assertion: - - ```js - import foo from 'foo' assert { name: 'value' }; - // ^^^^^^^^^^^^^^^^^ the assertion + * `attributes` {Object} The data from the attribute: + ```mjs + import foo from 'foo' with { name: 'value' }; + // ^^^^^^^^^^^^^^^^^ the attribute ``` - Per ECMA-262, hosts are expected to ignore assertions that they do not - support, as opposed to, for example, triggering an error if an - unsupported assertion is present. + Per ECMA-262, hosts are expected to trigger an error if an + unsupported attribute is present. + * `assert` {Object} Alias for `extra.attributes`. * Returns: {vm.Module|Promise} * Returns: {Promise} @@ -734,7 +741,7 @@ changes: - v17.0.0 - v16.12.0 pr-url: https://github.com/nodejs/node/pull/40249 - description: Added support for import assertions to the + description: Added support for import attributes to the `importModuleDynamically` parameter. --> @@ -767,7 +774,7 @@ changes: [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `module` {vm.Module} - * `importAssertions` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -982,7 +989,7 @@ changes: - v17.0.0 - v16.12.0 pr-url: https://github.com/nodejs/node/pull/40249 - description: Added support for import assertions to the + description: Added support for import attributes to the `importModuleDynamically` parameter. - version: v15.9.0 pr-url: https://github.com/nodejs/node/pull/35431 @@ -1028,7 +1035,7 @@ changes: [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `function` {Function} - * `importAssertions` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -1214,7 +1221,7 @@ changes: - v17.0.0 - v16.12.0 pr-url: https://github.com/nodejs/node/pull/40249 - description: Added support for import assertions to the + description: Added support for import attributes to the `importModuleDynamically` parameter. - version: v6.3.0 pr-url: https://github.com/nodejs/node/pull/6635 @@ -1254,7 +1261,7 @@ changes: [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAssertions` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -1294,7 +1301,7 @@ changes: - v17.0.0 - v16.12.0 pr-url: https://github.com/nodejs/node/pull/40249 - description: Added support for import assertions to the + description: Added support for import attributes to the `importModuleDynamically` parameter. - version: v14.6.0 pr-url: https://github.com/nodejs/node/pull/34023 @@ -1355,7 +1362,7 @@ changes: [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAssertions` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -1399,7 +1406,7 @@ changes: - v17.0.0 - v16.12.0 pr-url: https://github.com/nodejs/node/pull/40249 - description: Added support for import assertions to the + description: Added support for import attributes to the `importModuleDynamically` parameter. - version: v6.3.0 pr-url: https://github.com/nodejs/node/pull/6635 @@ -1437,7 +1444,7 @@ changes: [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAssertions` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index d9a073fd4d7906..a100c8beefc80a 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -305,8 +305,8 @@ class SourceTextModule extends Module { this[kLink] = async (linker) => { this.#statusOverride = 'linking'; - const promises = this[kWrap].link(async (identifier, assert) => { - const module = await linker(identifier, this, { assert }); + const promises = this[kWrap].link(async (identifier, attributes) => { + const module = await linker(identifier, this, { attributes, assert: attributes }); if (module[kWrap] === undefined) { throw new ERR_VM_MODULE_NOT_MODULE(); } diff --git a/test/parallel/test-vm-module-link.js b/test/parallel/test-vm-module-link.js index 16694d5d846075..6b19a4d4916868 100644 --- a/test/parallel/test-vm-module-link.js +++ b/test/parallel/test-vm-module-link.js @@ -1,6 +1,6 @@ 'use strict'; -// Flags: --experimental-vm-modules +// Flags: --experimental-vm-modules --harmony-import-attributes const common = require('../common'); @@ -126,12 +126,14 @@ async function circular2() { async function asserts() { const m = new SourceTextModule(` - import "foo" assert { n1: 'v1', n2: 'v2' }; + import "foo" with { n1: 'v1', n2: 'v2' }; `, { identifier: 'm' }); await m.link((s, r, p) => { assert.strictEqual(s, 'foo'); assert.strictEqual(r.identifier, 'm'); + assert.strictEqual(p.attributes.n1, 'v1'); assert.strictEqual(p.assert.n1, 'v1'); + assert.strictEqual(p.attributes.n2, 'v2'); assert.strictEqual(p.assert.n2, 'v2'); return new SourceTextModule(''); }); From db2a1cf1cb5f4d0a3a1aee175c5fed8f07f3a6be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 23 Oct 2023 18:52:41 +0200 Subject: [PATCH 83/87] doc: fix `navigator.hardwareConcurrency` example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50278 Refs: https://github.com/nodejs/node/pull/47769 Reviewed-By: Michaël Zasso Reviewed-By: Richard Lau Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Yagiz Nizipli Reviewed-By: Luigi Pinca --- doc/api/globals.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/globals.md b/doc/api/globals.md index 2a598baa81ffa5..91eb267945be11 100644 --- a/doc/api/globals.md +++ b/doc/api/globals.md @@ -634,7 +634,7 @@ The `navigator.hardwareConcurrency` read-only property returns the number of logical processors available to the current Node.js instance. ```js -console.log(`This process is running on ${navigator.hardwareConcurrency}`); +console.log(`This process is running on ${navigator.hardwareConcurrency} logical processors`); ``` ### `navigator.userAgent` From 269e268c38cd67def3b5260d9376ad30cbd2f062 Mon Sep 17 00:00:00 2001 From: "Node.js GitHub Bot" Date: Mon, 23 Oct 2023 17:44:37 +0100 Subject: [PATCH 84/87] deps: update ada to 2.7.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50338 Reviewed-By: Yagiz Nizipli Reviewed-By: Matteo Collina Reviewed-By: Vinícius Lourenço Claro Cardoso --- deps/ada/ada.cpp | 277 +++++++++++++- deps/ada/ada.h | 362 +++++++++++++----- deps/ada/ada_c.h | 73 ++++ .../maintaining/maintaining-dependencies.md | 6 +- 4 files changed, 605 insertions(+), 113 deletions(-) diff --git a/deps/ada/ada.cpp b/deps/ada/ada.cpp index 1175b114d8addd..909fd50034d4fe 100644 --- a/deps/ada/ada.cpp +++ b/deps/ada/ada.cpp @@ -1,4 +1,4 @@ -/* auto-generated on 2023-09-30 20:34:30 -0400. Do not edit! */ +/* auto-generated on 2023-10-22 19:50:50 -0400. Do not edit! */ /* begin file src/ada.cpp */ #include "ada.h" /* begin file src/checkers.cpp */ @@ -11674,10 +11674,9 @@ ada_really_inline void url::parse_path(std::string_view input) { path = "/"; } } - return; } -std::string url::to_string() const { +[[nodiscard]] std::string url::to_string() const { if (!is_valid) { return "null"; } @@ -11797,7 +11796,7 @@ namespace ada { return host.value_or(""); } -[[nodiscard]] const std::string_view url::get_pathname() const noexcept { +[[nodiscard]] std::string_view url::get_pathname() const noexcept { return path; } @@ -12983,7 +12982,7 @@ template url_aggregator parse_url( namespace ada { -bool url_components::check_offset_consistency() const noexcept { +[[nodiscard]] bool url_components::check_offset_consistency() const noexcept { /** * https://user:pass@example.com:1234/foo/bar?baz#quux * | | | | ^^^^| | | @@ -13059,7 +13058,7 @@ bool url_components::check_offset_consistency() const noexcept { return true; } -std::string url_components::to_string() const { +[[nodiscard]] std::string url_components::to_string() const { std::string answer; auto back = std::back_insert_iterator(answer); answer.append("{\n"); @@ -13847,7 +13846,7 @@ bool url_aggregator::set_hostname(const std::string_view input) { return helpers::substring(buffer, 0, components.protocol_end); } -std::string ada::url_aggregator::to_string() const { +[[nodiscard]] std::string ada::url_aggregator::to_string() const { ada_log("url_aggregator::to_string buffer:", buffer, "[", buffer.size(), " bytes]"); if (!is_valid) { @@ -14292,7 +14291,7 @@ bool url_aggregator::parse_opaque_host(std::string_view input) { return true; } -std::string url_aggregator::to_diagram() const { +[[nodiscard]] std::string url_aggregator::to_diagram() const { if (!is_valid) { return "invalid"; } @@ -14449,7 +14448,7 @@ std::string url_aggregator::to_diagram() const { return answer; } -bool url_aggregator::validate() const noexcept { +[[nodiscard]] bool url_aggregator::validate() const noexcept { if (!is_valid) { return true; } @@ -14817,6 +14816,11 @@ ada::result& get_instance(void* result) noexcept { extern "C" { typedef void* ada_url; +typedef void* ada_url_search_params; +typedef void* ada_strings; +typedef void* ada_url_search_params_keys_iter; +typedef void* ada_url_search_params_values_iter; +typedef void* ada_url_search_params_entries_iter; struct ada_string { const char* data; @@ -14828,6 +14832,11 @@ struct ada_owned_string { size_t length; }; +struct ada_string_pair { + ada_string key; + ada_string value; +}; + ada_string ada_string_create(const char* data, size_t length) { ada_string out{}; out.data = data; @@ -15267,6 +15276,256 @@ ada_owned_string ada_idna_to_ascii(const char* input, size_t length) { return owned; } +ada_url_search_params ada_parse_search_params(const char* input, + size_t length) { + return new ada::result( + ada::url_search_params(std::string_view(input, length))); +} + +void ada_free_search_params(ada_url_search_params result) { + ada::result* r = + (ada::result*)result; + delete r; +} + +ada_owned_string ada_search_params_to_string(ada_url_search_params result) { + ada::result& r = + *(ada::result*)result; + if (!r) return ada_owned_string{NULL, 0}; + std::string out = r->to_string(); + ada_owned_string owned{}; + owned.length = out.size(); + owned.data = new char[owned.length]; + memcpy((void*)owned.data, out.data(), owned.length); + return owned; +} + +size_t ada_search_params_size(ada_url_search_params result) { + ada::result& r = + *(ada::result*)result; + if (!r) return 0; + return r->size(); +} + +void ada_search_params_sort(ada_url_search_params result) { + ada::result& r = + *(ada::result*)result; + if (r) r->sort(); +} + +void ada_search_params_append(ada_url_search_params result, const char* key, + size_t key_length, const char* value, + size_t value_length) { + ada::result& r = + *(ada::result*)result; + if (r) { + r->append(std::string_view(key, key_length), + std::string_view(value, value_length)); + } +} + +void ada_search_params_set(ada_url_search_params result, const char* key, + size_t key_length, const char* value, + size_t value_length) { + ada::result& r = + *(ada::result*)result; + if (r) { + r->set(std::string_view(key, key_length), + std::string_view(value, value_length)); + } +} + +void ada_search_params_remove(ada_url_search_params result, const char* key, + size_t key_length) { + ada::result& r = + *(ada::result*)result; + if (r) { + r->remove(std::string_view(key, key_length)); + } +} + +void ada_search_params_remove_value(ada_url_search_params result, + const char* key, size_t key_length, + const char* value, size_t value_length) { + ada::result& r = + *(ada::result*)result; + if (r) { + r->remove(std::string_view(key, key_length), + std::string_view(value, value_length)); + } +} + +bool ada_search_params_has(ada_url_search_params result, const char* key, + size_t key_length) { + ada::result& r = + *(ada::result*)result; + if (!r) return false; + return r->has(std::string_view(key, key_length)); +} + +bool ada_search_params_has_value(ada_url_search_params result, const char* key, + size_t key_length, const char* value, + size_t value_length) { + ada::result& r = + *(ada::result*)result; + if (!r) return false; + return r->has(std::string_view(key, key_length), + std::string_view(value, value_length)); +} + +ada_string ada_search_params_get(ada_url_search_params result, const char* key, + size_t key_length) { + ada::result& r = + *(ada::result*)result; + if (!r) return ada_string_create(NULL, 0); + auto found = r->get(std::string_view(key, key_length)); + if (!found.has_value()) return ada_string_create(NULL, 0); + return ada_string_create(found->data(), found->length()); +} + +ada_strings ada_search_params_get_all(ada_url_search_params result, + const char* key, size_t key_length) { + ada::result& r = + *(ada::result*)result; + if (!r) { + return new ada::result>( + std::vector()); + } + return new ada::result>( + r->get_all(std::string_view(key, key_length))); +} + +ada_url_search_params_keys_iter ada_search_params_get_keys( + ada_url_search_params result) { + ada::result& r = + *(ada::result*)result; + if (!r) { + return new ada::result( + ada::url_search_params_keys_iter()); + } + return new ada::result(r->get_keys()); +} + +ada_url_search_params_values_iter ada_search_params_get_values( + ada_url_search_params result) { + ada::result& r = + *(ada::result*)result; + if (!r) { + return new ada::result( + ada::url_search_params_values_iter()); + } + return new ada::result(r->get_values()); +} + +ada_url_search_params_entries_iter ada_search_params_get_entries( + ada_url_search_params result) { + ada::result& r = + *(ada::result*)result; + if (!r) { + return new ada::result( + ada::url_search_params_entries_iter()); + } + return new ada::result(r->get_entries()); +} + +void ada_free_strings(ada_strings result) { + ada::result>* r = + (ada::result>*)result; + delete r; +} + +size_t ada_strings_size(ada_strings result) { + ada::result>* r = + (ada::result>*)result; + if (!r) return 0; + return (*r)->size(); +} + +ada_string ada_strings_get(ada_strings result, size_t index) { + ada::result>* r = + (ada::result>*)result; + if (!r) return ada_string_create(NULL, 0); + std::string_view view = (*r)->at(index); + return ada_string_create(view.data(), view.length()); +} + +void ada_free_search_params_keys_iter(ada_url_search_params_keys_iter result) { + ada::result* r = + (ada::result*)result; + delete r; +} + +ada_string ada_search_params_keys_iter_next( + ada_url_search_params_keys_iter result) { + ada::result* r = + (ada::result*)result; + if (!r) return ada_string_create(NULL, 0); + auto next = (*r)->next(); + if (!next.has_value()) return ada_string_create(NULL, 0); + return ada_string_create(next->data(), next->length()); +} + +bool ada_search_params_keys_iter_has_next( + ada_url_search_params_keys_iter result) { + ada::result* r = + (ada::result*)result; + if (!r) return false; + return (*r)->has_next(); +} + +void ada_free_search_params_values_iter( + ada_url_search_params_values_iter result) { + ada::result* r = + (ada::result*)result; + delete r; +} + +ada_string ada_search_params_values_iter_next( + ada_url_search_params_values_iter result) { + ada::result* r = + (ada::result*)result; + if (!r) return ada_string_create(NULL, 0); + auto next = (*r)->next(); + if (!next.has_value()) return ada_string_create(NULL, 0); + return ada_string_create(next->data(), next->length()); +} + +bool ada_search_params_values_iter_has_next( + ada_url_search_params_values_iter result) { + ada::result* r = + (ada::result*)result; + if (!r) return false; + return (*r)->has_next(); +} + +void ada_free_search_params_entries_iter( + ada_url_search_params_entries_iter result) { + ada::result* r = + (ada::result*)result; + delete r; +} + +ada_string_pair ada_search_params_entries_iter_next( + ada_url_search_params_entries_iter result) { + ada::result* r = + (ada::result*)result; + if (!r) return {ada_string_create(NULL, 0), ada_string_create(NULL, 0)}; + auto next = (*r)->next(); + if (!next.has_value()) + return {ada_string_create(NULL, 0), ada_string_create(NULL, 0)}; + return ada_string_pair{ + ada_string_create(next->first.data(), next->first.length()), + ada_string_create(next->second.data(), next->second.length())}; +} + +bool ada_search_params_entries_iter_has_next( + ada_url_search_params_entries_iter result) { + ada::result* r = + (ada::result*)result; + if (!r) return false; + return (*r)->has_next(); +} + } // extern "C" /* end file src/ada_c.cpp */ /* end file src/ada.cpp */ diff --git a/deps/ada/ada.h b/deps/ada/ada.h index d6f705a5d6db67..6d98b37075f892 100644 --- a/deps/ada/ada.h +++ b/deps/ada/ada.h @@ -1,4 +1,4 @@ -/* auto-generated on 2023-09-30 20:34:30 -0400. Do not edit! */ +/* auto-generated on 2023-10-22 19:50:50 -0400. Do not edit! */ /* begin file include/ada.h */ /** * @file ada.h @@ -98,7 +98,7 @@ namespace ada::idna { /** * @see https://www.unicode.org/reports/tr46/#Validity_Criteria */ -bool is_label_valid(const std::u32string_view label); +bool is_label_valid(std::u32string_view label); } // namespace ada::idna @@ -479,14 +479,14 @@ namespace ada { #endif // ADA_COMMON_DEFS_H /* end file include/ada/common_defs.h */ -#include +#include /** * @namespace ada::character_sets * @brief Includes the definitions for unicode character sets. */ namespace ada::character_sets { -ada_really_inline bool bit_at(const uint8_t a[], const uint8_t i); +ada_really_inline bool bit_at(const uint8_t a[], uint8_t i); } // namespace ada::character_sets #endif // ADA_CHARACTER_SETS_H @@ -996,7 +996,7 @@ ada_really_inline bool bit_at(const uint8_t a[], const uint8_t i) { } // namespace ada::character_sets -#endif // ADA_CHARACTER_SETS_H +#endif // ADA_CHARACTER_SETS_INL_H /* end file include/ada/character_sets-inl.h */ /* begin file include/ada/checkers-inl.h */ /** @@ -1312,12 +1312,12 @@ struct url_components { * @return true if the offset values are * consistent with a possible URL string */ - bool check_offset_consistency() const noexcept; + [[nodiscard]] bool check_offset_consistency() const noexcept; /** * Converts a url_components to JSON stringified version. */ - std::string to_string() const; + [[nodiscard]] std::string to_string() const; }; // struct url_components @@ -1505,13 +1505,17 @@ struct url_base { * @return On failure, it returns zero. * @see https://url.spec.whatwg.org/#host-parsing */ - virtual ada_really_inline size_t parse_port( - std::string_view view, bool check_trailing_content = false) noexcept = 0; + virtual size_t parse_port(std::string_view view, + bool check_trailing_content) noexcept = 0; + + virtual ada_really_inline size_t parse_port(std::string_view view) noexcept { + return this->parse_port(view, false); + } /** * Returns a JSON string representation of this URL. */ - virtual std::string to_string() const = 0; + [[nodiscard]] virtual std::string to_string() const = 0; /** @private */ virtual inline void clear_pathname() = 0; @@ -1520,10 +1524,10 @@ struct url_base { virtual inline void clear_search() = 0; /** @private */ - virtual inline bool has_hash() const noexcept = 0; + [[nodiscard]] virtual inline bool has_hash() const noexcept = 0; /** @private */ - virtual inline bool has_search() const noexcept = 0; + [[nodiscard]] virtual inline bool has_search() const noexcept = 0; }; // url_base @@ -1593,7 +1597,7 @@ ada_really_inline bool shorten_path(std::string_view& path, * * @see https://url.spec.whatwg.org/ */ -ada_really_inline void parse_prepared_path(const std::string_view input, +ada_really_inline void parse_prepared_path(std::string_view input, ada::scheme::type type, std::string& path); @@ -4381,7 +4385,7 @@ constexpr ada::scheme::type get_scheme_type(std::string_view scheme) noexcept { } // namespace ada::scheme -#endif // ADA_SCHEME_H +#endif // ADA_SCHEME_INL_H /* end file include/ada/scheme-inl.h */ /* begin file include/ada/serializers.h */ /** @@ -4423,7 +4427,7 @@ std::string ipv6(const std::array& address) noexcept; * network address. * @see https://url.spec.whatwg.org/#concept-ipv4-serializer */ -std::string ipv4(const uint64_t address) noexcept; +std::string ipv4(uint64_t address) noexcept; } // namespace ada::serializers @@ -4508,8 +4512,7 @@ ada_really_inline bool has_tabs_or_newline( * Checks if the input is a forbidden host code point. * @see https://url.spec.whatwg.org/#forbidden-host-code-point */ -ada_really_inline constexpr bool is_forbidden_host_code_point( - const char c) noexcept; +ada_really_inline constexpr bool is_forbidden_host_code_point(char c) noexcept; /** * Checks if the input contains a forbidden domain code point. @@ -4533,12 +4536,12 @@ contains_forbidden_domain_code_point_or_upper(const char* input, * @see https://url.spec.whatwg.org/#forbidden-domain-code-point */ ada_really_inline constexpr bool is_forbidden_domain_code_point( - const char c) noexcept; + char c) noexcept; /** * Checks if the input is alphanumeric, '+', '-' or '.' */ -ada_really_inline constexpr bool is_alnum_plus(const char c) noexcept; +ada_really_inline constexpr bool is_alnum_plus(char c) noexcept; /** * @details An ASCII hex digit is an ASCII upper hex digit or ASCII lower hex @@ -4546,7 +4549,7 @@ ada_really_inline constexpr bool is_alnum_plus(const char c) noexcept; * range U+0041 (A) to U+0046 (F), inclusive. An ASCII lower hex digit is an * ASCII digit or a code point in the range U+0061 (a) to U+0066 (f), inclusive. */ -ada_really_inline constexpr bool is_ascii_hex_digit(const char c) noexcept; +ada_really_inline constexpr bool is_ascii_hex_digit(char c) noexcept; /** * Checks if the input is a C0 control or space character. @@ -4555,33 +4558,33 @@ ada_really_inline constexpr bool is_ascii_hex_digit(const char c) noexcept; * A C0 control is a code point in the range U+0000 NULL to U+001F INFORMATION * SEPARATOR ONE, inclusive. */ -ada_really_inline constexpr bool is_c0_control_or_space(const char c) noexcept; +ada_really_inline constexpr bool is_c0_control_or_space(char c) noexcept; /** * Checks if the input is a ASCII tab or newline character. * * @details An ASCII tab or newline is U+0009 TAB, U+000A LF, or U+000D CR. */ -ada_really_inline constexpr bool is_ascii_tab_or_newline(const char c) noexcept; +ada_really_inline constexpr bool is_ascii_tab_or_newline(char c) noexcept; /** * @details A double-dot path segment must be ".." or an ASCII case-insensitive * match for ".%2e", "%2e.", or "%2e%2e". */ ada_really_inline ada_constexpr bool is_double_dot_path_segment( - const std::string_view input) noexcept; + std::string_view input) noexcept; /** * @details A single-dot path segment must be "." or an ASCII case-insensitive * match for "%2e". */ ada_really_inline constexpr bool is_single_dot_path_segment( - const std::string_view input) noexcept; + std::string_view input) noexcept; /** * @details ipv4 character might contain 0-9 or a-f character ranges. */ -ada_really_inline constexpr bool is_lowercase_hex(const char c) noexcept; +ada_really_inline constexpr bool is_lowercase_hex(char c) noexcept; /** * @details Convert hex to binary. Caller is responsible to ensure that @@ -4597,20 +4600,20 @@ ada_really_inline unsigned constexpr convert_hex_to_binary(char c) noexcept; * @see https://github.com/nodejs/node/blob/main/src/node_url.cc#L245 * @see https://encoding.spec.whatwg.org/#utf-8-decode-without-bom */ -std::string percent_decode(const std::string_view input, size_t first_percent); +std::string percent_decode(std::string_view input, size_t first_percent); /** * Returns a percent-encoding string whether percent encoding was needed or not. * @see https://github.com/nodejs/node/blob/main/src/node_url.cc#L226 */ -std::string percent_encode(const std::string_view input, +std::string percent_encode(std::string_view input, const uint8_t character_set[]); /** * Returns a percent-encoded string version of input, while starting the percent * encoding at the provided index. * @see https://github.com/nodejs/node/blob/main/src/node_url.cc#L226 */ -std::string percent_encode(const std::string_view input, +std::string percent_encode(std::string_view input, const uint8_t character_set[], size_t index); /** * Returns true if percent encoding was needed, in which case, we store @@ -4620,13 +4623,13 @@ std::string percent_encode(const std::string_view input, * @see https://github.com/nodejs/node/blob/main/src/node_url.cc#L226 */ template -bool percent_encode(const std::string_view input, const uint8_t character_set[], +bool percent_encode(std::string_view input, const uint8_t character_set[], std::string& out); /** * Returns the index at which percent encoding should start, or (equivalently), * the length of the prefix that does not require percent encoding. */ -ada_really_inline size_t percent_encode_index(const std::string_view input, +ada_really_inline size_t percent_encode_index(std::string_view input, const uint8_t character_set[]); /** * Lowers the string in-place, assuming that the content is ASCII. @@ -4673,18 +4676,18 @@ struct url_aggregator : url_base { url_aggregator(url_aggregator &&u) noexcept = default; url_aggregator &operator=(url_aggregator &&u) noexcept = default; url_aggregator &operator=(const url_aggregator &u) = default; - ~url_aggregator() = default; - - bool set_href(const std::string_view input); - bool set_host(const std::string_view input); - bool set_hostname(const std::string_view input); - bool set_protocol(const std::string_view input); - bool set_username(const std::string_view input); - bool set_password(const std::string_view input); - bool set_port(const std::string_view input); - bool set_pathname(const std::string_view input); - void set_search(const std::string_view input); - void set_hash(const std::string_view input); + ~url_aggregator() override = default; + + bool set_href(std::string_view input); + bool set_host(std::string_view input); + bool set_hostname(std::string_view input); + bool set_protocol(std::string_view input); + bool set_username(std::string_view input); + bool set_password(std::string_view input); + bool set_port(std::string_view input); + bool set_pathname(std::string_view input); + void set_search(std::string_view input); + void set_hash(std::string_view input); [[nodiscard]] bool has_valid_domain() const noexcept override; /** @@ -4702,7 +4705,7 @@ struct url_aggregator : url_base { * @see https://url.spec.whatwg.org/#dom-url-href * @see https://url.spec.whatwg.org/#concept-url-serializer */ - inline std::string_view get_href() const noexcept; + [[nodiscard]] inline std::string_view get_href() const noexcept; /** * The username getter steps are to return this's URL's username. * This function does not allocate memory. @@ -4762,7 +4765,7 @@ struct url_aggregator : url_base { * @return size of the pathname in bytes * @see https://url.spec.whatwg.org/#dom-url-pathname */ - ada_really_inline uint32_t get_pathname_length() const noexcept; + [[nodiscard]] ada_really_inline uint32_t get_pathname_length() const noexcept; /** * Return U+003F (?), followed by this's URL's query. * This function does not allocate memory. @@ -4811,18 +4814,18 @@ struct url_aggregator : url_base { /** * Returns a string representation of this URL. */ - std::string to_string() const override; + [[nodiscard]] std::string to_string() const override; /** * Returns a string diagram of this URL. */ - std::string to_diagram() const; + [[nodiscard]] std::string to_diagram() const; /** * Verifies that the parsed URL could be valid. Useful for debugging purposes. * @return true if the URL is valid, otherwise return true of the offsets are * possible. */ - bool validate() const noexcept; + [[nodiscard]] bool validate() const noexcept; /** @return true if it has an host but it is the empty string */ [[nodiscard]] inline bool has_empty_hostname() const noexcept; @@ -4869,9 +4872,12 @@ struct url_aggregator : url_base { */ inline void reserve(uint32_t capacity); - ada_really_inline size_t - parse_port(std::string_view view, - bool check_trailing_content = false) noexcept override; + ada_really_inline size_t parse_port( + std::string_view view, bool check_trailing_content) noexcept override; + + ada_really_inline size_t parse_port(std::string_view view) noexcept override { + return this->parse_port(view, false); + } /** * Return true on success. @@ -4900,7 +4906,7 @@ struct url_aggregator : url_base { [[nodiscard]] inline bool cannot_have_credentials_or_port() const; template - bool set_host_or_hostname(const std::string_view input); + bool set_host_or_hostname(std::string_view input); ada_really_inline bool parse_host(std::string_view input); @@ -4911,26 +4917,26 @@ struct url_aggregator : url_base { inline void update_base_search(std::string_view input); inline void update_base_search(std::string_view input, const uint8_t *query_percent_encode_set); - inline void update_base_pathname(const std::string_view input); - inline void update_base_username(const std::string_view input); - inline void append_base_username(const std::string_view input); - inline void update_base_password(const std::string_view input); - inline void append_base_password(const std::string_view input); + inline void update_base_pathname(std::string_view input); + inline void update_base_username(std::string_view input); + inline void append_base_username(std::string_view input); + inline void update_base_password(std::string_view input); + inline void append_base_password(std::string_view input); inline void update_base_port(uint32_t input); - inline void append_base_pathname(const std::string_view input); - inline uint32_t retrieve_base_port() const; + inline void append_base_pathname(std::string_view input); + [[nodiscard]] inline uint32_t retrieve_base_port() const; inline void clear_hostname(); inline void clear_password(); inline void clear_pathname() override; - inline bool has_dash_dot() const noexcept; + [[nodiscard]] inline bool has_dash_dot() const noexcept; void delete_dash_dot(); inline void consume_prepared_path(std::string_view input); template [[nodiscard]] ada_really_inline bool parse_scheme_with_colon( - const std::string_view input); + std::string_view input); ada_really_inline uint32_t replace_and_resize(uint32_t start, uint32_t end, std::string_view input); - inline bool has_authority() const noexcept; + [[nodiscard]] inline bool has_authority() const noexcept; inline void set_protocol_as_file(); inline void set_scheme(std::string_view new_scheme) noexcept; /** @@ -5092,7 +5098,7 @@ struct url : url_base { url(url &&u) noexcept = default; url &operator=(url &&u) noexcept = default; url &operator=(const url &u) = default; - ~url() = default; + ~url() override = default; /** * @private @@ -5153,7 +5159,7 @@ struct url : url_base { /** * Returns a JSON string representation of this URL. */ - std::string to_string() const override; + [[nodiscard]] std::string to_string() const override; /** * @see https://url.spec.whatwg.org/#dom-url-href @@ -5200,7 +5206,7 @@ struct url : url_base { * @return a newly allocated string. * @see https://url.spec.whatwg.org/#dom-url-pathname */ - [[nodiscard]] const std::string_view get_pathname() const noexcept; + [[nodiscard]] std::string_view get_pathname() const noexcept; /** * Compute the pathname length in bytes without instantiating a view or a @@ -5208,7 +5214,7 @@ struct url : url_base { * @return size of the pathname in bytes * @see https://url.spec.whatwg.org/#dom-url-pathname */ - ada_really_inline size_t get_pathname_length() const noexcept; + [[nodiscard]] ada_really_inline size_t get_pathname_length() const noexcept; /** * Return U+003F (?), followed by this's URL's query. @@ -5228,60 +5234,60 @@ struct url : url_base { * @return Returns true on successful operation. * @see https://url.spec.whatwg.org/#dom-url-username */ - bool set_username(const std::string_view input); + bool set_username(std::string_view input); /** * @return Returns true on success. * @see https://url.spec.whatwg.org/#dom-url-password */ - bool set_password(const std::string_view input); + bool set_password(std::string_view input); /** * @return Returns true on success. * @see https://url.spec.whatwg.org/#dom-url-port */ - bool set_port(const std::string_view input); + bool set_port(std::string_view input); /** * This function always succeeds. * @see https://url.spec.whatwg.org/#dom-url-hash */ - void set_hash(const std::string_view input); + void set_hash(std::string_view input); /** * This function always succeeds. * @see https://url.spec.whatwg.org/#dom-url-search */ - void set_search(const std::string_view input); + void set_search(std::string_view input); /** * @return Returns true on success. * @see https://url.spec.whatwg.org/#dom-url-search */ - bool set_pathname(const std::string_view input); + bool set_pathname(std::string_view input); /** * @return Returns true on success. * @see https://url.spec.whatwg.org/#dom-url-host */ - bool set_host(const std::string_view input); + bool set_host(std::string_view input); /** * @return Returns true on success. * @see https://url.spec.whatwg.org/#dom-url-hostname */ - bool set_hostname(const std::string_view input); + bool set_hostname(std::string_view input); /** * @return Returns true on success. * @see https://url.spec.whatwg.org/#dom-url-protocol */ - bool set_protocol(const std::string_view input); + bool set_protocol(std::string_view input); /** * @see https://url.spec.whatwg.org/#dom-url-href */ - bool set_href(const std::string_view input); + bool set_href(std::string_view input); /** * The password getter steps are to return this's URL's password. @@ -5352,9 +5358,9 @@ struct url : url_base { inline void update_base_search(std::string_view input, const uint8_t query_percent_encode_set[]); inline void update_base_search(std::optional input); - inline void update_base_pathname(const std::string_view input); - inline void update_base_username(const std::string_view input); - inline void update_base_password(const std::string_view input); + inline void update_base_pathname(std::string_view input); + inline void update_base_username(std::string_view input); + inline void update_base_password(std::string_view input); inline void update_base_port(std::optional input); /** @@ -5400,9 +5406,12 @@ struct url : url_base { */ [[nodiscard]] inline bool cannot_have_credentials_or_port() const; - ada_really_inline size_t - parse_port(std::string_view view, - bool check_trailing_content = false) noexcept override; + ada_really_inline size_t parse_port( + std::string_view view, bool check_trailing_content) noexcept override; + + ada_really_inline size_t parse_port(std::string_view view) noexcept override { + return this->parse_port(view, false); + } /** * Take the scheme from another URL. The scheme string is copied from the @@ -5421,8 +5430,7 @@ struct url : url_base { [[nodiscard]] ada_really_inline bool parse_host(std::string_view input); template - [[nodiscard]] ada_really_inline bool parse_scheme( - const std::string_view input); + [[nodiscard]] ada_really_inline bool parse_scheme(std::string_view input); inline void clear_pathname() override; inline void clear_search() override; @@ -5438,7 +5446,7 @@ struct url : url_base { * * @see https://url.spec.whatwg.org/ */ - ada_really_inline void parse_path(const std::string_view input); + ada_really_inline void parse_path(std::string_view input); /** * Set the scheme for this URL. The provided scheme should be a valid @@ -5525,7 +5533,9 @@ inline std::ostream &operator<<(std::ostream &out, const ada::url &u) { return out << u.to_string(); } -size_t url::get_pathname_length() const noexcept { return path.size(); } +[[nodiscard]] size_t url::get_pathname_length() const noexcept { + return path.size(); +} [[nodiscard]] ada_really_inline ada::url_components url::get_components() const noexcept { @@ -5902,7 +5912,7 @@ inline void url_aggregator::update_base_hostname(const std::string_view input) { ADA_ASSERT_TRUE(validate()); } -ada_really_inline uint32_t +[[nodiscard]] ada_really_inline uint32_t url_aggregator::get_pathname_length() const noexcept { ada_log("url_aggregator::get_pathname_length"); uint32_t ending_index = uint32_t(buffer.size()); @@ -6337,7 +6347,7 @@ inline void url_aggregator::clear_port() { ADA_ASSERT_TRUE(validate()); } -inline uint32_t url_aggregator::retrieve_base_port() const { +[[nodiscard]] inline uint32_t url_aggregator::retrieve_base_port() const { ada_log("url_aggregator::retrieve_base_port"); return components.port; } @@ -6562,28 +6572,40 @@ inline bool url_aggregator::has_port() const noexcept { return has_hostname() && components.pathname_start != components.host_end; } -inline bool url_aggregator::has_dash_dot() const noexcept { +[[nodiscard]] inline bool url_aggregator::has_dash_dot() const noexcept { // If url's host is null, url does not have an opaque path, url's path's size // is greater than 1, and url's path[0] is the empty string, then append // U+002F (/) followed by U+002E (.) to output. ada_log("url_aggregator::has_dash_dot"); - // Performance: instead of doing this potentially expensive check, we could - // just have a boolean value in the structure. #if ADA_DEVELOPMENT_CHECKS - if (components.pathname_start + 1 < buffer.size() && - components.pathname_start == components.host_end + 2) { - ADA_ASSERT_TRUE(buffer[components.host_end] == '/'); - ADA_ASSERT_TRUE(buffer[components.host_end + 1] == '.'); + // If pathname_start and host_end are exactly two characters apart, then we + // either have a one-digit port such as http://test.com:5?param=1 or else we + // have a /.: sequence such as "non-spec:/.//". We test that this is the case. + if (components.pathname_start == components.host_end + 2) { + ADA_ASSERT_TRUE((buffer[components.host_end] == '/' && + buffer[components.host_end + 1] == '.') || + (buffer[components.host_end] == ':' && + checkers::is_digit(buffer[components.host_end + 1]))); + } + if (components.pathname_start == components.host_end + 2 && + buffer[components.host_end] == '/' && + buffer[components.host_end + 1] == '.') { + ADA_ASSERT_TRUE(components.pathname_start + 1 < buffer.size()); ADA_ASSERT_TRUE(buffer[components.pathname_start] == '/'); ADA_ASSERT_TRUE(buffer[components.pathname_start + 1] == '/'); } #endif - return !has_opaque_path && - components.pathname_start == components.host_end + 2 && - components.pathname_start + 1 < buffer.size(); + // Performance: it should be uncommon for components.pathname_start == + // components.host_end + 2 to be true. So we put this check first in the + // sequence. Most times, we do not have an opaque path. Checking for '/.' is + // more expensive, but should be uncommon. + return components.pathname_start == components.host_end + 2 && + !has_opaque_path && buffer[components.host_end] == '/' && + buffer[components.host_end + 1] == '.'; } -inline std::string_view url_aggregator::get_href() const noexcept { +[[nodiscard]] inline std::string_view url_aggregator::get_href() + const noexcept { ada_log("url_aggregator::get_href"); return buffer; } @@ -6675,6 +6697,26 @@ inline std::ostream &operator<<(std::ostream &out, namespace ada { +enum class url_search_params_iter_type { + KEYS, + VALUES, + ENTRIES, +}; + +template +struct url_search_params_iter; + +typedef std::pair key_value_view_pair; + +using url_search_params_keys_iter = + url_search_params_iter; +using url_search_params_values_iter = + url_search_params_iter; +using url_search_params_entries_iter = + url_search_params_iter; + /** * @see https://url.spec.whatwg.org/#interface-urlsearchparams */ @@ -6737,6 +6779,42 @@ struct url_search_params { */ inline std::string to_string(); + /** + * Returns a simple JS-style iterator over all of the keys in this + * url_search_params. The keys in the iterator are not unique. The valid + * lifespan of the iterator is tied to the url_search_params. The iterator + * must be freed when you're done with it. + * @see https://url.spec.whatwg.org/#interface-urlsearchparams + */ + inline url_search_params_keys_iter get_keys(); + + /** + * Returns a simple JS-style iterator over all of the values in this + * url_search_params. The valid lifespan of the iterator is tied to the + * url_search_params. The iterator must be freed when you're done with it. + * @see https://url.spec.whatwg.org/#interface-urlsearchparams + */ + inline url_search_params_values_iter get_values(); + + /** + * Returns a simple JS-style iterator over all of the entries in this + * url_search_params. The entries are pairs of keys and corresponding values. + * The valid lifespan of the iterator is tied to the url_search_params. The + * iterator must be freed when you're done with it. + * @see https://url.spec.whatwg.org/#interface-urlsearchparams + */ + inline url_search_params_entries_iter get_entries(); + + /** + * C++ style conventional iterator support. const only because we + * do not really want the params to be modified via the iterator. + */ + inline auto begin() const { return params.begin(); } + inline auto end() const { return params.end(); } + inline auto front() const { return params.front(); } + inline auto back() const { return params.back(); } + inline auto operator[](size_t index) const { return params[index]; } + private: typedef std::pair key_value_pair; std::vector params{}; @@ -6745,8 +6823,44 @@ struct url_search_params { * @see https://url.spec.whatwg.org/#concept-urlencoded-parser */ void initialize(std::string_view init); + + template + friend struct url_search_params_iter; }; // url_search_params +/** + * Implements a non-conventional iterator pattern that is closer in style to + * JavaScript's definition of an iterator. + * + * @see https://webidl.spec.whatwg.org/#idl-iterable + */ +template +struct url_search_params_iter { + inline url_search_params_iter() : params(EMPTY) {} + url_search_params_iter(const url_search_params_iter &u) = default; + url_search_params_iter(url_search_params_iter &&u) noexcept = default; + url_search_params_iter &operator=(url_search_params_iter &&u) noexcept = + default; + url_search_params_iter &operator=(const url_search_params_iter &u) = default; + ~url_search_params_iter() = default; + + /** + * Return the next item in the iterator or std::nullopt if done. + */ + inline std::optional next(); + + inline bool has_next(); + + private: + static url_search_params EMPTY; + inline url_search_params_iter(url_search_params ¶ms_) : params(params_) {} + + url_search_params ¶ms; + size_t pos = 0; + + friend struct url_search_params; +}; + } // namespace ada #endif /* end file include/ada/url_search_params.h */ @@ -6767,6 +6881,10 @@ struct url_search_params { namespace ada { +// A default, empty url_search_params for use with empty iterators. +template +url_search_params url_search_params_iter::EMPTY; + inline void url_search_params::initialize(std::string_view input) { if (!input.empty() && input.front() == '?') { input.remove_prefix(1); @@ -6914,6 +7032,48 @@ inline void url_search_params::sort() { }); } +inline url_search_params_keys_iter url_search_params::get_keys() { + return url_search_params_keys_iter(*this); +} + +/** + * @see https://url.spec.whatwg.org/#interface-urlsearchparams + */ +inline url_search_params_values_iter url_search_params::get_values() { + return url_search_params_values_iter(*this); +} + +/** + * @see https://url.spec.whatwg.org/#interface-urlsearchparams + */ +inline url_search_params_entries_iter url_search_params::get_entries() { + return url_search_params_entries_iter(*this); +} + +template +inline bool url_search_params_iter::has_next() { + return pos < params.params.size(); +} + +template <> +inline std::optional url_search_params_keys_iter::next() { + if (!has_next()) return std::nullopt; + return params.params[pos++].first; +} + +template <> +inline std::optional url_search_params_values_iter::next() { + if (!has_next()) return std::nullopt; + return params.params[pos++].second; +} + +template <> +inline std::optional +url_search_params_entries_iter::next() { + if (!has_next()) return std::nullopt; + return params.params[pos++]; +} + } // namespace ada #endif // ADA_URL_SEARCH_PARAMS_INL_H @@ -6928,14 +7088,14 @@ inline void url_search_params::sort() { #ifndef ADA_ADA_VERSION_H #define ADA_ADA_VERSION_H -#define ADA_VERSION "2.6.10" +#define ADA_VERSION "2.7.2" namespace ada { enum { ADA_VERSION_MAJOR = 2, - ADA_VERSION_MINOR = 6, - ADA_VERSION_REVISION = 10, + ADA_VERSION_MINOR = 7, + ADA_VERSION_REVISION = 2, }; } // namespace ada diff --git a/deps/ada/ada_c.h b/deps/ada/ada_c.h index 040915518f32a0..173e27bb16af06 100644 --- a/deps/ada/ada_c.h +++ b/deps/ada/ada_c.h @@ -109,4 +109,77 @@ const ada_url_components* ada_get_components(ada_url result); ada_owned_string ada_idna_to_unicode(const char* input, size_t length); ada_owned_string ada_idna_to_ascii(const char* input, size_t length); +// url search params +typedef void* ada_url_search_params; + +// Represents an std::vector +typedef void* ada_strings; +typedef void* ada_url_search_params_keys_iter; +typedef void* ada_url_search_params_values_iter; + +typedef struct { + ada_string key; + ada_string value; +} ada_string_pair; + +typedef void* ada_url_search_params_entries_iter; + +ada_url_search_params ada_parse_search_params(const char* input, size_t length); +void ada_free_search_params(ada_url_search_params result); + +size_t ada_search_params_size(ada_url_search_params result); +void ada_search_params_sort(ada_url_search_params result); +ada_owned_string ada_search_params_to_string(ada_url_search_params result); + +void ada_search_params_append(ada_url_search_params result, const char* key, + size_t key_length, const char* value, + size_t value_length); +void ada_search_params_set(ada_url_search_params result, const char* key, + size_t key_length, const char* value, + size_t value_length); +void ada_search_params_remove(ada_url_search_params result, const char* key, + size_t key_length); +void ada_search_params_remove_value(ada_url_search_params result, + const char* key, size_t key_length, + const char* value, size_t value_length); +bool ada_search_params_has(ada_url_search_params result, const char* key, + size_t key_length); +bool ada_search_params_has_value(ada_url_search_params result, const char* key, + size_t key_length, const char* value, + size_t value_length); +ada_string ada_search_params_get(ada_url_search_params result, const char* key, + size_t key_length); +ada_strings ada_search_params_get_all(ada_url_search_params result, + const char* key, size_t key_length); +ada_url_search_params_keys_iter ada_search_params_get_keys( + ada_url_search_params result); +ada_url_search_params_values_iter ada_search_params_get_values( + ada_url_search_params result); +ada_url_search_params_entries_iter ada_search_params_get_entries( + ada_url_search_params result); + +void ada_free_strings(ada_strings result); +size_t ada_strings_size(ada_strings result); +ada_string ada_strings_get(ada_strings result, size_t index); + +void ada_free_search_params_keys_iter(ada_url_search_params_keys_iter result); +ada_string ada_search_params_keys_iter_next( + ada_url_search_params_keys_iter result); +bool ada_search_params_keys_iter_has_next( + ada_url_search_params_keys_iter result); + +void ada_free_search_params_values_iter( + ada_url_search_params_values_iter result); +ada_string ada_search_params_values_iter_next( + ada_url_search_params_values_iter result); +bool ada_search_params_values_iter_has_next( + ada_url_search_params_values_iter result); + +void ada_free_search_params_entries_iter( + ada_url_search_params_entries_iter result); +ada_string_pair ada_search_params_entries_iter_next( + ada_url_search_params_entries_iter result); +bool ada_search_params_entries_iter_has_next( + ada_url_search_params_entries_iter result); + #endif // ADA_C_H diff --git a/doc/contributing/maintaining/maintaining-dependencies.md b/doc/contributing/maintaining/maintaining-dependencies.md index 5b17f64adcaf32..c048c58c8ff1cd 100644 --- a/doc/contributing/maintaining/maintaining-dependencies.md +++ b/doc/contributing/maintaining/maintaining-dependencies.md @@ -9,7 +9,7 @@ All dependencies are located within the `deps` directory. This a list of all the dependencies: * [acorn 8.10.0][] -* [ada 2.6.10][] +* [ada 2.7.2][] * [base64 0.5.0][] * [brotli 1.0.9][] * [c-ares 1.20.1][] @@ -150,7 +150,7 @@ The [acorn](https://github.com/acornjs/acorn) dependency is a JavaScript parser. [acorn-walk](https://github.com/acornjs/acorn/tree/master/acorn-walk) is an abstract syntax tree walker for the ESTree format. -### ada 2.6.10 +### ada 2.7.2 The [ada](https://github.com/ada-url/ada) dependency is a fast and spec-compliant URL parser written in C++. @@ -319,7 +319,7 @@ it comes from the Chromium team's zlib fork which incorporated performance improvements not currently available in standard zlib. [acorn 8.10.0]: #acorn-8100 -[ada 2.6.10]: #ada-2610 +[ada 2.7.2]: #ada-272 [base64 0.5.0]: #base64-050 [brotli 1.0.9]: #brotli-109 [c-ares 1.20.1]: #c-ares-1201 From 998feda118121aae5d854095dacbc0eca2b8342b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 23 Oct 2023 18:01:59 +0200 Subject: [PATCH 85/87] esm: do not give wrong hints when detecting file format PR-URL: https://github.com/nodejs/node/pull/50314 Reviewed-By: Geoffrey Booth Reviewed-By: Moshe Atlow --- lib/internal/modules/esm/get_format.js | 16 +++++++++---- lib/internal/modules/esm/load.js | 24 ++++++++++++-------- test/es-module/test-esm-detect-ambiguous.mjs | 23 +++++++++++++++++++ 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index ee50bea472758d..1931688e85d05e 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -110,8 +110,10 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE default: { // The user did not pass `--experimental-default-type`. // `source` is undefined when this is called from `defaultResolve`; // but this gets called again from `defaultLoad`/`defaultLoadSync`. - if (source && getOptionValue('--experimental-detect-module')) { - return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs'; + if (getOptionValue('--experimental-detect-module')) { + return source ? + (containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs') : + null; } return 'commonjs'; } @@ -136,9 +138,13 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE return 'commonjs'; } default: { // The user did not pass `--experimental-default-type`. - if (source && getOptionValue('--experimental-detect-module') && - getFormatOfExtensionlessFile(url) === 'module') { - return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs'; + if (getOptionValue('--experimental-detect-module')) { + if (!source) { return null; } + const format = getFormatOfExtensionlessFile(url); + if (format === 'module') { + return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs'; + } + return format; } return 'commonjs'; } diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index f706b5b808da27..6f9b73abd8a761 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -105,7 +105,7 @@ function getSourceSync(url, context) { * @param {LoadContext} context * @returns {LoadReturn} */ -async function defaultLoad(url, context = { __proto__: null }) { +async function defaultLoad(url, context = kEmptyObject) { let responseURL = url; let { importAttributes, @@ -129,18 +129,22 @@ async function defaultLoad(url, context = { __proto__: null }) { if (urlInstance.protocol === 'node:') { source = null; - } else if (source == null) { - ({ responseURL, source } = await getSource(urlInstance, context)); - context.source = source; - } + format ??= 'builtin'; + } else { + let contextToPass = context; + if (source == null) { + ({ responseURL, source } = await getSource(urlInstance, context)); + contextToPass = { __proto__: context, source }; + } - if (format == null || format === 'commonjs') { // Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax. - format = await defaultGetFormat(urlInstance, context); - } + format ??= await defaultGetFormat(urlInstance, contextToPass); - if (format === 'commonjs') { - source = null; // Let the CommonJS loader handle it (for now) + if (format === 'commonjs' && contextToPass !== context) { + // For backward compatibility reasons, we need to discard the source in + // order for the CJS loader to re-fetch it. + source = null; + } } validateAttributes(url, format, importAttributes); diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index 61629965518a82..34c5f17f007c8f 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -152,6 +152,29 @@ describe('--experimental-detect-module', { concurrency: true }, () => { strictEqual(signal, null); }); } + + it('should not hint wrong format in resolve hook', async () => { + let writeSync; + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--no-warnings', + '--loader', + `data:text/javascript,import { writeSync } from "node:fs"; export ${encodeURIComponent( + async function resolve(s, c, next) { + const result = await next(s, c); + writeSync(1, result.format + '\n'); + return result; + } + )}`, + fixtures.path('es-modules/package-without-type/noext-esm'), + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'null\nexecuted\n'); + strictEqual(code, 0); + strictEqual(signal, null); + + }); }); describe('file input in a "type": "commonjs" package', { concurrency: true }, () => { From f4da308f8d7a93f7ea6919cd7914a117e11fbbfc Mon Sep 17 00:00:00 2001 From: Luke Albao Date: Mon, 23 Oct 2023 08:27:33 -0700 Subject: [PATCH 86/87] deps: V8: cherry-pick f7d000a7ae7b MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [logging] Bugfix: LinuxPerfBasicLogger should log JS functions This patch fixes a typo that was introduced in commit c51041f45400928cd64fbc8f389c0dd0dd15f82f / https://chromium-review.googlesource.com/c/v8/v8/+/2336793, which reversed the behavior of the perf_basic_prof_only_functions flag. This also refactors the equivalent guard in LinuxPerfJitLogger to use the same inline CodeKind API for identifying JS Functions. This is unrelated to the bug, but it seems a fair rider to add on here. Bug: v8:14387 Change-Id: I25766b0d45f4c65dfec5ae01e094a1ed94111054 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4954225 Reviewed-by: Camillo Bruni Commit-Queue: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#90501} Refs: https://github.com/v8/v8/commit/f7d000a7ae7b731805338338eb51a81fbcfe2628 PR-URL: https://github.com/nodejs/node/pull/50302 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Debadree Chatterjee --- common.gypi | 2 +- deps/v8/AUTHORS | 1 + deps/v8/src/diagnostics/perf-jit.cc | 5 ++--- deps/v8/src/logging/log.cc | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common.gypi b/common.gypi index 4c62386f4e8fce..d5328f39b28837 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.13', + 'v8_embedder_string': '-node.14', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/AUTHORS b/deps/v8/AUTHORS index b9b702c5d22400..cfc1360fa68ce9 100644 --- a/deps/v8/AUTHORS +++ b/deps/v8/AUTHORS @@ -178,6 +178,7 @@ Kyounga Ra Loo Rong Jie Lu Yahan Luis Reis +Luke Albao Luke Zarko Ma Aiguo Maciej Małecki diff --git a/deps/v8/src/diagnostics/perf-jit.cc b/deps/v8/src/diagnostics/perf-jit.cc index 59a774b270ce15..3ab0e68a4b1a80 100644 --- a/deps/v8/src/diagnostics/perf-jit.cc +++ b/deps/v8/src/diagnostics/perf-jit.cc @@ -43,6 +43,7 @@ #include "src/codegen/assembler.h" #include "src/codegen/source-position-table.h" #include "src/diagnostics/eh-frame.h" +#include "src/objects/code-kind.h" #include "src/objects/objects-inl.h" #include "src/objects/shared-function-info.h" #include "src/snapshot/embedded/embedded-data.h" @@ -225,9 +226,7 @@ void LinuxPerfJitLogger::LogRecordedBuffer( DisallowGarbageCollection no_gc; if (v8_flags.perf_basic_prof_only_functions) { CodeKind code_kind = abstract_code->kind(isolate_); - if (code_kind != CodeKind::INTERPRETED_FUNCTION && - code_kind != CodeKind::TURBOFAN && code_kind != CodeKind::MAGLEV && - code_kind != CodeKind::BASELINE) { + if (!CodeKindIsJSFunction(code_kind)) { return; } } diff --git a/deps/v8/src/logging/log.cc b/deps/v8/src/logging/log.cc index 8bd10449687bcf..b10f5d01186dcb 100644 --- a/deps/v8/src/logging/log.cc +++ b/deps/v8/src/logging/log.cc @@ -446,7 +446,7 @@ void LinuxPerfBasicLogger::LogRecordedBuffer(Tagged code, DisallowGarbageCollection no_gc; PtrComprCageBase cage_base(isolate_); if (v8_flags.perf_basic_prof_only_functions && - CodeKindIsBuiltinOrJSFunction(code->kind(cage_base))) { + !CodeKindIsBuiltinOrJSFunction(code->kind(cage_base))) { return; } From f4e5bebe7d83727cd64ed4762a59e1336f5f3c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Mon, 23 Oct 2023 11:32:17 +0200 Subject: [PATCH 87/87] 2023-10-24, Version 21.1.0 (Current) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Notable changes: doc: * add H4ad to collaborators (Vinícius Lourenço) https://github.com/nodejs/node/pull/50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) https://github.com/nodejs/node/pull/50096 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) https://github.com/nodejs/node/pull/50095 lib: * (SEMVER-MINOR) add `navigator.userAgent` (Yagiz Nizipli) https://github.com/nodejs/node/pull/50200 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) https://github.com/nodejs/node/pull/50187 * call helper function from push and unshift (Raz Luvaton) https://github.com/nodejs/node/pull/50173 PR-URL: https://github.com/nodejs/node/pull/50335 --- CHANGELOG.md | 3 +- doc/api/cli.md | 2 +- doc/api/errors.md | 10 +-- doc/api/fs.md | 8 +-- doc/api/globals.md | 2 +- doc/api/stream.md | 2 +- doc/api/vm.md | 2 +- doc/changelogs/CHANGELOG_V21.md | 124 ++++++++++++++++++++++++++++++++ src/node_version.h | 6 +- 9 files changed, 142 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01b8655f1c5357..9274cbed3f32fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,8 @@ release. -21.0.0
+21.1.0
+21.0.0
20.8.1
diff --git a/doc/api/cli.md b/doc/api/cli.md index 61f0ba9f053bc9..3cb36818ae4997 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -624,7 +624,7 @@ JavaScript. > Stability: 1.0 - Early development diff --git a/doc/api/errors.md b/doc/api/errors.md index 9ff60a01977693..50e9f658fcbf3a 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1755,7 +1755,7 @@ An attempt was made to construct an object using a non-public constructor. An import `type` attribute was provided, but the specified module is of a @@ -1767,7 +1767,7 @@ different type. An import attribute is missing, preventing the specified module to be imported. @@ -3302,7 +3302,7 @@ An invalid transfer object was passed to `postMessage()`. added: - v17.1.0 - v16.14.0 -removed: REPLACEME +removed: v21.1.0 --> An import assertion has failed, preventing the specified module to be imported. @@ -3315,7 +3315,7 @@ An import assertion has failed, preventing the specified module to be imported. added: - v17.1.0 - v16.14.0 -removed: REPLACEME +removed: v21.1.0 --> An import assertion is missing, preventing the specified module to be imported. @@ -3328,7 +3328,7 @@ An import assertion is missing, preventing the specified module to be imported. added: - v17.1.0 - v16.14.0 -removed: REPLACEME +removed: v21.1.0 --> An import attribute is not supported by this version of Node.js. diff --git a/doc/api/fs.md b/doc/api/fs.md index fabbb60eeed73b..8079b7cc316fec 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -180,7 +180,7 @@ longer be used. @@ -2063,7 +2063,7 @@ the user from reading or writing to it. * {string} diff --git a/doc/api/stream.md b/doc/api/stream.md index 680bd4d260f504..83b26c4d6a95b2 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -2823,7 +2823,7 @@ const server = http.createServer((req, res) => {