Merge pull request #2679 from murgatroid99/node_explicit_insecure_channel

Make insecure channel/stub construction explicit

3 files changed, 63 insertions, 37 deletions

diff --git a/src/node/ext/channel.cc b/src/node/ext/channel.cc
index c43b55f115..d02ed95672 100644
--- a/src/node/ext/channel.cc
+++ b/src/node/ext/channel.cc
@@ -98,31 +98,30 @@ NAN_METHOD(Channel::New) {
 
   if (args.IsConstructCall()) {
     if (!args[0]->IsString()) {
-      return NanThrowTypeError("Channel expects a string and an object");
+      return NanThrowTypeError(
+          "Channel expects a string, a credential and an object");
     }
     grpc_channel *wrapped_channel;
     // Owned by the Channel object
     NanUtf8String *host = new NanUtf8String(args[0]);
     NanUtf8String *host_override = NULL;
-    if (args[1]->IsUndefined()) {
+    grpc_credentials *creds;
+    if (!Credentials::HasInstance(args[1])) {
+      return NanThrowTypeError(
+          "Channel's second argument must be a credential");
+    }
+    Credentials *creds_object = ObjectWrap::Unwrap<Credentials>(
+        args[1]->ToObject());
+    creds = creds_object->GetWrappedCredentials();
+    grpc_channel_args *channel_args_ptr;
+    if (args[2]->IsUndefined()) {
+      channel_args_ptr = NULL;
       wrapped_channel = grpc_insecure_channel_create(**host, NULL);
-    } else if (args[1]->IsObject()) {
-      grpc_credentials *creds = NULL;
-      Handle<Object> args_hash(args[1]->ToObject()->Clone());
+    } else if (args[2]->IsObject()) {
+      Handle<Object> args_hash(args[2]->ToObject()->Clone());
       if (args_hash->HasOwnProperty(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))) {
         host_override = new NanUtf8String(args_hash->Get(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG)));
       }
-      if (args_hash->HasOwnProperty(NanNew("credentials"))) {
-        Handle<Value> creds_value = args_hash->Get(NanNew("credentials"));
-        if (!Credentials::HasInstance(creds_value)) {
-          return NanThrowTypeError(
-              "credentials arg must be a Credentials object");
-        }
-        Credentials *creds_object =
-            ObjectWrap::Unwrap<Credentials>(creds_value->ToObject());
-        creds = creds_object->GetWrappedCredentials();
-        args_hash->Delete(NanNew("credentials"));
-      }
       Handle<Array> keys(args_hash->GetOwnPropertyNames());
       grpc_channel_args channel_args;
       channel_args.num_args = keys->Length();
@@ -149,16 +148,19 @@ NAN_METHOD(Channel::New) {
           return NanThrowTypeError("Arg values must be strings");
         }
       }
-      if (creds == NULL) {
-        wrapped_channel = grpc_insecure_channel_create(**host, &channel_args);
-      } else {
-        wrapped_channel =
-            grpc_secure_channel_create(creds, **host, &channel_args);
-      }
-      free(channel_args.args);
+      channel_args_ptr = &channel_args;
     } else {
       return NanThrowTypeError("Channel expects a string and an object");
     }
+    if (creds == NULL) {
+      wrapped_channel = grpc_insecure_channel_create(**host, channel_args_ptr);
+    } else {
+      wrapped_channel =
+          grpc_secure_channel_create(creds, **host, channel_args_ptr);
+    }
+    if (channel_args_ptr != NULL) {
+      free(channel_args_ptr->args);
+    }
     Channel *channel;
     if (host_override == NULL) {
       channel = new Channel(wrapped_channel, host);
@@ -168,8 +170,8 @@ NAN_METHOD(Channel::New) {
     channel->Wrap(args.This());
     NanReturnValue(args.This());
   } else {
-    const int argc = 2;
-    Local<Value> argv[argc] = {args[0], args[1]};
+    const int argc = 3;
+    Local<Value> argv[argc] = {args[0], args[1], args[2]};
     NanReturnValue(constructor->GetFunction()->NewInstance(argc, argv));
   }
 }
diff --git a/src/node/ext/credentials.cc b/src/node/ext/credentials.cc
index d6cff0631d..21d61f1a7f 100644
--- a/src/node/ext/credentials.cc
+++ b/src/node/ext/credentials.cc
@@ -81,6 +81,8 @@ void Credentials::Init(Handle<Object> exports) {
            NanNew<FunctionTemplate>(CreateGce)->GetFunction());
   ctr->Set(NanNew("createIam"),
            NanNew<FunctionTemplate>(CreateIam)->GetFunction());
+  ctr->Set(NanNew("createInsecure"),
+           NanNew<FunctionTemplate>(CreateInsecure)->GetFunction());
   constructor = new NanCallback(ctr);
   exports->Set(NanNew("Credentials"), ctr);
 }
@@ -92,9 +94,6 @@ bool Credentials::HasInstance(Handle<Value> val) {
 
 Handle<Value> Credentials::WrapStruct(grpc_credentials *credentials) {
   NanEscapableScope();
-  if (credentials == NULL) {
-    return NanEscapeScope(NanNull());
-  }
   const int argc = 1;
   Handle<Value> argv[argc] = {
     NanNew<External>(reinterpret_cast<void *>(credentials))};
@@ -128,7 +127,11 @@ NAN_METHOD(Credentials::New) {
 
 NAN_METHOD(Credentials::CreateDefault) {
   NanScope();
-  NanReturnValue(WrapStruct(grpc_google_default_credentials_create()));
+  grpc_credentials *creds = grpc_google_default_credentials_create();
+  if (creds == NULL) {
+    NanReturnNull();
+  }
+  NanReturnValue(WrapStruct(creds));
 }
 
 NAN_METHOD(Credentials::CreateSsl) {
@@ -152,9 +155,12 @@ NAN_METHOD(Credentials::CreateSsl) {
     return NanThrowTypeError(
         "createSSl's third argument must be a Buffer if provided");
   }
-
-  NanReturnValue(WrapStruct(grpc_ssl_credentials_create(
-      root_certs, key_cert_pair.private_key == NULL ? NULL : &key_cert_pair)));
+  grpc_credentials *creds = grpc_ssl_credentials_create(
+      root_certs, key_cert_pair.private_key == NULL ? NULL : &key_cert_pair);
+  if (creds == NULL) {
+    NanReturnNull();
+  }
+  NanReturnValue(WrapStruct(creds));
 }
 
 NAN_METHOD(Credentials::CreateComposite) {
@@ -169,13 +175,21 @@ NAN_METHOD(Credentials::CreateComposite) {
   }
   Credentials *creds1 = ObjectWrap::Unwrap<Credentials>(args[0]->ToObject());
   Credentials *creds2 = ObjectWrap::Unwrap<Credentials>(args[1]->ToObject());
-  NanReturnValue(WrapStruct(grpc_composite_credentials_create(
-      creds1->wrapped_credentials, creds2->wrapped_credentials)));
+  grpc_credentials *creds = grpc_composite_credentials_create(
+      creds1->wrapped_credentials, creds2->wrapped_credentials);
+  if (creds == NULL) {
+    NanReturnNull();
+  }
+  NanReturnValue(WrapStruct(creds));
 }
 
 NAN_METHOD(Credentials::CreateGce) {
   NanScope();
-  NanReturnValue(WrapStruct(grpc_compute_engine_credentials_create()));
+  grpc_credentials *creds = grpc_compute_engine_credentials_create();
+  if (creds == NULL) {
+    NanReturnNull();
+  }
+  NanReturnValue(WrapStruct(creds));
 }
 
 NAN_METHOD(Credentials::CreateIam) {
@@ -188,8 +202,17 @@ NAN_METHOD(Credentials::CreateIam) {
   }
   NanUtf8String auth_token(args[0]);
   NanUtf8String auth_selector(args[1]);
-  NanReturnValue(
-      WrapStruct(grpc_iam_credentials_create(*auth_token, *auth_selector)));
+  grpc_credentials *creds = grpc_iam_credentials_create(*auth_token,
+                                                       *auth_selector);
+  if (creds == NULL) {
+    NanReturnNull();
+  }
+  NanReturnValue(WrapStruct(creds));
+}
+
+NAN_METHOD(Credentials::CreateInsecure) {
+  NanScope();
+  NanReturnValue(WrapStruct(NULL));
 }
 
 }  // namespace node
diff --git a/src/node/ext/credentials.h b/src/node/ext/credentials.h
index 794736fe1a..62957e61c3 100644
--- a/src/node/ext/credentials.h
+++ b/src/node/ext/credentials.h
@@ -68,6 +68,7 @@ class Credentials : public ::node::ObjectWrap {
   static NAN_METHOD(CreateGce);
   static NAN_METHOD(CreateFake);
   static NAN_METHOD(CreateIam);
+  static NAN_METHOD(CreateInsecure);
   static NanCallback *constructor;
   // Used for typechecking instances of this javascript class
   static v8::Persistent<v8::FunctionTemplate> fun_tpl;