Emit verify-certificate only when default verifier fails

atom/browser/api/atom_api_session.cc

@@ -266,11 +266,12 @@ void Session::RequestCertVerification(
     const scoped_refptr<AtomCertVerifier::CertVerifyRequest>& request) {
   bool prevent_default = Emit(
       "verify-certificate",
-      request->hostname(),
-      make_scoped_refptr(request->cert()),
+      request->args().hostname,
+      request->args().cert,
       base::Bind(&PassVerificationResult, request));
 
   if (!prevent_default)
+    // Tell the request to use the result of default verifier.
     request->ContinueWithResult(net::ERR_IO_PENDING);
 }
 

atom/browser/net/atom_cert_verifier.cc

@@ -15,6 +15,9 @@ using content::BrowserThread;
 
 namespace atom {
 
+AtomCertVerifier::CertVerifyRequest::~CertVerifyRequest() {
+}
+
 void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -22,25 +25,10 @@ void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) {
     return;
 
   handled_ = true;
-
-  if (result != net::ERR_IO_PENDING) {
-    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                            base::Bind(callback_, result));
-    return;
-  }
-
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&CertVerifyRequest::DelegateToDefaultVerifier, this));
-}
-
-void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  int result = cert_verifier_->default_cert_verifier_->Verify(
-      cert_.get(), hostname_, ocsp_response_, flags_, crl_set_.get(),
-      verify_result_, callback_, out_req_, net_log_);
-  if (result != net::ERR_IO_PENDING)
-    callback_.Run(result);
+      base::Bind(args_.callback,
+                 result == net::ERR_IO_PENDING ? result_ : result));
 }
 
 AtomCertVerifier::AtomCertVerifier()
@@ -66,18 +54,42 @@ int AtomCertVerifier::Verify(
   if (callback.is_null() || !verify_result || hostname.empty() || !delegate_)
     return net::ERR_INVALID_ARGUMENT;
 
-  CertVerifyRequest* request = new CertVerifyRequest(
-      this, cert, hostname, ocsp_response, flags, crl_set, verify_result,
-      callback, out_req, net_log);
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                          base::Bind(&Delegate::RequestCertVerification,
-                                     base::Unretained(delegate_),
-                                     make_scoped_refptr(request)));
-  return net::ERR_IO_PENDING;
+  VerifyArgs args = { cert, hostname, callback };
+  int result = default_cert_verifier_->Verify(
+      cert, hostname, ocsp_response, flags, crl_set, verify_result,
+      base::Bind(&AtomCertVerifier::OnDefaultVerificationResult,
+                 base::Unretained(this), args),
+      out_req, net_log);
+  if (result != net::OK && result != net::ERR_IO_PENDING) {
+    // The default verifier fails immediately.
+    VerifyCertificateFromDelegate(args, result);
+    return net::ERR_IO_PENDING;
+  }
+
+  return result;
 }
 
 bool AtomCertVerifier::SupportsOCSPStapling() {
   return true;
 }
 
+void AtomCertVerifier::VerifyCertificateFromDelegate(
+    const VerifyArgs& args, int result) {
+  CertVerifyRequest* request = new CertVerifyRequest(this, result, args);
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(&Delegate::RequestCertVerification,
+                                     base::Unretained(delegate_),
+                                     make_scoped_refptr(request)));
+}
+
+void AtomCertVerifier::OnDefaultVerificationResult(
+    const VerifyArgs& args, int result) {
+  if (result == net::OK) {
+    args.callback.Run(result);
+    return;
+  }
+
+  VerifyCertificateFromDelegate(args, result);
+}
+
 }  // namespace atom

atom/browser/net/atom_cert_verifier.h

@@ -14,55 +14,35 @@ namespace atom {
 
 class AtomCertVerifier : public net::CertVerifier {
  public:
+  struct VerifyArgs {
+    scoped_refptr<net::X509Certificate> cert;
+    const std::string& hostname;
+    net::CompletionCallback callback;
+  };
+
   class CertVerifyRequest
       : public base::RefCountedThreadSafe<CertVerifyRequest> {
    public:
     CertVerifyRequest(AtomCertVerifier* cert_verifier,
-                      net::X509Certificate* cert,
-                      const std::string& hostname,
-                      const std::string& ocsp_response,
-                      int flags,
-                      net::CRLSet* crl_set,
-                      net::CertVerifyResult* verify_result,
-                      const net::CompletionCallback& callback,
-                      scoped_ptr<Request>* out_req,
-                      const net::BoundNetLog& net_log)
+                      int result,
+                      const VerifyArgs& args)
         : cert_verifier_(cert_verifier),
-          cert_(cert),
-          hostname_(hostname),
-          ocsp_response_(ocsp_response),
-          flags_(flags),
-          crl_set_(crl_set),
-          verify_result_(verify_result),
-          callback_(callback),
-          out_req_(out_req),
-          net_log_(net_log),
+          result_(result),
+          args_(args),
           handled_(false) {
     }
 
     void ContinueWithResult(int result);
 
-    const std::string& hostname() const { return hostname_; }
-    net::X509Certificate* cert() const { return cert_.get(); }
+    const VerifyArgs& args() const { return args_; }
 
    private:
     friend class base::RefCountedThreadSafe<CertVerifyRequest>;
-    ~CertVerifyRequest() {}
-
-    void DelegateToDefaultVerifier();
+    ~CertVerifyRequest();
 
     AtomCertVerifier* cert_verifier_;
-
-    scoped_refptr<net::X509Certificate> cert_;
-    std::string hostname_;
-    std::string ocsp_response_;
-    int flags_;
-    scoped_refptr<net::CRLSet> crl_set_;
-    net::CertVerifyResult* verify_result_;
-    net::CompletionCallback callback_;
-    scoped_ptr<Request>* out_req_;
-    const net::BoundNetLog& net_log_;
-
+    int result_;
+    VerifyArgs args_;
     bool handled_;
 
     DISALLOW_COPY_AND_ASSIGN(CertVerifyRequest);
@@ -100,6 +80,9 @@ class AtomCertVerifier : public net::CertVerifier {
  private:
   friend class CertVerifyRequest;
 
+  void VerifyCertificateFromDelegate(const VerifyArgs& args, int result);
+  void OnDefaultVerificationResult(const VerifyArgs& args, int result);
+
   Delegate* delegate_;
   scoped_ptr<net::CertVerifier> default_cert_verifier_;
 

docs/api/session.md

@@ -21,7 +21,7 @@ var session = win.webContents.session
 * `item` [DownloadItem](download-item.md)
 * `webContents` [WebContents](web-contents.md)
 
-Fired when Electron is about to download `item` in `webContents`.
+Emitted when Electron is about to download `item` in `webContents`.
 
 Calling `event.preventDefault()` will cancel the download.
 
@@ -43,18 +43,19 @@ session.on('will-download', function(event, item, webContents) {
   * `issuerName` String
 * `callback` Function
 
-Fired whenever a server certificate verification is requested by the
-network layer with `hostname`, `certificate` and `callback`.
-`callback` should be called with a boolean response to
-indicate continuation or cancellation of the request.
+Emitted when failed to verify the `certificate` for `hostname`, to trust the
+certificate you should prevent the default behavior with
+`event.preventDefault()` and call `callback(true)`.
 
 ```js
 session.on('verify-certificate', function(event, hostname, certificate, callback) {
   if (hostname == "github.com") {
-    // verification logic
+    // Verification logic.
+    event.preventDefault();
     callback(true);
+  } else {
+    callback(false);
   }
-  callback(false);
 });
 ```