fix: crash when `setWindowOpenHandler` callback throws (#34772)

fix: crash when `setWindowOpenHandler` callback throws (#34523)

lib/browser/api/web-contents.ts

@@ -500,6 +500,7 @@ WebContents.prototype._callWindowOpenHandler = function (event: Electron.Event,
   if (!this._windowOpenHandler) {
     return null;
   }
+
   const response = this._windowOpenHandler(details);
 
   if (typeof response !== 'object') {
@@ -652,7 +653,15 @@ WebContents.prototype._init = function () {
         postBody,
         disposition
       };
-      const options = this._callWindowOpenHandler(event, details);
+
+      let options;
+      try {
+        options = this._callWindowOpenHandler(event, details);
+      } catch (err) {
+        event.preventDefault();
+        throw err;
+      }
+
       if (!event.defaultPrevented) {
         openGuestWindow({
           event,
@@ -680,7 +689,16 @@ WebContents.prototype._init = function () {
         referrer,
         postBody
       };
-      windowOpenOverriddenOptions = this._callWindowOpenHandler(event, details);
+
+      let result;
+      try {
+        result = this._callWindowOpenHandler(event, details);
+      } catch (err) {
+        event.preventDefault();
+        throw err;
+      }
+
+      windowOpenOverriddenOptions = result;
       if (!event.defaultPrevented) {
         const secureOverrideWebPreferences = windowOpenOverriddenOptions ? {
           // Allow setting of backgroundColor as a webPreference even though

spec-main/guest-window-manager-spec.ts

@@ -123,6 +123,58 @@ describe('webContents.setWindowOpenHandler', () => {
 
       afterEach(closeAllWindows);
 
+      it('does not fire window creation events if the handler callback throws an error', (done) => {
+        const error = new Error('oh no');
+        const listeners = process.listeners('uncaughtException');
+        process.removeAllListeners('uncaughtException');
+        process.on('uncaughtException', (thrown) => {
+          try {
+            expect(thrown).to.equal(error);
+            done();
+          } catch (e) {
+            done(e);
+          } finally {
+            process.removeAllListeners('uncaughtException');
+            listeners.forEach((listener) => process.on('uncaughtException', listener));
+          }
+        });
+
+        browserWindow.webContents.on('new-window', () => {
+          assert.fail('new-window should not be called with an overridden window.open');
+        });
+
+        browserWindow.webContents.on('did-create-window', () => {
+          assert.fail('did-create-window should not be called with an overridden window.open');
+        });
+
+        browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true");
+
+        browserWindow.webContents.setWindowOpenHandler(() => {
+          throw error;
+        });
+      });
+
+      it('does not fire window creation events if the handler callback returns a bad result', async () => {
+        const bad = new Promise((resolve) => {
+          browserWindow.webContents.setWindowOpenHandler(() => {
+            setTimeout(resolve);
+            return [1, 2, 3] as any;
+          });
+        });
+
+        browserWindow.webContents.on('new-window', () => {
+          assert.fail('new-window should not be called with an overridden window.open');
+        });
+
+        browserWindow.webContents.on('did-create-window', () => {
+          assert.fail('did-create-window should not be called with an overridden window.open');
+        });
+
+        browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true");
+
+        await bad;
+      });
+
       it('does not fire window creation events if an override returns action: deny', async () => {
         const denied = new Promise((resolve) => {
           browserWindow.webContents.setWindowOpenHandler(() => {
@@ -131,11 +183,11 @@ describe('webContents.setWindowOpenHandler', () => {
           });
         });
         browserWindow.webContents.on('new-window', () => {
-          assert.fail('new-window should not to be called with an overridden window.open');
+          assert.fail('new-window should not be called with an overridden window.open');
         });
 
         browserWindow.webContents.on('did-create-window', () => {
-          assert.fail('did-create-window should not to be called with an overridden window.open');
+          assert.fail('did-create-window should not be called with an overridden window.open');
         });
 
         browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true");
@@ -151,11 +203,11 @@ describe('webContents.setWindowOpenHandler', () => {
           });
         });
         browserWindow.webContents.on('new-window', () => {
-          assert.fail('new-window should not to be called with an overridden window.open');
+          assert.fail('new-window should not to called with an overridden window.open');
         });
 
         browserWindow.webContents.on('did-create-window', () => {
-          assert.fail('did-create-window should not to be called with an overridden window.open');
+          assert.fail('did-create-window should not be called with an overridden window.open');
         });
 
         await browserWindow.webContents.loadURL('data:text/html,<a target="_blank" href="http://example.com" style="display: block; width: 100%; height: 100%; position: fixed; left: 0; top: 0;">link</a>');
@@ -173,11 +225,11 @@ describe('webContents.setWindowOpenHandler', () => {
           });
         });
         browserWindow.webContents.on('new-window', () => {
-          assert.fail('new-window should not to be called with an overridden window.open');
+          assert.fail('new-window should not to called with an overridden window.open');
         });
 
         browserWindow.webContents.on('did-create-window', () => {
-          assert.fail('did-create-window should not to be called with an overridden window.open');
+          assert.fail('did-create-window should not be called with an overridden window.open');
         });
 
         await browserWindow.webContents.loadURL('data:text/html,<a href="http://example.com" style="display: block; width: 100%; height: 100%; position: fixed; left: 0; top: 0;">link</a>');