fix: don't crash on invalid certs (#22124)

* test: add test for app.on('certificate-error') event (#21978)

* types

* fix: don't crash on invalid certs (#21976)

shell/browser/api/electron_api_app.cc

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/files/file_path.h"
@@ -701,15 +702,17 @@ void App::AllowCertificateError(
     bool is_main_frame_request,
     bool strict_enforcement,
     base::OnceCallback<void(content::CertificateRequestResultType)> callback) {
+  auto adapted_callback = base::AdaptCallbackForRepeating(std::move(callback));
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());
-  bool prevent_default = Emit(
-      "certificate-error", WebContents::FromOrCreate(isolate(), web_contents),
-      request_url, net::ErrorToString(cert_error), ssl_info.cert, callback);
+  bool prevent_default =
+      Emit("certificate-error",
+           WebContents::FromOrCreate(isolate(), web_contents), request_url,
+           net::ErrorToString(cert_error), ssl_info.cert, adapted_callback);
 
   // Deny the certificate by default.
   if (!prevent_default)
-    std::move(callback).Run(content::CERTIFICATE_REQUEST_RESULT_TYPE_DENY);
+    adapted_callback.Run(content::CERTIFICATE_REQUEST_RESULT_TYPE_DENY);
 }
 
 base::OnceClosure App::SelectClientCertificate(

spec-main/api-app-spec.ts

@@ -8,7 +8,7 @@ import * as fs from 'fs'
 import * as path from 'path'
 import { homedir } from 'os'
 import split = require('split')
-import { app, BrowserWindow, Menu } from 'electron'
+import { app, BrowserWindow, Menu, session } from 'electron'
 import { emittedOnce } from './events-helpers';
 import { closeWindow, closeAllWindows } from './window-helpers';
 import { ifdescribe } from './spec-helpers';
@@ -317,6 +317,15 @@ describe('app module', () => {
     })
   })
 
+  describe('certificate-error event', () => {
+    afterEach(closeAllWindows)
+    it('is emitted when visiting a server with a self-signed cert', async () => {
+      const w = new BrowserWindow({ show: false })
+      w.loadURL(secureUrl)
+      await emittedOnce(app, 'certificate-error')
+    })
+  })
+
   // xdescribe('app.importCertificate', () => {
   //   let w = null
 
@@ -745,6 +754,7 @@ describe('app module', () => {
       if (process.platform === 'linux') {
         this.skip()
       }
+      session.fromPartition('empty-certificate').setCertificateVerifyProc((req, cb) => { cb(0) })
     })
 
     beforeEach(() => {
@@ -759,6 +769,8 @@ describe('app module', () => {
 
     afterEach(() => closeWindow(w).then(() => { w = null as any }))
 
+    after(() => session.fromPartition('empty-certificate').setCertificateVerifyProc(null))
+
     it('can respond with empty certificate list', async () => {
       app.once('select-client-certificate', function (event, webContents, url, list, callback) {
         console.log('select-client-certificate emitted')

spec-main/index.js

@@ -20,9 +20,6 @@ v8.setFlagsFromString('--expose_gc')
 app.commandLine.appendSwitch('js-flags', '--expose_gc')
 // Prevent the spec runner quiting when the first window closes
 app.on('window-all-closed', () => null)
-// TODO: This API should _probably_ only be enabled for the specific test that needs it
-// not the entire test suite
-app.commandLine.appendSwitch('ignore-certificate-errors')
 
 // Use fake device for Media Stream to replace actual camera and microphone.
 app.commandLine.appendSwitch('use-fake-device-for-media-stream')