Browse Source

fix: check for Node.js-created module when `contextIsolation` disabled (#40993)

Shelley Vohr 1 year ago
parent
commit
6803624576

+ 18 - 1
patches/node/chore_expose_importmoduledynamically_and.patch

@@ -87,7 +87,7 @@ index 52c30dcb47d1faba0c2267e4381a624e450baa02..ba4c1a0d5a987e4d410b49f5c4716694
  
  MaybeLocal<Value> ModuleWrap::SyntheticModuleEvaluationStepsCallback(
 diff --git a/src/module_wrap.h b/src/module_wrap.h
-index 1fc801edced9c5e44613846b4dc555804c5bae97..23a0d7aee1dfa0ebe26e0507e31eacb0b4d137ed 100644
+index 1fc801edced9c5e44613846b4dc555804c5bae97..767d183c6b6b6d97726cf5d63c0b202bd9ae10a6 100644
 --- a/src/module_wrap.h
 +++ b/src/module_wrap.h
 @@ -30,7 +30,14 @@ enum HostDefinedOptions : int {
@@ -106,3 +106,20 @@ index 1fc801edced9c5e44613846b4dc555804c5bae97..23a0d7aee1dfa0ebe26e0507e31eacb0
   public:
    enum InternalFields {
      kModuleSlot = BaseObject::kInternalFieldCount,
+@@ -65,6 +72,8 @@ class ModuleWrap : public BaseObject {
+     return true;
+   }
+ 
++  static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
++
+  private:
+   ModuleWrap(Environment* env,
+              v8::Local<v8::Object> object,
+@@ -99,7 +108,6 @@ class ModuleWrap : public BaseObject {
+       v8::Local<v8::String> specifier,
+       v8::Local<v8::FixedArray> import_attributes,
+       v8::Local<v8::Module> referrer);
+-  static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
+ 
+   v8::Global<v8::Module> module_;
+   std::unordered_map<std::string, v8::Global<v8::Promise>> resolve_cache_;

+ 1 - 1
patches/node/fix_missing_include_for_node_extern.patch

@@ -13,7 +13,7 @@ causing the following error:
 This should be upstreamed.
 
 diff --git a/src/module_wrap.h b/src/module_wrap.h
-index 23a0d7aee1dfa0ebe26e0507e31eacb0b4d137ed..0733017d8e1ac6e60589082b402bd44a98ddc312 100644
+index 767d183c6b6b6d97726cf5d63c0b202bd9ae10a6..a134b3817c0a10174ead927589626789851760d0 100644
 --- a/src/module_wrap.h
 +++ b/src/module_wrap.h
 @@ -7,6 +7,7 @@

+ 15 - 5
shell/common/node_bindings.cc

@@ -119,6 +119,8 @@ ELECTRON_TESTING_BINDINGS(V)
 #endif
 #undef V
 
+using node::loader::ModuleWrap;
+
 namespace {
 
 void stop_and_close_uv_loop(uv_loop_t* loop) {
@@ -209,16 +211,24 @@ v8::MaybeLocal<v8::Promise> HostImportModuleDynamically(
 void HostInitializeImportMetaObject(v8::Local<v8::Context> context,
                                     v8::Local<v8::Module> module,
                                     v8::Local<v8::Object> meta) {
-  if (node::Environment::GetCurrent(context) == nullptr) {
+  node::Environment* env = node::Environment::GetCurrent(context);
+  if (env == nullptr) {
     if (electron::IsBrowserProcess() || electron::IsUtilityProcess())
       return;
     return blink::V8Initializer::HostGetImportMetaProperties(context, module,
                                                              meta);
   }
 
-  // If we're running with contextIsolation enabled in the renderer process,
-  // fall back to Blink's logic.
   if (electron::IsRendererProcess()) {
+    // If the module is created by Node.js, use Node.js' handling.
+    if (env != nullptr) {
+      ModuleWrap* wrap = ModuleWrap::GetFromModule(env, module);
+      if (wrap)
+        return ModuleWrap::HostInitializeImportMetaObjectCallback(context,
+                                                                  module, meta);
+    }
+
+    // If contextIsolation is enabled, fall back to Blink's handling.
     blink::WebLocalFrame* frame =
         blink::WebLocalFrame::FrameForContext(context);
     if (!frame || frame->GetScriptContextWorldId(context) !=
@@ -228,8 +238,8 @@ void HostInitializeImportMetaObject(v8::Local<v8::Context> context,
     }
   }
 
-  return node::loader::ModuleWrap::HostInitializeImportMetaObjectCallback(
-      context, module, meta);
+  return ModuleWrap::HostInitializeImportMetaObjectCallback(context, module,
+                                                            meta);
 }
 
 v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(

+ 8 - 3
spec/esm-spec.ts

@@ -141,7 +141,7 @@ describe('esm', () => {
       const hostsUrl = pathToFileURL(process.platform === 'win32' ? 'C:\\Windows\\System32\\drivers\\etc\\hosts' : '/etc/hosts');
 
       describe('without context isolation', () => {
-        it('should use blinks dynamic loader in the main world', async () => {
+        it('should use Blinks dynamic loader in the main world', async () => {
           const [webContents] = await loadWindowWithPreload('', {
             nodeIntegration: true,
             sandbox: false,
@@ -159,6 +159,11 @@ describe('esm', () => {
           // This is a blink specific error message
           expect(error?.message).to.include('Failed to fetch dynamically imported module');
         });
+
+        it('should use import.meta callback handling from Node.js for Node.js modules', async () => {
+          const result = await runFixture(path.resolve(fixturePath, 'import-meta'));
+          expect(result.code).to.equal(0);
+        });
       });
 
       describe('with context isolation', () => {
@@ -173,7 +178,7 @@ describe('esm', () => {
           await fs.promises.unlink(badFilePath);
         });
 
-        it('should use nodes esm dynamic loader in the isolated context', async () => {
+        it('should use Node.js ESM dynamic loader in the isolated context', async () => {
           const [, preloadError] = await loadWindowWithPreload(`await import(${JSON.stringify((pathToFileURL(badFilePath)))})`, {
             nodeIntegration: true,
             sandbox: false,
@@ -185,7 +190,7 @@ describe('esm', () => {
           expect(preloadError!.toString()).to.include('Unknown file extension');
         });
 
-        it('should use blinks dynamic loader in the main world', async () => {
+        it('should use Blinks dynamic loader in the main world', async () => {
           const [webContents] = await loadWindowWithPreload('', {
             nodeIntegration: true,
             sandbox: false,

+ 15 - 0
spec/fixtures/esm/import-meta/index.html

@@ -0,0 +1,15 @@
+
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="UTF-8">
+    <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'">
+    <title>Hello World!</title>
+  </head>
+  <body>
+    <h1>Hello World!</h1>
+    We are using Node.js <span id="node-version"></span>,
+    Chromium <span id="chrome-version"></span>,
+    and Electron <span id="electron-version"></span>.
+  </body>
+</html>

+ 33 - 0
spec/fixtures/esm/import-meta/main.mjs

@@ -0,0 +1,33 @@
+import { app, BrowserWindow } from 'electron'
+import { fileURLToPath } from 'node:url'
+import { dirname, join } from 'node:path';
+
+async function createWindow() {
+  const mainWindow = new BrowserWindow({
+    show: false,
+    webPreferences: {
+      preload: fileURLToPath(new URL('preload.mjs', import.meta.url)),
+      sandbox: false,
+      contextIsolation: false
+    }
+  })
+
+  await mainWindow.loadFile('index.html')
+
+  const importMetaPreload = await mainWindow.webContents.executeJavaScript('window.importMetaPath');
+  const expected = join(dirname(fileURLToPath(import.meta.url)), 'preload.mjs');
+
+  process.exit(importMetaPreload === expected ? 0 : 1);
+}
+
+app.whenReady().then(() => {
+  createWindow()
+
+  app.on('activate', function () {
+    if (BrowserWindow.getAllWindows().length === 0) createWindow()
+  })
+})
+
+app.on('window-all-closed', function () {
+  if (process.platform !== 'darwin') app.quit()
+})

+ 4 - 0
spec/fixtures/esm/import-meta/package.json

@@ -0,0 +1,4 @@
+{
+  "main": "main.mjs",
+  "type": "module"
+}

+ 3 - 0
spec/fixtures/esm/import-meta/preload.mjs

@@ -0,0 +1,3 @@
+import { fileURLToPath } from 'node:url'
+
+window.importMetaPath = fileURLToPath(import.meta.url)