src: allow embedders to disable esm loader

codebytere · addaleax · commit 5e28660121fe · 2020-09-23T22:34:06.000+02:00
PR-URL: #34060 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
diff --git 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');
@@ -64,7 +67,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
@@ -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
@@ -859,6 +859,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
@@ -1063,6 +1063,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
@@ -396,7 +396,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
@@ -994,6 +994,12 @@ void Initialize(Local<Object> 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<Object> 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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -859,6 +859,10 @@ inline bool Environment::is_main_thread() const {`
`859`	`859`	`return worker_context() == nullptr;`
`860`	`860`	`}`
`861`	`861`
	`862`	`+inline bool Environment::should_not_register_esm_loader() const {`
	`863`	`+ return flags_ & EnvironmentFlags::kNoRegisterESMLoader;`
	`864`	`+}`
	`865`	`+`
`862`	`866`	`inline bool Environment::owns_process_state() const {`
`863`	`867`	`return flags_ & EnvironmentFlags::kOwnsProcessState;`
`864`	`868`	`}`