Merge pull request #2683 from murgatroid99/node_explicit_insecure_server

Made binding a server to a port insecurely explicit

src/node/examples/math_server.js

@@ -115,7 +115,7 @@ server.addProtoService(math.Math.service, {
 });
 
 if (require.main === module) {
-  server.bind('0.0.0.0:50051');
+  server.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure());
   server.start();
 }
 

src/node/examples/route_guide_server.js

@@ -239,7 +239,7 @@ function getServer() {
 if (require.main === module) {
   // If this is run as a script, start a server on an unused port
   var routeServer = getServer();
-  routeServer.bind('0.0.0.0:50051');
+  routeServer.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure());
   var argv = parseArgs(process.argv, {
     string: 'db_path'
   });

src/node/examples/stock_server.js

@@ -80,7 +80,7 @@ stockServer.addProtoService(examples.Stock.service, {
 });
 
 if (require.main === module) {
-  stockServer.bind('0.0.0.0:50051');
+  stockServer.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure());
   stockServer.listen();
 }
 

src/node/ext/server.cc

@@ -136,10 +136,6 @@ void Server::Init(Handle<Object> exports) {
       tpl, "addHttp2Port",
       NanNew<FunctionTemplate>(AddHttp2Port)->GetFunction());
 
-  NanSetPrototypeTemplate(
-      tpl, "addSecureHttp2Port",
-      NanNew<FunctionTemplate>(AddSecureHttp2Port)->GetFunction());
-
   NanSetPrototypeTemplate(tpl, "start",
                           NanNew<FunctionTemplate>(Start)->GetFunction());
 
@@ -246,45 +242,37 @@ NAN_METHOD(Server::RequestCall) {
 }
 
 NAN_METHOD(Server::AddHttp2Port) {
-  NanScope();
-  if (!HasInstance(args.This())) {
-    return NanThrowTypeError("addHttp2Port can only be called on a Server");
-  }
-  if (!args[0]->IsString()) {
-    return NanThrowTypeError("addHttp2Port's argument must be a String");
-  }
-  Server *server = ObjectWrap::Unwrap<Server>(args.This());
-  if (server->wrapped_server == NULL) {
-    return NanThrowError("addHttp2Port cannot be called on a shut down Server");
-  }
-  NanReturnValue(NanNew<Number>(grpc_server_add_http2_port(
-      server->wrapped_server, *NanUtf8String(args[0]))));
-}
-
-NAN_METHOD(Server::AddSecureHttp2Port) {
   NanScope();
   if (!HasInstance(args.This())) {
     return NanThrowTypeError(
-        "addSecureHttp2Port can only be called on a Server");
+        "addHttp2Port can only be called on a Server");
   }
   if (!args[0]->IsString()) {
     return NanThrowTypeError(
-        "addSecureHttp2Port's first argument must be a String");
+        "addHttp2Port's first argument must be a String");
   }
   if (!ServerCredentials::HasInstance(args[1])) {
     return NanThrowTypeError(
-        "addSecureHttp2Port's second argument must be ServerCredentials");
+        "addHttp2Port's second argument must be ServerCredentials");
   }
   Server *server = ObjectWrap::Unwrap<Server>(args.This());
   if (server->wrapped_server == NULL) {
     return NanThrowError(
-        "addSecureHttp2Port cannot be called on a shut down Server");
+        "addHttp2Port cannot be called on a shut down Server");
   }
-  ServerCredentials *creds = ObjectWrap::Unwrap<ServerCredentials>(
+  ServerCredentials *creds_object = ObjectWrap::Unwrap<ServerCredentials>(
       args[1]->ToObject());
-  NanReturnValue(NanNew<Number>(grpc_server_add_secure_http2_port(
-      server->wrapped_server, *NanUtf8String(args[0]),
-      creds->GetWrappedServerCredentials())));
+  grpc_server_credentials *creds = creds_object->GetWrappedServerCredentials();
+  int port;
+  if (creds == NULL) {
+    port = grpc_server_add_http2_port(server->wrapped_server,
+                                      *NanUtf8String(args[0]));
+  } else {
+    port = grpc_server_add_secure_http2_port(server->wrapped_server,
+                                             *NanUtf8String(args[0]),
+                                             creds);
+  }
+  NanReturnValue(NanNew<Number>(port));
 }
 
 NAN_METHOD(Server::Start) {

src/node/ext/server.h

@@ -66,7 +66,6 @@ class Server : public ::node::ObjectWrap {
   static NAN_METHOD(New);
   static NAN_METHOD(RequestCall);
   static NAN_METHOD(AddHttp2Port);
-  static NAN_METHOD(AddSecureHttp2Port);
   static NAN_METHOD(Start);
   static NAN_METHOD(Shutdown);
   static NanCallback *constructor;

src/node/ext/server_credentials.cc

@@ -73,6 +73,8 @@ void ServerCredentials::Init(Handle<Object> exports) {
   Handle<Function> ctr = tpl->GetFunction();
   ctr->Set(NanNew("createSsl"),
            NanNew<FunctionTemplate>(CreateSsl)->GetFunction());
+  ctr->Set(NanNew("createInsecure"),
+           NanNew<FunctionTemplate>(CreateInsecure)->GetFunction());
   constructor = new NanCallback(ctr);
   exports->Set(NanNew("ServerCredentials"), ctr);
 }
@@ -85,9 +87,6 @@ bool ServerCredentials::HasInstance(Handle<Value> val) {
 Handle<Value> ServerCredentials::WrapStruct(
     grpc_server_credentials *credentials) {
   NanEscapableScope();
-  if (credentials == NULL) {
-    return NanEscapeScope(NanNull());
-  }
   const int argc = 1;
   Handle<Value> argv[argc] = {
     NanNew<External>(reinterpret_cast<void *>(credentials))};
@@ -140,8 +139,17 @@ NAN_METHOD(ServerCredentials::CreateSsl) {
   key_cert_pair.cert_chain = ::node::Buffer::Data(args[2]);
   // TODO Add a force_client_auth parameter and pass it as the last parameter
   // here.
-  NanReturnValue(WrapStruct(
-      grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1, 0)));
+  grpc_server_credentials *creds =
+      grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1, 0);
+  if (creds == NULL) {
+    NanReturnNull();
+  }
+  NanReturnValue(WrapStruct(creds));
+}
+
+NAN_METHOD(ServerCredentials::CreateInsecure) {
+  NanScope();
+  NanReturnValue(WrapStruct(NULL));
 }
 
 }  // namespace node

src/node/ext/server_credentials.h

@@ -63,6 +63,7 @@ class ServerCredentials : public ::node::ObjectWrap {
 
   static NAN_METHOD(New);
   static NAN_METHOD(CreateSsl);
+  static NAN_METHOD(CreateInsecure);
   static NanCallback *constructor;
   // Used for typechecking instances of this javascript class
   static v8::Persistent<v8::FunctionTemplate> fun_tpl;

src/node/interop/interop_server.js

@@ -161,7 +161,7 @@ function handleHalfDuplex(call) {
 function getServer(port, tls) {
   // TODO(mlumish): enable TLS functionality
   var options = {};
-  var server_creds = null;
+  var server_creds;
   if (tls) {
     var key_path = path.join(__dirname, '../test/data/server1.key');
     var pem_path = path.join(__dirname, '../test/data/server1.pem');
@@ -171,6 +171,8 @@ function getServer(port, tls) {
     server_creds = grpc.ServerCredentials.createSsl(null,
                                                     key_data,
                                                     pem_data);
+  } else {
+    server_creds = grpc.ServerCredentials.createInsecure();
   }
   var server = new grpc.Server(options);
   server.addProtoService(testProto.TestService.service, {

src/node/src/server.js

@@ -714,11 +714,7 @@ Server.prototype.bind = function(port, creds) {
   if (this.started) {
     throw new Error('Can\'t bind an already running server to an address');
   }
-  if (creds) {
-    return this._server.addSecureHttp2Port(port, creds);
-  } else {
-    return this._server.addHttp2Port(port);
-  }
+  return this._server.addHttp2Port(port, creds);
 };
 
 /**

src/node/test/call_test.js

@@ -55,7 +55,8 @@ describe('call', function() {
   var server;
   before(function() {
     server = new grpc.Server();
-    var port = server.addHttp2Port('localhost:0');
+    var port = server.addHttp2Port('localhost:0',
+                                   grpc.ServerCredentials.createInsecure());
     server.start();
     channel = new grpc.Channel('localhost:' + port, insecureCreds);
   });

src/node/test/end_to_end_test.js

@@ -64,7 +64,8 @@ describe('end-to-end', function() {
   var channel;
   before(function() {
     server = new grpc.Server();
-    var port_num = server.addHttp2Port('0.0.0.0:0');
+    var port_num = server.addHttp2Port('0.0.0.0:0',
+                                       grpc.ServerCredentials.createInsecure());
     server.start();
     channel = new grpc.Channel('localhost:' + port_num, insecureCreds);
   });

src/node/test/health_test.js

@@ -54,7 +54,8 @@ describe('Health Checking', function() {
                                new health.Implementation(statusMap));
   var healthClient;
   before(function() {
-    var port_num = healthServer.bind('0.0.0.0:0');
+    var port_num = healthServer.bind('0.0.0.0:0',
+                                     grpc.ServerCredentials.createInsecure());
     healthServer.start();
     healthClient = new health.Client('localhost:' + port_num,
                                      grpc.Credentials.createInsecure());

src/node/test/math_client_test.js

@@ -51,7 +51,8 @@ var server = require('../examples/math_server.js');
 
 describe('Math client', function() {
   before(function(done) {
-    var port_num = server.bind('0.0.0.0:0');
+    var port_num = server.bind('0.0.0.0:0',
+                               grpc.ServerCredentials.createInsecure());
     server.start();
     math_client = new math.Math('localhost:' + port_num,
                                 grpc.Credentials.createInsecure());

src/node/test/server_test.js

@@ -59,16 +59,11 @@ describe('server', function() {
     it('should bind to an unused port', function() {
       var port;
       assert.doesNotThrow(function() {
-        port = server.addHttp2Port('0.0.0.0:0');
+        port = server.addHttp2Port('0.0.0.0:0',
+                                   grpc.ServerCredentials.createInsecure());
       });
       assert(port > 0);
     });
-  });
-  describe('addSecureHttp2Port', function() {
-    var server;
-    before(function() {
-      server = new grpc.Server();
-    });
     it('should bind to an unused port with ssl credentials', function() {
       var port;
       var key_path = path.join(__dirname, '../test/data/server1.key');
@@ -77,16 +72,22 @@ describe('server', function() {
       var pem_data = fs.readFileSync(pem_path);
       var creds = grpc.ServerCredentials.createSsl(null, key_data, pem_data);
       assert.doesNotThrow(function() {
-        port = server.addSecureHttp2Port('0.0.0.0:0', creds);
+        port = server.addHttp2Port('0.0.0.0:0', creds);
       });
       assert(port > 0);
     });
   });
+  describe('addSecureHttp2Port', function() {
+    var server;
+    before(function() {
+      server = new grpc.Server();
+    });
+  });
   describe('listen', function() {
     var server;
     before(function() {
       server = new grpc.Server();
-      server.addHttp2Port('0.0.0.0:0');
+      server.addHttp2Port('0.0.0.0:0', grpc.ServerCredentials.createInsecure());
     });
     after(function() {
       server.shutdown();

src/node/test/surface_test.js

@@ -47,6 +47,8 @@ var mathService = math_proto.lookup('math.Math');
 
 var _ = require('lodash');
 
+var server_insecure_creds = grpc.ServerCredentials.createInsecure();
+
 describe('File loader', function() {
   it('Should load a proto file by default', function() {
     assert.doesNotThrow(function() {
@@ -122,7 +124,7 @@ describe('Echo service', function() {
         callback(null, call.request);
       }
     });
-    var port = server.bind('localhost:0');
+    var port = server.bind('localhost:0', server_insecure_creds);
     var Client = surface_client.makeProtobufClientConstructor(echo_service);
     client = new Client('localhost:' + port, grpc.Credentials.createInsecure());
     server.start();
@@ -166,7 +168,7 @@ describe('Generic client and server', function() {
           callback(null, _.capitalize(call.request));
         }
       });
-      var port = server.bind('localhost:0');
+      var port = server.bind('localhost:0', server_insecure_creds);
       server.start();
       var Client = grpc.makeGenericClientConstructor(string_service_attrs);
       client = new Client('localhost:' + port,
@@ -215,7 +217,7 @@ describe('Echo metadata', function() {
         });
       }
     });
-    var port = server.bind('localhost:0');
+    var port = server.bind('localhost:0', server_insecure_creds);
     var Client = surface_client.makeProtobufClientConstructor(test_service);
     client = new Client('localhost:' + port, grpc.Credentials.createInsecure());
     server.start();
@@ -336,7 +338,7 @@ describe('Other conditions', function() {
         });
       }
     });
-    port = server.bind('localhost:0');
+    port = server.bind('localhost:0', server_insecure_creds);
     var Client = surface_client.makeProtobufClientConstructor(test_service);
     client = new Client('localhost:' + port, grpc.Credentials.createInsecure());
     server.start();
@@ -602,7 +604,7 @@ describe('Cancelling surface client', function() {
       'fib': function(stream) {},
       'sum': function(stream) {}
     });
-    var port = server.bind('localhost:0');
+    var port = server.bind('localhost:0', server_insecure_creds);
     var Client = surface_client.makeProtobufClientConstructor(mathService);
     client = new Client('localhost:' + port, grpc.Credentials.createInsecure());
     server.start();