Browse Source

fix: setCertVerifyProc canceling unrelated requests (#26517)

Jeremy Rose 4 years ago
parent
commit
ead13791a8
3 changed files with 32 additions and 11 deletions
  1. 2 0
      docs/api/session.md
  2. 0 3
      shell/browser/api/electron_api_session.cc
  3. 30 8
      spec-main/api-session-spec.ts

+ 2 - 0
docs/api/session.md

@@ -483,6 +483,8 @@ win.webContents.session.setCertificateVerifyProc((request, callback) => {
 })
 ```
 
+> **NOTE:** The result of this procedure is cached by the network service.
+
 #### `ses.setPermissionRequestHandler(handler)`
 
 * `handler` Function | null

+ 0 - 3
shell/browser/api/electron_api_session.cc

@@ -619,9 +619,6 @@ void Session::SetCertVerifyProc(v8::Local<v8::Value> val,
   content::BrowserContext::GetDefaultStoragePartition(browser_context_)
       ->GetNetworkContext()
       ->SetCertVerifierClient(std::move(cert_verifier_client_remote));
-
-  // This causes the cert verifier cache to be cleared.
-  content::GetNetworkService()->OnCertDBChanged();
 }
 
 void Session::SetPermissionRequestHandler(v8::Local<v8::Value> val,

+ 30 - 8
spec-main/api-session-spec.ts

@@ -539,7 +539,6 @@ describe('session module', () => {
           fs.readFileSync(path.join(certPath, 'rootCA.pem')),
           fs.readFileSync(path.join(certPath, 'intermediateCA.pem'))
         ],
-        requestCert: true,
         rejectUnauthorized: false
       };
 
@@ -551,25 +550,26 @@ describe('session module', () => {
     });
 
     afterEach((done) => {
-      session.defaultSession.setCertificateVerifyProc(null);
       server.close(done);
     });
     afterEach(closeAllWindows);
 
     it('accepts the request when the callback is called with 0', async () => {
-      session.defaultSession.setCertificateVerifyProc(({ verificationResult, errorCode }, callback) => {
+      const ses = session.fromPartition(`${Math.random()}`);
+      ses.setCertificateVerifyProc(({ verificationResult, errorCode }, callback) => {
         expect(['net::ERR_CERT_AUTHORITY_INVALID', 'net::ERR_CERT_COMMON_NAME_INVALID'].includes(verificationResult)).to.be.true();
         expect([-202, -200].includes(errorCode)).to.be.true();
         callback(0);
       });
 
-      const w = new BrowserWindow({ show: false });
+      const w = new BrowserWindow({ show: false, webPreferences: { session: ses } });
       await w.loadURL(`https://127.0.0.1:${(server.address() as AddressInfo).port}`);
       expect(w.webContents.getTitle()).to.equal('hello');
     });
 
     it('rejects the request when the callback is called with -2', async () => {
-      session.defaultSession.setCertificateVerifyProc(({ hostname, certificate, verificationResult }, callback) => {
+      const ses = session.fromPartition(`${Math.random()}`);
+      ses.setCertificateVerifyProc(({ hostname, certificate, verificationResult }, callback) => {
         expect(hostname).to.equal('127.0.0.1');
         expect(certificate.issuerName).to.equal('Intermediate CA');
         expect(certificate.subjectName).to.equal('localhost');
@@ -585,26 +585,48 @@ describe('session module', () => {
       });
 
       const url = `https://127.0.0.1:${(server.address() as AddressInfo).port}`;
-      const w = new BrowserWindow({ show: false });
+      const w = new BrowserWindow({ show: false, webPreferences: { session: ses } });
       await expect(w.loadURL(url)).to.eventually.be.rejectedWith(/ERR_FAILED/);
       expect(w.webContents.getTitle()).to.equal(url);
     });
 
     it('saves cached results', async () => {
+      const ses = session.fromPartition(`${Math.random()}`);
       let numVerificationRequests = 0;
-      session.defaultSession.setCertificateVerifyProc((e, callback) => {
+      ses.setCertificateVerifyProc((e, callback) => {
         numVerificationRequests++;
         callback(-2);
       });
 
       const url = `https://127.0.0.1:${(server.address() as AddressInfo).port}`;
-      const w = new BrowserWindow({ show: false });
+      const w = new BrowserWindow({ show: false, webPreferences: { session: ses } });
       await expect(w.loadURL(url), 'first load').to.eventually.be.rejectedWith(/ERR_FAILED/);
       await emittedOnce(w.webContents, 'did-stop-loading');
       await expect(w.loadURL(url + '/test'), 'second load').to.eventually.be.rejectedWith(/ERR_FAILED/);
       expect(w.webContents.getTitle()).to.equal(url + '/test');
       expect(numVerificationRequests).to.equal(1);
     });
+
+    it('does not cancel requests in other sessions', async () => {
+      const ses1 = session.fromPartition(`${Math.random()}`);
+      ses1.setCertificateVerifyProc((opts, cb) => cb(0));
+      const ses2 = session.fromPartition(`${Math.random()}`);
+
+      const url = `https://127.0.0.1:${(server.address() as AddressInfo).port}`;
+      const req = net.request({ url, session: ses1, credentials: 'include' });
+      req.end();
+      setTimeout(() => {
+        ses2.setCertificateVerifyProc((opts, callback) => callback(0));
+      });
+      await expect(new Promise((resolve, reject) => {
+        req.on('error', (err) => {
+          reject(err);
+        });
+        req.on('response', () => {
+          resolve();
+        });
+      })).to.eventually.be.fulfilled();
+    });
   });
 
   describe('ses.clearAuthCache()', () => {