modify CertVerifier Class

* respond to multiple similar verification requests.
* accept net error result as callback response.

atom/browser/api/atom_api_session.cc

@@ -738,7 +738,7 @@ void Session::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setDownloadPath", &Session::SetDownloadPath)
       .SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation)
       .SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation)
-      .SetMethod("setCertificateVerifyProc", &Session::SetCertVerifyProc)
+      .SetMethod("_setCertificateVerifyProc", &Session::SetCertVerifyProc)
       .SetMethod("setPermissionRequestHandler",
                  &Session::SetPermissionRequestHandler)
       .SetMethod("clearHostResolverCache", &Session::ClearHostResolverCache)

atom/browser/net/atom_cert_verifier.cc

@@ -7,6 +7,9 @@
 #include "atom/browser/browser.h"
 #include "atom/browser/net/atom_ct_delegate.h"
 #include "atom/common/native_mate_converters/net_converter.h"
+#include "base/containers/linked_list.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_verify_result.h"
@@ -19,17 +22,119 @@ namespace atom {
 
 namespace {
 
-void OnResult(
-    net::CertVerifyResult* verify_result,
-    const net::CompletionCallback& callback,
-    bool result) {
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(callback, result ? net::OK : net::ERR_FAILED));
-}
+class Response : public base::LinkNode<Response> {
+ public:
+  Response(net::CertVerifyResult* verify_result,
+           const net::CompletionCallback& callback)
+      : verify_result_(verify_result), callback_(callback) {}
+  net::CertVerifyResult* verify_result() { return verify_result_; }
+  net::CompletionCallback callback() { return callback_; }
+
+ private:
+  net::CertVerifyResult* verify_result_;
+  net::CompletionCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(Response);
+};
 
 }  // namespace
 
+class CertVerifierRequest : public AtomCertVerifier::Request {
+ public:
+  CertVerifierRequest(const AtomCertVerifier::RequestParams& params,
+                      AtomCertVerifier* cert_verifier)
+      : params_(params),
+        cert_verifier_(cert_verifier),
+        error_(net::ERR_IO_PENDING),
+        custom_response_(net::ERR_IO_PENDING),
+        first_response_(true),
+        weak_ptr_factory_(this) {}
+
+  ~CertVerifierRequest() {
+    cert_verifier_->RemoveRequest(params_);
+    default_verifier_request_.reset();
+    while (!response_list_.empty() && !first_response_) {
+      base::LinkNode<Response>* response_node = response_list_.head();
+      response_node->RemoveFromList();
+      Response* response = response_node->value();
+      RunResponse(response);
+    }
+    cert_verifier_ = nullptr;
+    weak_ptr_factory_.InvalidateWeakPtrs();
+  }
+
+  void RunResponse(Response* response) {
+    if (custom_response_ == net::ERR_ABORTED) {
+      *(response->verify_result()) = result_;
+      response->callback().Run(error_);
+    } else {
+      response->verify_result()->Reset();
+      response->verify_result()->verified_cert = params_.certificate();
+      cert_verifier_->ct_delegate()->AddCTExcludedHost(params_.hostname());
+      response->callback().Run(custom_response_);
+    }
+    delete response;
+  }
+
+  void Start(net::CRLSet* crl_set,
+             const net::BoundNetLog& net_log) {
+    int error = cert_verifier_->default_verifier()->Verify(
+        params_, crl_set, &result_,
+        base::Bind(&CertVerifierRequest::OnDefaultVerificationDone,
+                   weak_ptr_factory_.GetWeakPtr()),
+        &default_verifier_request_, net_log);
+    if (error != net::ERR_IO_PENDING)
+      OnDefaultVerificationDone(error);
+  }
+
+  void OnDefaultVerificationDone(int error) {
+    error_ = error;
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(cert_verifier_->verify_proc(), params_.hostname(),
+                   params_.certificate(), net::ErrorToString(error),
+                   base::Bind(&CertVerifierRequest::OnResponseInUI,
+                              weak_ptr_factory_.GetWeakPtr())));
+  }
+
+  void OnResponseInUI(int result) {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&CertVerifierRequest::NotifyResponseInIO,
+                                       weak_ptr_factory_.GetWeakPtr(), result));
+  }
+
+  void NotifyResponseInIO(int result) {
+    custom_response_ = result;
+    first_response_ = false;
+    // Responding to first request in the list will initiate destruction of
+    // the class, respond to others in the list inside destructor.
+    base::LinkNode<Response>* response_node = response_list_.head();
+    response_node->RemoveFromList();
+    Response* response = response_node->value();
+    RunResponse(response);
+  }
+
+  void AddResponseListener(net::CertVerifyResult* verify_result,
+                           const net::CompletionCallback& callback) {
+    response_list_.Append(new Response(verify_result, callback));
+  }
+
+  const AtomCertVerifier::RequestParams& params() const { return params_; }
+
+ private:
+  using ResponseList = base::LinkedList<Response>;
+
+  const AtomCertVerifier::RequestParams params_;
+  AtomCertVerifier* cert_verifier_;
+  int error_;
+  int custom_response_;
+  bool first_response_;
+  ResponseList response_list_;
+  net::CertVerifyResult result_;
+  std::unique_ptr<AtomCertVerifier::Request> default_verifier_request_;
+  base::WeakPtrFactory<CertVerifierRequest> weak_ptr_factory_;
+};
+
 AtomCertVerifier::AtomCertVerifier(AtomCTDelegate* ct_delegate)
     : default_cert_verifier_(net::CertVerifier::CreateDefault()),
       ct_delegate_(ct_delegate) {}
@@ -51,23 +156,43 @@ int AtomCertVerifier::Verify(
 
   if (verify_proc_.is_null()) {
     ct_delegate_->ClearCTExcludedHostsList();
-    return default_cert_verifier_->Verify(
-        params, crl_set, verify_result, callback, out_req, net_log);
+    return default_cert_verifier_->Verify(params, crl_set, verify_result,
+                                          callback, out_req, net_log);
+  } else {
+    CertVerifierRequest* request = FindRequest(params);
+    if (!request) {
+      out_req->reset();
+      std::unique_ptr<CertVerifierRequest> new_request =
+          base::MakeUnique<CertVerifierRequest>(params, this);
+      new_request->Start(crl_set, net_log);
+      request = new_request.get();
+      *out_req = std::move(new_request);
+      inflight_requests_[params] = request;
+    }
+    request->AddResponseListener(verify_result, callback);
+
+    return net::ERR_IO_PENDING;
   }
-
-  verify_result->Reset();
-  verify_result->verified_cert = params.certificate();
-  ct_delegate_->AddCTExcludedHost(params.hostname());
-
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(verify_proc_, params.hostname(), params.certificate(),
-                 base::Bind(OnResult, verify_result, callback)));
-  return net::ERR_IO_PENDING;
 }
 
 bool AtomCertVerifier::SupportsOCSPStapling() {
-  return true;
+  if (verify_proc_.is_null())
+    return default_cert_verifier_->SupportsOCSPStapling();
+  return false;
+}
+
+void AtomCertVerifier::RemoveRequest(const RequestParams& params) {
+  auto it = inflight_requests_.find(params);
+  if (it != inflight_requests_.end())
+    inflight_requests_.erase(it);
+}
+
+CertVerifierRequest* AtomCertVerifier::FindRequest(
+    const RequestParams& params) {
+  auto it = inflight_requests_.find(params);
+  if (it != inflight_requests_.end())
+    return it->second;
+  return nullptr;
 }
 
 }  // namespace atom

atom/browser/net/atom_cert_verifier.h

@@ -5,6 +5,7 @@
 #ifndef ATOM_BROWSER_NET_ATOM_CERT_VERIFIER_H_
 #define ATOM_BROWSER_NET_ATOM_CERT_VERIFIER_H_
 
+#include <map>
 #include <memory>
 #include <string>
 
@@ -13,19 +14,26 @@
 namespace atom {
 
 class AtomCTDelegate;
+class CertVerifierRequest;
 
 class AtomCertVerifier : public net::CertVerifier {
  public:
   explicit AtomCertVerifier(AtomCTDelegate* ct_delegate);
   virtual ~AtomCertVerifier();
 
-  using VerifyProc =
-      base::Callback<void(const std::string& hostname,
-                          scoped_refptr<net::X509Certificate>,
-                          const base::Callback<void(bool)>&)>;
+  using VerifyProc = base::Callback<void(const std::string& hostname,
+                                         scoped_refptr<net::X509Certificate>,
+                                         const std::string& default_result,
+                                         const net::CompletionCallback&)>;
 
   void SetVerifyProc(const VerifyProc& proc);
 
+  const VerifyProc verify_proc() const { return verify_proc_; }
+  AtomCTDelegate* ct_delegate() const { return ct_delegate_; }
+  net::CertVerifier* default_verifier() const {
+    return default_cert_verifier_.get();
+  }
+
  protected:
   // net::CertVerifier:
   int Verify(const RequestParams& params,
@@ -37,6 +45,12 @@ class AtomCertVerifier : public net::CertVerifier {
   bool SupportsOCSPStapling() override;
 
  private:
+  friend class CertVerifierRequest;
+
+  void RemoveRequest(const RequestParams& params);
+  CertVerifierRequest* FindRequest(const RequestParams& params);
+
+  std::map<RequestParams, CertVerifierRequest*> inflight_requests_;
   VerifyProc verify_proc_;
   std::unique_ptr<net::CertVerifier> default_cert_verifier_;
   AtomCTDelegate* ct_delegate_;

docs/api/session.md

@@ -252,8 +252,14 @@ the original network configuration.
 * `proc` Function
   * `hostname` String
   * `certificate` [Certificate](structures/certificate.md)
+  * `error` String - Verification result from chromium.
   * `callback` Function
-    * `isTrusted` Boolean - Determines if the certificate should be trusted
+    * `verificationResult` Integer - Value can be one of certificate error codes
+    from [here](https://code.google.com/p/chromium/codesearch#chromium/src/net/base/net_error_list.h).
+    Apart from the certificate error codes, the following special codes can be used.
+      * `0` - Indicates success and disables Certificate Transperancy verification.
+      * `-2` - Indicates failure.
+      * `-3` - Uses the verification result from chromium.
 
 Sets the certificate verify proc for `session`, the `proc` will be called with
 `proc(hostname, certificate, callback)` whenever a server certificate

lib/browser/api/session.js

@@ -1,5 +1,5 @@
 const {EventEmitter} = require('events')
-const {app} = require('electron')
+const {app, deprecate} = require('electron')
 const {fromPartition, Session, Cookies} = process.atomBinding('session')
 
 // Public API.
@@ -20,3 +20,22 @@ Object.setPrototypeOf(Cookies.prototype, EventEmitter.prototype)
 Session.prototype._init = function () {
   app.emit('session-created', this)
 }
+
+// Remove after 2.0
+Session.prototype.setCertificateVerifyProc = function (verifyProc) {
+  if (!verifyProc) {
+    this._setCertificateVerifyProc(null)
+    return
+  }
+  if (verifyProc.length <= 3) {
+    deprecate.warn('setCertificateVerifyproc(hostname, certificate, callback)',
+      'setCertificateVerifyproc(hostname, certificate, error, callback)')
+    this._setCertificateVerifyProc((hostname, certificate, error, cb) => {
+      verifyProc(hostname, certificate, (result) => {
+        cb(result ? 0 : -2)
+      })
+    })
+  } else {
+    this._setCertificateVerifyProc(verifyProc)
+  }
+}

lib/browser/rpc-server.js

@@ -222,6 +222,7 @@ const unwrapArgs = function (sender, args) {
             removeRemoteListenersAndLogWarning(meta, args, callIntoRenderer)
           }
         }
+        Object.defineProperty(callIntoRenderer, 'length', { value: meta.length })
 
         v8Util.setRemoteCallbackFreer(callIntoRenderer, meta.id, sender)
         rendererFunctions.set(objectId, callIntoRenderer)

lib/renderer/api/remote.js

@@ -79,7 +79,8 @@ const wrapArgs = function (args, visited) {
       return {
         type: 'function',
         id: callbacksRegistry.add(value),
-        location: v8Util.getHiddenValue(value, 'location')
+        location: v8Util.getHiddenValue(value, 'location'),
+        length: value.length
       }
     } else {
       return {

spec/api-session-spec.js

@@ -557,8 +557,8 @@ describe('session module', function () {
     })
 
     it('accepts the request when the callback is called with true', function (done) {
-      session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) {
-        callback(true)
+      session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, error, callback) {
+        callback(0)
       })
 
       w.webContents.once('did-finish-load', function () {
@@ -569,7 +569,7 @@ describe('session module', function () {
     })
 
     it('rejects the request when the callback is called with false', function (done) {
-      session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) {
+      session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, error, callback) {
         assert.equal(hostname, '127.0.0.1')
         assert.equal(certificate.issuerName, 'Intermediate CA')
         assert.equal(certificate.subjectName, 'localhost')
@@ -580,7 +580,7 @@ describe('session module', function () {
         assert.equal(certificate.issuerCert.issuerCert.issuer.commonName, 'Root CA')
         assert.equal(certificate.issuerCert.issuerCert.subject.commonName, 'Root CA')
         assert.equal(certificate.issuerCert.issuerCert.issuerCert, undefined)
-        callback(false)
+        callback(-2)
       })
 
       var url = `https://127.0.0.1:${server.address().port}`