From 0c49a57ff7f61746f0c8fa85bba4f6b30ef18b96 Mon Sep 17 00:00:00 2001
From: Ian Haken <ihaken@netflix.com>
Date: Wed, 20 Jun 2018 16:13:08 -0700
Subject: [PATCH] Simplify getting checkServerIdentity out of the fourth
 createSsl argument. Add some tests asserting type-checking behavior.

---
 .../ext/channel_credentials.cc                | 36 ++++++-------------
 .../grpc-native-core/test/credentials_test.js | 19 ++++++++++
 2 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/packages/grpc-native-core/ext/channel_credentials.cc b/packages/grpc-native-core/ext/channel_credentials.cc
index 5aaa19b8..434b6e9b 100644
--- a/packages/grpc-native-core/ext/channel_credentials.cc
+++ b/packages/grpc-native-core/ext/channel_credentials.cc
@@ -186,35 +186,21 @@ NAN_METHOD(ChannelCredentials::CreateSsl) {
   verify_peer_options verify_options = {NULL, NULL, NULL};
   if (!info[3]->IsUndefined()) {
     if (!info[3]->IsObject()) {
-      return Nan::ThrowTypeError("createSsl's fourth argument must be an "
-          "object");
+      return Nan::ThrowTypeError("createSsl's fourth argument must be an object");
     }
-    Local<Context> context = Nan::New<Context>();
     Local<Object> object = info[3]->ToObject();
-    MaybeLocal<Array> maybe_props = object->GetOwnPropertyNames(context);
-    if (!maybe_props.IsEmpty()) {
-      Local<Array> props = maybe_props.ToLocalChecked();
-      for(uint32_t i=0; i < props->Length(); i++) {
-        Local<Value> key = props->Get(i);
-        Local<Value> value = object->Get(key);
 
-        if (key->IsString()) {
-          Nan::Utf8String keyStr(key->ToString());
-
-          if (strcmp("checkServerIdentity", (const char*)(*keyStr)) == 0) {
-
-            if (!value->IsFunction()) {
-                return Nan::ThrowError("Value of checkServerIdentity must be a function.");
-            }
-            Nan::Callback *callback = new Callback(Local<Function>::Cast(value));
-            verify_options.verify_peer_callback = verify_peer_callback_wrapper;
-            verify_options.verify_peer_callback_userdata = (void*)callback;
-            verify_options.verify_peer_destruct = verify_peer_callback_destruct;
-
-          }
-        }
-        // do stuff with key / value
+    Local<Value> checkServerIdentityValue = Nan::Get(object,
+        Nan::New("checkServerIdentity").ToLocalChecked()).ToLocalChecked();
+    if (!checkServerIdentityValue->IsUndefined()) {
+      if (!checkServerIdentityValue->IsFunction()) {
+        return Nan::ThrowTypeError("Value of checkServerIdentity must be a function.");
       }
+      Nan::Callback *callback = new Callback(Local<Function>::Cast(
+        checkServerIdentityValue));
+      verify_options.verify_peer_callback = verify_peer_callback_wrapper;
+      verify_options.verify_peer_callback_userdata = (void*)callback;
+      verify_options.verify_peer_destruct = verify_peer_callback_destruct;
     }
   }
 
diff --git a/packages/grpc-native-core/test/credentials_test.js b/packages/grpc-native-core/test/credentials_test.js
index b3f922fa..542f93f3 100644
--- a/packages/grpc-native-core/test/credentials_test.js
+++ b/packages/grpc-native-core/test/credentials_test.js
@@ -128,6 +128,25 @@ describe('channel credentials', function() {
         grpc.credentials.createSsl(null, null, pem_data);
       });
     });
+    it('works if the fourth argument is an empty object', function() {
+      var creds;
+      assert.doesNotThrow(function() {
+        creds = grpc.credentials.createSsl(ca_data, null, null, {});
+      });
+      assert.notEqual(creds, null);
+    });
+    it('fails if the fourth argument is a non-object value', function() {
+      assert.throws(function() {
+        grpc.credentials.createSsl(ca_data, null, null, 'test');
+      }, TypeError);
+    });
+    it('fails if the checkServerIdentity is a non-function', function() {
+      assert.throws(function() {
+        grpc.credentials.createSsl(ca_data, null, null, {
+          "checkServerIdentity": 'test'
+        });
+      }, TypeError);
+    });
   });
 });