From c23d2fd3f8a018bcc624f1468a9ce176f6ce93f2 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 25 Jun 2020 18:07:37 -0700 Subject: [PATCH] src: allow embedders to disable esm loader PR-URL: https://github.com/nodejs/node/pull/34060 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: David Carlier Reviewed-By: Gus Caplan --- lib/internal/bootstrap/pre_execution.js | 10 ++++- lib/internal/options.js | 3 +- src/env-inl.h | 4 ++ src/env.h | 1 + src/node.h | 6 ++- src/node_options.cc | 6 +++ test/cctest/test_environment.cc | 50 +++++++++++++++++++++++++ 7 files changed, 76 insertions(+), 4 deletions(-) diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index f60814d2dc9e28..a51dbf05ec4f86 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -6,7 +6,10 @@ const { SafeWeakMap, } = primordials; -const { getOptionValue } = require('internal/options'); +const { + getOptionValue, + shouldNotRegisterESMLoader +} = require('internal/options'); const { Buffer } = require('buffer'); const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes; const assert = require('internal/assert'); @@ -56,7 +59,10 @@ function prepareMainThreadExecution(expandArgv1 = false) { initializeDeprecations(); initializeWASI(); initializeCJSLoader(); - initializeESMLoader(); + + if (!shouldNotRegisterESMLoader) { + initializeESMLoader(); + } const CJSLoader = require('internal/modules/cjs/loader'); assert(!CJSLoader.hasLoadedAnyUserCJSModule); diff --git a/lib/internal/options.js b/lib/internal/options.js index 03586f9dae6d76..10c6aa2d9a0978 100644 --- a/lib/internal/options.js +++ b/lib/internal/options.js @@ -1,6 +1,6 @@ 'use strict'; -const { getOptions } = internalBinding('options'); +const { getOptions, shouldNotRegisterESMLoader } = internalBinding('options'); const { options, aliases } = getOptions(); let warnOnAllowUnauthorized = true; @@ -32,4 +32,5 @@ module.exports = { aliases, getOptionValue, getAllowUnauthorized, + shouldNotRegisterESMLoader }; diff --git a/src/env-inl.h b/src/env-inl.h index 5cbb2cd60bba99..690e7970436e7b 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -767,6 +767,10 @@ inline bool Environment::is_main_thread() const { return worker_context() == nullptr; } +inline bool Environment::should_not_register_esm_loader() const { + return flags_ & EnvironmentFlags::kNoRegisterESMLoader; +} + inline bool Environment::owns_process_state() const { return flags_ & EnvironmentFlags::kOwnsProcessState; } diff --git a/src/env.h b/src/env.h index 175e1a3f8d227a..2add16c8eddaa7 100644 --- a/src/env.h +++ b/src/env.h @@ -1054,6 +1054,7 @@ class Environment : public MemoryRetainer { inline void set_has_serialized_options(bool has_serialized_options); inline bool is_main_thread() const; + inline bool should_not_register_esm_loader() const; inline bool owns_process_state() const; inline bool owns_inspector() const; inline uint64_t thread_id() const; diff --git a/src/node.h b/src/node.h index b3b7c0c1693178..6a6a40113b271b 100644 --- a/src/node.h +++ b/src/node.h @@ -408,7 +408,11 @@ enum Flags : uint64_t { // Set if this Environment instance is associated with the global inspector // handling code (i.e. listening on SIGUSR1). // This is set when using kDefaultFlags. - kOwnsInspector = 1 << 2 + kOwnsInspector = 1 << 2, + // Set if Node.js should not run its own esm loader. This is needed by some + // embedders, because it's possible for the Node.js esm loader to conflict + // with another one in an embedder environment, e.g. Blink's in Chromium. + kNoRegisterESMLoader = 1 << 3 }; } // namespace EnvironmentFlags diff --git a/src/node_options.cc b/src/node_options.cc index d7434b4b124872..913366b3500442 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -979,6 +979,12 @@ void Initialize(Local target, context, FIXED_ONE_BYTE_STRING(isolate, "envSettings"), env_settings) .Check(); + target + ->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), + Boolean::New(isolate, env->should_not_register_esm_loader())) + .Check(); + Local types = Object::New(isolate); NODE_DEFINE_CONSTANT(types, kNoOp); NODE_DEFINE_CONSTANT(types, kV8Option); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index d0b241528ba31e..0314c7e5b8ad04 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -32,6 +32,56 @@ class EnvironmentTest : public EnvironmentTestFixture { } }; +TEST_F(EnvironmentTest, EnvironmentWithESMLoader) { + const v8::HandleScope handle_scope(isolate_); + Argv argv; + Env env {handle_scope, argv}; + + node::Environment* envi = *env; + envi->options()->experimental_vm_modules = true; + + SetProcessExitHandler(*env, [&](node::Environment* env_, int exit_code) { + EXPECT_EQ(*env, env_); + EXPECT_EQ(exit_code, 0); + node::Stop(*env); + }); + + node::LoadEnvironment( + *env, + "const { SourceTextModule } = require('vm');" + "try {" + "new SourceTextModule('export const a = 1;');" + "process.exit(0);" + "} catch {" + "process.exit(42);" + "}"); +} + +TEST_F(EnvironmentTest, EnvironmentWithNoESMLoader) { + const v8::HandleScope handle_scope(isolate_); + Argv argv; + Env env {handle_scope, argv, node::EnvironmentFlags::kNoRegisterESMLoader}; + + node::Environment* envi = *env; + envi->options()->experimental_vm_modules = true; + + SetProcessExitHandler(*env, [&](node::Environment* env_, int exit_code) { + EXPECT_EQ(*env, env_); + EXPECT_EQ(exit_code, 42); + node::Stop(*env); + }); + + node::LoadEnvironment( + *env, + "const { SourceTextModule } = require('vm');" + "try {" + "new SourceTextModule('export const a = 1;');" + "process.exit(0);" + "} catch {" + "process.exit(42);" + "}"); +} + TEST_F(EnvironmentTest, PreExecutionPreparation) { const v8::HandleScope handle_scope(isolate_); const Argv argv;