Browse Source

Use object for verification request

Kevin Sawicki 8 years ago
parent
commit
70178adb6e

+ 12 - 0
atom/browser/api/atom_api_session.cc

@@ -204,6 +204,18 @@ struct Converter<net::ProxyConfig> {
   }
 };
 
+template<>
+struct Converter<atom::VerifyRequest> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   atom::VerifyRequest val) {
+    mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate);
+    dict.Set("hostname", val.hostname);
+    dict.Set("certificate", val.certificate);
+    dict.Set("verificationResult", val.default_result);
+    return dict.GetHandle();
+  }
+};
+
 }  // namespace mate
 
 namespace atom {

+ 6 - 2
atom/browser/net/atom_cert_verifier.cc

@@ -89,10 +89,14 @@ class CertVerifierRequest : public AtomCertVerifier::Request {
 
   void OnDefaultVerificationDone(int error) {
     error_ = error;
+    VerifyRequest request = {
+      params_.hostname(),
+      net::ErrorToString(error),
+      params_.certificate()
+    };
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(cert_verifier_->verify_proc(), params_.hostname(),
-                   params_.certificate(), net::ErrorToString(error),
+        base::Bind(cert_verifier_->verify_proc(), request,
                    base::Bind(&CertVerifierRequest::OnResponseInUI,
                               weak_ptr_factory_.GetWeakPtr())));
   }

+ 7 - 3
atom/browser/net/atom_cert_verifier.h

@@ -16,14 +16,18 @@ namespace atom {
 class AtomCTDelegate;
 class CertVerifierRequest;
 
+struct VerifyRequest {
+  std::string hostname;
+  std::string default_result;
+  scoped_refptr<net::X509Certificate> certificate;
+};
+
 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 std::string& default_result,
+  using VerifyProc = base::Callback<void(VerifyRequest request,
                                          const net::CompletionCallback&)>;
 
   void SetVerifyProc(const VerifyProc& proc);

+ 7 - 6
docs/api/session.md

@@ -250,9 +250,10 @@ the original network configuration.
 #### `ses.setCertificateVerifyProc(proc)`
 
 * `proc` Function
-  * `hostname` String
-  * `certificate` [Certificate](structures/certificate.md)
-  * `error` String - Verification result from chromium.
+  * `request` Object
+    * `hostname` String
+    * `certificate` [Certificate](structures/certificate.md)
+    * `error` String - Verification result from chromium.
   * `callback` Function
     * `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).
@@ -262,9 +263,9 @@ the original network configuration.
       * `-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
-verification is requested. Calling `callback(true)` accepts the certificate,
-calling `callback(false)` rejects it.
+`proc(request, callback)` whenever a server certificate
+verification is requested. Calling `callback(0)` accepts the certificate,
+calling `callback(-2)` rejects it.
 
 Calling `setCertificateVerifyProc(null)` will revert back to default certificate
 verify proc.

+ 1 - 1
docs/tutorial/planned-breaking-changes.md

@@ -99,7 +99,7 @@ ses.setCertificateVerifyProc(function (hostname, certificate, callback) {
   callback(true)
 })
 // Replace with
-ses.setCertificateVerifyProc(function (hostname, certificate, error, callback) {
+ses.setCertificateVerifyProc(function (request, callback) {
   callback(0)
 })
 ```

+ 2 - 2
lib/browser/api/session.js

@@ -22,9 +22,9 @@ Session.prototype._init = function () {
 }
 
 Session.prototype.setCertificateVerifyProc = function (verifyProc) {
-  if (verifyProc != null && verifyProc.length <= 3) {
+  if (verifyProc != null && verifyProc.length > 2) {
     // TODO(kevinsawicki): Remove in 2.0, deprecate before then with warnings
-    this._setCertificateVerifyProc((hostname, certificate, error, cb) => {
+    this._setCertificateVerifyProc(({hostname, certificate, verificationResult}, cb) => {
       verifyProc(hostname, certificate, (result) => {
         cb(result ? 0 : -2)
       })

+ 17 - 4
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, error, callback) {
-        assert.equal(error, 'net::ERR_CERT_AUTHORITY_INVALID')
+      session.defaultSession.setCertificateVerifyProc(function ({hostname, certificate, verificationResult}, callback) {
+        assert.equal(verificationResult, 'net::ERR_CERT_AUTHORITY_INVALID')
         callback(0)
       })
 
@@ -569,8 +569,21 @@ describe('session module', function () {
       w.loadURL(`https://127.0.0.1:${server.address().port}`)
     })
 
+    it('supports the old function signature', function (done) {
+      session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) {
+        assert.equal(hostname, '127.0.0.1')
+        callback(true)
+      })
+
+      w.webContents.once('did-finish-load', function () {
+        assert.equal(w.webContents.getTitle(), 'hello')
+        done()
+      })
+      w.loadURL(`https://127.0.0.1:${server.address().port}`)
+    })
+
     it('rejects the request when the callback is called with false', function (done) {
-      session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, error, callback) {
+      session.defaultSession.setCertificateVerifyProc(function ({hostname, certificate, verificationResult}, callback) {
         assert.equal(hostname, '127.0.0.1')
         assert.equal(certificate.issuerName, 'Intermediate CA')
         assert.equal(certificate.subjectName, 'localhost')
@@ -581,7 +594,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)
-        assert.equal(error, 'net::ERR_CERT_AUTHORITY_INVALID')
+        assert.equal(verificationResult, 'net::ERR_CERT_AUTHORITY_INVALID')
         callback(-2)
       })