Merge pull request grpc#1 from murgatroid99/node_secure_server_api

Updated Node library to new secure server API
tatsuhiro-t · Mar 3, 2015 · 7e0d99f · 7e0d99f
2 parents 22176cb + da02a67
commit 7e0d99f
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 34 deletions.
diff --git a/src/node/ext/server.cc b/src/node/ext/server.cc
@@ -164,19 +164,7 @@ NAN_METHOD(Server::New) {
   if (args[0]->IsUndefined()) {
     wrapped_server = grpc_server_create(queue, NULL);
   } else if (args[0]->IsObject()) {
-    grpc_server_credentials *creds = NULL;
-    Handle<Object> args_hash(args[0]->ToObject()->Clone());
-    if (args_hash->HasOwnProperty(NanNew("credentials"))) {
-      Handle<Value> creds_value = args_hash->Get(NanNew("credentials"));
-      if (!ServerCredentials::HasInstance(creds_value)) {
-        return NanThrowTypeError(
-            "credentials arg must be a ServerCredentials object");
-      }
-      ServerCredentials *creds_object =
-          ObjectWrap::Unwrap<ServerCredentials>(creds_value->ToObject());
-      creds = creds_object->GetWrappedServerCredentials();
-      args_hash->Delete(NanNew("credentials"));
-    }
+    Handle<Object> args_hash(args[0]->ToObject());
     Handle<Array> keys(args_hash->GetOwnPropertyNames());
     grpc_channel_args channel_args;
     channel_args.num_args = keys->Length();
@@ -203,11 +191,7 @@ NAN_METHOD(Server::New) {
         return NanThrowTypeError("Arg values must be strings");
       }
     }
-    if (creds == NULL) {
-      wrapped_server = grpc_server_create(queue, &channel_args);
-    } else {
-      wrapped_server = grpc_secure_server_create(creds, queue, &channel_args);
-    }
+    wrapped_server = grpc_server_create(queue, &channel_args);
     free(channel_args.args);
   } else {
     return NanThrowTypeError("Server expects an object");
@@ -258,11 +242,19 @@ NAN_METHOD(Server::AddSecureHttp2Port) {
         "addSecureHttp2Port can only be called on a Server");
   }
   if (!args[0]->IsString()) {
-    return NanThrowTypeError("addSecureHttp2Port's argument must be a String");
+    return NanThrowTypeError(
+        "addSecureHttp2Port's first argument must be a String");
+  }
+  if (!ServerCredentials::HasInstance(args[1])) {
+    return NanThrowTypeError(
+        "addSecureHttp2Port's second argument must be ServerCredentials");
   }
   Server *server = ObjectWrap::Unwrap<Server>(args.This());
+  ServerCredentials *creds = ObjectWrap::Unwrap<ServerCredentials>(
+      args[1]->ToObject());
   NanReturnValue(NanNew<Number>(grpc_server_add_secure_http2_port(
-      server->wrapped_server, *NanUtf8String(args[0]))));
+      server->wrapped_server, *NanUtf8String(args[0]),
+      creds->GetWrappedServerCredentials())));
 }
 
 NAN_METHOD(Server::Start) {

diff --git a/src/node/interop/interop_server.js b/src/node/interop/interop_server.js
@@ -165,16 +165,16 @@ function handleHalfDuplex(call) {
 function getServer(port, tls) {
   // TODO(mlumish): enable TLS functionality
   var options = {};
+  var server_creds = null;
   if (tls) {
     var key_path = path.join(__dirname, '../test/data/server1.key');
     var pem_path = path.join(__dirname, '../test/data/server1.pem');
 
     var key_data = fs.readFileSync(key_path);
     var pem_data = fs.readFileSync(pem_path);
-    var server_creds = grpc.ServerCredentials.createSsl(null,
-                                                        key_data,
-                                                        pem_data);
-    options.credentials = server_creds;
+    server_creds = grpc.ServerCredentials.createSsl(null,
+                                                    key_data,
+                                                    pem_data);
   }
   var server = new Server({
     'grpc.testing.TestService' : {
@@ -186,7 +186,7 @@ function getServer(port, tls) {
       halfDuplexCall: handleHalfDuplex
     }
   }, null, options);
-  var port_num = server.bind('0.0.0.0:' + port, tls);
+  var port_num = server.bind('0.0.0.0:' + port, server_creds);
   return {server: server, port: port_num};
 }
 

diff --git a/src/node/src/server.js b/src/node/src/server.js
@@ -517,14 +517,15 @@ Server.prototype.register = function(name, handler, serialize, deserialize,
 };
 
 /**
- * Binds the server to the given port, with SSL enabled if secure is specified
+ * Binds the server to the given port, with SSL enabled if creds is given
  * @param {string} port The port that the server should bind on, in the format
  *     "address:port"
- * @param {boolean=} secure Whether the server should open a secure port
+ * @param {boolean=} creds Server credential object to be used for SSL. Pass
+ *     nothing for an insecure port
  */
-Server.prototype.bind = function(port, secure) {
-  if (secure) {
-    return this._server.addSecureHttp2Port(port);
+Server.prototype.bind = function(port, creds) {
+  if (creds) {
+    return this._server.addSecureHttp2Port(port, creds);
   } else {
     return this._server.addHttp2Port(port);
   }
@@ -604,14 +605,14 @@ function makeServerConstructor(services) {
   }
 
   /**
-   * Binds the server to the given port, with SSL enabled if secure is specified
+   * Binds the server to the given port, with SSL enabled if creds is supplied
    * @param {string} port The port that the server should bind on, in the format
    *     "address:port"
-   * @param {boolean=} secure Whether the server should open a secure port
+   * @param {boolean=} creds Credentials to use for SSL
    * @return {SurfaceServer} this
    */
-  SurfaceServer.prototype.bind = function(port, secure) {
-    return this.inner_server.bind(port, secure);
+  SurfaceServer.prototype.bind = function(port, creds) {
+    return this.inner_server.bind(port, creds);
   };
 
   /**