From 77d3141bf8785b626f359873e62108f91e2b2607 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 14:16:44 -0700 Subject: [PATCH 01/17] Added explicit insecure credentials constructors --- ext/credentials.cc | 47 ++++++++++++++++++++++++++++++++++------------ ext/credentials.h | 1 + 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/ext/credentials.cc b/ext/credentials.cc index 34872017..23fe20ed 100644 --- a/ext/credentials.cc +++ b/ext/credentials.cc @@ -83,6 +83,8 @@ void Credentials::Init(Handle exports) { NanNew(CreateFake)->GetFunction()); ctr->Set(NanNew("createIam"), NanNew(CreateIam)->GetFunction()); + ctr->Set(NanNew("createInsecure"), + NanNew(CreateIam)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("Credentials"), ctr); } @@ -94,9 +96,6 @@ bool Credentials::HasInstance(Handle val) { Handle Credentials::WrapStruct(grpc_credentials *credentials) { NanEscapableScope(); - if (credentials == NULL) { - return NanEscapeScope(NanNull()); - } const int argc = 1; Handle argv[argc] = { NanNew(reinterpret_cast(credentials))}; @@ -130,7 +129,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) { @@ -154,9 +157,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) { @@ -171,13 +177,21 @@ NAN_METHOD(Credentials::CreateComposite) { } Credentials *creds1 = ObjectWrap::Unwrap(args[0]->ToObject()); Credentials *creds2 = ObjectWrap::Unwrap(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::CreateFake) { @@ -195,8 +209,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_credentisl *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/ext/credentials.h b/ext/credentials.h index 794736fe..62957e61 100644 --- a/ext/credentials.h +++ b/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 fun_tpl; From 954538063c547210cb87a2a28c26bac1de37e99b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 14:56:40 -0700 Subject: [PATCH 02/17] Made credentials an explicit required argument to channels --- examples/perf_test.js | 3 +- examples/qps_test.js | 3 +- examples/route_guide_client.js | 3 +- examples/stock_client.js | 3 +- ext/channel.cc | 52 ++++++++++++++++++---------------- ext/credentials.cc | 4 +-- interop/interop_client.js | 8 ++++-- src/client.js | 6 ++-- test/call_test.js | 6 ++-- test/channel_test.js | 30 +++++++++++++------- test/end_to_end_test.js | 4 ++- test/health_test.js | 3 +- test/math_client_test.js | 3 +- test/surface_test.js | 14 +++++---- 14 files changed, 85 insertions(+), 57 deletions(-) diff --git a/examples/perf_test.js b/examples/perf_test.js index da919ece..0f38725f 100644 --- a/examples/perf_test.js +++ b/examples/perf_test.js @@ -41,7 +41,8 @@ var interop_server = require('../interop/interop_server.js'); function runTest(iterations, callback) { var testServer = interop_server.getServer(0, false); testServer.server.listen(); - var client = new testProto.TestService('localhost:' + testServer.port); + var client = new testProto.TestService('localhost:' + testServer.port, + grpc.Credentials.createInsecure()); function runIterations(finish) { var start = process.hrtime(); diff --git a/examples/qps_test.js b/examples/qps_test.js index 00293b46..1ce4dbe0 100644 --- a/examples/qps_test.js +++ b/examples/qps_test.js @@ -61,7 +61,8 @@ var interop_server = require('../interop/interop_server.js'); function runTest(concurrent_calls, seconds, callback) { var testServer = interop_server.getServer(0, false); testServer.server.listen(); - var client = new testProto.TestService('localhost:' + testServer.port); + var client = new testProto.TestService('localhost:' + testServer.port, + grpc.Credentials.createInsecure()); var warmup_num = 100; diff --git a/examples/route_guide_client.js b/examples/route_guide_client.js index 8cd532fe..647f3ffa 100644 --- a/examples/route_guide_client.js +++ b/examples/route_guide_client.js @@ -40,7 +40,8 @@ var path = require('path'); var _ = require('lodash'); var grpc = require('..'); var examples = grpc.load(__dirname + '/route_guide.proto').examples; -var client = new examples.RouteGuide('localhost:50051'); +var client = new examples.RouteGuide('localhost:50051', + grpc.Credentials.createInsecure()); var COORD_FACTOR = 1e7; diff --git a/examples/stock_client.js b/examples/stock_client.js index b37e66df..ab9b050e 100644 --- a/examples/stock_client.js +++ b/examples/stock_client.js @@ -38,7 +38,8 @@ var examples = grpc.load(__dirname + '/stock.proto').examples; * This exports a client constructor for the Stock service. The usage looks like * * var StockClient = require('stock_client.js'); - * var stockClient = new StockClient(server_address); + * var stockClient = new StockClient(server_address, + * grpc.Credentials.createInsecure()); * stockClient.getLastTradePrice({symbol: 'GOOG'}, function(error, response) { * console.log(error || response); * }); diff --git a/ext/channel.cc b/ext/channel.cc index c43b55f1..d02ed956 100644 --- a/ext/channel.cc +++ b/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( + 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 args_hash(args[1]->ToObject()->Clone()); + } else if (args[2]->IsObject()) { + Handle 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 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(creds_value->ToObject()); - creds = creds_object->GetWrappedCredentials(); - args_hash->Delete(NanNew("credentials")); - } Handle 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 argv[argc] = {args[0], args[1]}; + const int argc = 3; + Local argv[argc] = {args[0], args[1], args[2]}; NanReturnValue(constructor->GetFunction()->NewInstance(argc, argv)); } } diff --git a/ext/credentials.cc b/ext/credentials.cc index a695e365..21d61f1a 100644 --- a/ext/credentials.cc +++ b/ext/credentials.cc @@ -82,7 +82,7 @@ void Credentials::Init(Handle exports) { ctr->Set(NanNew("createIam"), NanNew(CreateIam)->GetFunction()); ctr->Set(NanNew("createInsecure"), - NanNew(CreateIam)->GetFunction()); + NanNew(CreateInsecure)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("Credentials"), ctr); } @@ -202,7 +202,7 @@ NAN_METHOD(Credentials::CreateIam) { } NanUtf8String auth_token(args[0]); NanUtf8String auth_selector(args[1]); - grpc_credentisl *creds = grpc_iam_credentials_create(*auth_token, + grpc_credentials *creds = grpc_iam_credentials_create(*auth_token, *auth_selector); if (creds == NULL) { NanReturnNull(); diff --git a/interop/interop_client.js b/interop/interop_client.js index e810e68e..236b3661 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -397,6 +397,7 @@ var test_cases = { function runTest(address, host_override, test_case, tls, test_ca, done) { // TODO(mlumish): enable TLS functionality var options = {}; + var creds; if (tls) { var ca_path; if (test_ca) { @@ -405,13 +406,14 @@ function runTest(address, host_override, test_case, tls, test_ca, done) { ca_path = process.env.SSL_CERT_FILE; } var ca_data = fs.readFileSync(ca_path); - var creds = grpc.Credentials.createSsl(ca_data); - options.credentials = creds; + creds = grpc.Credentials.createSsl(ca_data); if (host_override) { options['grpc.ssl_target_name_override'] = host_override; } + } else { + creds = grpc.Credentials.createInsecure(); } - var client = new testProto.TestService(address, options); + var client = new testProto.TestService(address, creds, options); test_cases[test_case](client, done); } diff --git a/src/client.js b/src/client.js index d89c656c..50338a6e 100644 --- a/src/client.js +++ b/src/client.js @@ -524,11 +524,13 @@ function makeClientConstructor(methods, serviceName) { * Create a client with the given methods * @constructor * @param {string} address The address of the server to connect to + * @param {grpc.Credentials} credentials Credentials to use to connect + * to the server * @param {Object} options Options to pass to the underlying channel * @param {function(string, Object, function)=} updateMetadata function to * update the metadata for each request */ - function Client(address, options, updateMetadata) { + function Client(address, credentials, options, updateMetadata) { if (!updateMetadata) { updateMetadata = function(uri, metadata, callback) { callback(null, metadata); @@ -538,7 +540,7 @@ function makeClientConstructor(methods, serviceName) { options = {}; } options['grpc.primary_user_agent'] = 'grpc-node/' + version; - this.channel = new grpc.Channel(address, options); + this.channel = new grpc.Channel(address, credentials, options); this.server_address = address.replace(/\/$/, ''); this.auth_uri = this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; diff --git a/test/call_test.js b/test/call_test.js index 942c31ac..c5edab8b 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -48,6 +48,8 @@ function getDeadline(timeout_secs) { return deadline; } +var insecureCreds = grpc.Credentials.createInsecure(); + describe('call', function() { var channel; var server; @@ -55,7 +57,7 @@ describe('call', function() { server = new grpc.Server(); var port = server.addHttp2Port('localhost:0'); server.start(); - channel = new grpc.Channel('localhost:' + port); + channel = new grpc.Channel('localhost:' + port, insecureCreds); }); after(function() { server.shutdown(); @@ -82,7 +84,7 @@ describe('call', function() { }); }); it('should fail with a closed channel', function() { - var local_channel = new grpc.Channel('hostname'); + var local_channel = new grpc.Channel('hostname', insecureCreds); local_channel.close(); assert.throws(function() { new grpc.Call(channel, 'method'); diff --git a/test/channel_test.js b/test/channel_test.js index 3e61d3bb..3cb43334 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -36,11 +36,13 @@ var assert = require('assert'); var grpc = require('bindings')('grpc.node'); +var insecureCreds = grpc.Credentials.createInsecure(); + describe('channel', function() { describe('constructor', function() { it('should require a string for the first argument', function() { assert.doesNotThrow(function() { - new grpc.Channel('hostname'); + new grpc.Channel('hostname', insecureCreds); }); assert.throws(function() { new grpc.Channel(); @@ -49,38 +51,46 @@ describe('channel', function() { new grpc.Channel(5); }); }); - it('should accept an object for the second parameter', function() { + it('should accept a credential for the second argument', function() { assert.doesNotThrow(function() { - new grpc.Channel('hostname', {}); + new grpc.Channel('hostname', insecureCreds); }); assert.throws(function() { new grpc.Channel('hostname', 5); }); }); + it('should accept an object for the third argument', function() { + assert.doesNotThrow(function() { + new grpc.Channel('hostname', insecureCreds, {}); + }); + assert.throws(function() { + new grpc.Channel('hostname', insecureCreds, 'abc'); + }); + }); it('should only accept objects with string or int values', function() { assert.doesNotThrow(function() { - new grpc.Channel('hostname', {'key' : 'value'}); + new grpc.Channel('hostname', insecureCreds,{'key' : 'value'}); }); assert.doesNotThrow(function() { - new grpc.Channel('hostname', {'key' : 5}); + new grpc.Channel('hostname', insecureCreds, {'key' : 5}); }); assert.throws(function() { - new grpc.Channel('hostname', {'key' : null}); + new grpc.Channel('hostname', insecureCreds, {'key' : null}); }); assert.throws(function() { - new grpc.Channel('hostname', {'key' : new Date()}); + new grpc.Channel('hostname', insecureCreds, {'key' : new Date()}); }); }); }); describe('close', function() { it('should succeed silently', function() { - var channel = new grpc.Channel('hostname', {}); + var channel = new grpc.Channel('hostname', insecureCreds, {}); assert.doesNotThrow(function() { channel.close(); }); }); it('should be idempotent', function() { - var channel = new grpc.Channel('hostname', {}); + var channel = new grpc.Channel('hostname', insecureCreds, {}); assert.doesNotThrow(function() { channel.close(); channel.close(); @@ -89,7 +99,7 @@ describe('channel', function() { }); describe('getTarget', function() { it('should return a string', function() { - var channel = new grpc.Channel('localhost', {}); + var channel = new grpc.Channel('localhost', insecureCreds, {}); assert.strictEqual(typeof channel.getTarget(), 'string'); }); }); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index 5d3baf82..9133ceca 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -57,6 +57,8 @@ function multiDone(done, count) { }; } +var insecureCreds = grpc.Credentials.createInsecure(); + describe('end-to-end', function() { var server; var channel; @@ -64,7 +66,7 @@ describe('end-to-end', function() { server = new grpc.Server(); var port_num = server.addHttp2Port('0.0.0.0:0'); server.start(); - channel = new grpc.Channel('localhost:' + port_num); + channel = new grpc.Channel('localhost:' + port_num, insecureCreds); }); after(function() { server.shutdown(); diff --git a/test/health_test.js b/test/health_test.js index bb700cc4..93b068be 100644 --- a/test/health_test.js +++ b/test/health_test.js @@ -56,7 +56,8 @@ describe('Health Checking', function() { before(function() { var port_num = healthServer.bind('0.0.0.0:0'); healthServer.start(); - healthClient = new health.Client('localhost:' + port_num); + healthClient = new health.Client('localhost:' + port_num, + grpc.Credentials.createInsecure()); }); after(function() { healthServer.shutdown(); diff --git a/test/math_client_test.js b/test/math_client_test.js index f2751857..1bd85ca4 100644 --- a/test/math_client_test.js +++ b/test/math_client_test.js @@ -53,7 +53,8 @@ describe('Math client', function() { before(function(done) { var port_num = server.bind('0.0.0.0:0'); server.start(); - math_client = new math.Math('localhost:' + port_num); + math_client = new math.Math('localhost:' + port_num, + grpc.Credentials.createInsecure()); done(); }); after(function() { diff --git a/test/surface_test.js b/test/surface_test.js index 9005cbd5..d63f48d6 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -124,7 +124,7 @@ describe('Echo service', function() { }); var port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(echo_service); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { @@ -169,7 +169,8 @@ describe('Generic client and server', function() { var port = server.bind('localhost:0'); server.start(); var Client = grpc.makeGenericClientConstructor(string_service_attrs); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, + grpc.Credentials.createInsecure()); }); after(function() { server.shutdown(); @@ -216,7 +217,7 @@ describe('Echo metadata', function() { }); var port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(test_service); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { @@ -338,7 +339,7 @@ describe('Other conditions', function() { }); port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(test_service); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { @@ -383,7 +384,8 @@ describe('Other conditions', function() { }; var Client = surface_client.makeClientConstructor(test_service_attrs, 'TestService'); - misbehavingClient = new Client('localhost:' + port); + misbehavingClient = new Client('localhost:' + port, + grpc.Credentials.createInsecure()); }); it('should respond correctly to a unary call', function(done) { misbehavingClient.unary(badArg, function(err, data) { @@ -603,7 +605,7 @@ describe('Cancelling surface client', function() { }); var port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(mathService); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { From eda1e5fb016ea0098c3ccb59253aa9a5336a861c Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 15:54:13 -0700 Subject: [PATCH 03/17] Added test that credential channel argument is required --- test/channel_test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/channel_test.js b/test/channel_test.js index 3cb43334..c991d7b2 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -51,13 +51,16 @@ describe('channel', function() { new grpc.Channel(5); }); }); - it('should accept a credential for the second argument', function() { + it('should require a credential for the second argument', function() { assert.doesNotThrow(function() { new grpc.Channel('hostname', insecureCreds); }); assert.throws(function() { new grpc.Channel('hostname', 5); }); + assert.throws(function() { + new grpc.Channel('hostname'); + }); }); it('should accept an object for the third argument', function() { assert.doesNotThrow(function() { From d03da0cd75a1c223402ded660cf1cd533ac857da Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 16:13:28 -0700 Subject: [PATCH 04/17] Made binding a server to a port insecurely explicit --- examples/math_server.js | 2 +- examples/route_guide_server.js | 2 +- examples/stock_server.js | 2 +- ext/server.cc | 44 +++++++++++++--------------------- ext/server.h | 1 - ext/server_credentials.cc | 18 ++++++++++---- ext/server_credentials.h | 1 + interop/interop_server.js | 4 +++- src/server.js | 6 +---- test/call_test.js | 3 ++- test/end_to_end_test.js | 3 ++- test/health_test.js | 3 ++- test/math_client_test.js | 3 ++- test/server_test.js | 19 ++++++++------- test/surface_test.js | 12 ++++++---- 15 files changed, 62 insertions(+), 61 deletions(-) diff --git a/examples/math_server.js b/examples/math_server.js index b1f8a632..31892c65 100644 --- a/examples/math_server.js +++ b/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(); } diff --git a/examples/route_guide_server.js b/examples/route_guide_server.js index 70044a32..bb8e79b5 100644 --- a/examples/route_guide_server.js +++ b/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' }); diff --git a/examples/stock_server.js b/examples/stock_server.js index f2eb6ad4..dfcfe30e 100644 --- a/examples/stock_server.js +++ b/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(); } diff --git a/ext/server.cc b/ext/server.cc index 8554fce7..04fabc87 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -136,10 +136,6 @@ void Server::Init(Handle exports) { tpl, "addHttp2Port", NanNew(AddHttp2Port)->GetFunction()); - NanSetPrototypeTemplate( - tpl, "addSecureHttp2Port", - NanNew(AddSecureHttp2Port)->GetFunction()); - NanSetPrototypeTemplate(tpl, "start", NanNew(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(args.This()); - if (server->wrapped_server == NULL) { - return NanThrowError("addHttp2Port cannot be called on a shut down Server"); - } - NanReturnValue(NanNew(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(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 *creds_object = ObjectWrap::Unwrap( args[1]->ToObject()); - NanReturnValue(NanNew(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(port)); } NAN_METHOD(Server::Start) { diff --git a/ext/server.h b/ext/server.h index 5b4b18a0..faab7e34 100644 --- a/ext/server.h +++ b/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; diff --git a/ext/server_credentials.cc b/ext/server_credentials.cc index 66aaa330..51cdbcde 100644 --- a/ext/server_credentials.cc +++ b/ext/server_credentials.cc @@ -73,6 +73,8 @@ void ServerCredentials::Init(Handle exports) { Handle ctr = tpl->GetFunction(); ctr->Set(NanNew("createSsl"), NanNew(CreateSsl)->GetFunction()); + ctr->Set(NanNew("createInsecure"), + NanNew(CreateInsecure)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("ServerCredentials"), ctr); } @@ -85,9 +87,6 @@ bool ServerCredentials::HasInstance(Handle val) { Handle ServerCredentials::WrapStruct( grpc_server_credentials *credentials) { NanEscapableScope(); - if (credentials == NULL) { - return NanEscapeScope(NanNull()); - } const int argc = 1; Handle argv[argc] = { NanNew(reinterpret_cast(credentials))}; @@ -138,8 +137,17 @@ NAN_METHOD(ServerCredentials::CreateSsl) { return NanThrowTypeError("createSsl's third argument must be a Buffer"); } key_cert_pair.cert_chain = ::node::Buffer::Data(args[2]); - NanReturnValue(WrapStruct( - grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1))); + grpc_server_credentials *creds = + grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); +} + +NAN_METHOD(ServerCredentials::CreateInsecure) { + NanScope(); + NanReturnValue(WrapStruct(NULL)); } } // namespace node diff --git a/ext/server_credentials.h b/ext/server_credentials.h index 80747504..63903f66 100644 --- a/ext/server_credentials.h +++ b/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 fun_tpl; diff --git a/interop/interop_server.js b/interop/interop_server.js index 505c6bb5..ece22cce 100644 --- a/interop/interop_server.js +++ b/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, { diff --git a/src/server.js b/src/server.js index e876313d..fac013f4 100644 --- a/src/server.js +++ b/src/server.js @@ -673,11 +673,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); }; /** diff --git a/test/call_test.js b/test/call_test.js index 942c31ac..4f183949 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -53,7 +53,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); }); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index 5d3baf82..bb8ad625 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -62,7 +62,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); }); diff --git a/test/health_test.js b/test/health_test.js index bb700cc4..fa23dc3e 100644 --- a/test/health_test.js +++ b/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); }); diff --git a/test/math_client_test.js b/test/math_client_test.js index f2751857..567faf9c 100644 --- a/test/math_client_test.js +++ b/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); done(); diff --git a/test/server_test.js b/test/server_test.js index 9c7bb465..a9df4390 100644 --- a/test/server_test.js +++ b/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(); diff --git a/test/surface_test.js b/test/surface_test.js index 9005cbd5..fd326e44 100644 --- a/test/surface_test.js +++ b/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); 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); @@ -214,7 +216,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); 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); server.start(); @@ -601,7 +603,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); server.start(); From 288bdb4eabdea544b9f264e4f6a8e9009b56cffd Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 10:09:36 -0700 Subject: [PATCH 05/17] Ensure that client generated methods don't conflict with other properties --- src/client.js | 27 +++++++++++++++------------ test/surface_test.js | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/client.js b/src/client.js index f843669b..405e2be6 100644 --- a/src/client.js +++ b/src/client.js @@ -236,7 +236,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { deadline = Infinity; } var emitter = new EventEmitter(); - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -246,7 +246,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { emitter.getPeer = function getPeer() { return call.getPeer(); }; - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -309,12 +309,12 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientWritableStream(call, serialize); - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -383,12 +383,12 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientReadableStream(call, deserialize); - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -455,12 +455,12 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientDuplexStream(call, serialize, deserialize); - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -545,14 +545,17 @@ exports.makeClientConstructor = function(methods, serviceName) { options = {}; } options['grpc.primary_user_agent'] = 'grpc-node/' + version; - this.channel = new grpc.Channel(address, options); - this.server_address = address.replace(/\/$/, ''); - this.auth_uri = this.server_address + '/' + serviceName; - this.updateMetadata = updateMetadata; + this.$channel = new grpc.Channel(address, options); + this.$server_address = address.replace(/\/$/, ''); + this.$auth_uri = this.$server_address + '/' + serviceName; + this.$updateMetadata = updateMetadata; } _.each(methods, function(attrs, name) { var method_type; + if (_.startsWith(name, '$')) { + throw new Error('Method names cannot start with $'); + } if (attrs.requestStream) { if (attrs.responseStream) { method_type = 'bidi'; diff --git a/test/surface_test.js b/test/surface_test.js index 98f9b15b..1afdb330 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -110,6 +110,24 @@ describe('Server.prototype.addProtoService', function() { }); }); }); +describe('Client constructor building', function() { + var illegal_service_attrs = { + $method : { + path: '/illegal/$method', + requestStream: false, + responseStream: false, + requestSerialize: _.identity, + requestDeserialize: _.identity, + responseSerialize: _.identity, + responseDeserialize: _.identity + } + }; + it('Should reject method names starting with $', function() { + assert.throws(function() { + grpc.makeGenericClientConstructor(illegal_service_attrs); + }, /\$/); + }); +}); describe('Echo service', function() { var server; var client; @@ -344,7 +362,7 @@ describe('Other conditions', function() { server.shutdown(); }); it('channel.getTarget should be available', function() { - assert.strictEqual(typeof client.channel.getTarget(), 'string'); + assert.strictEqual(typeof client.$channel.getTarget(), 'string'); }); describe('Server recieving bad input', function() { var misbehavingClient; From c451627d3620713661d97c39311be4888a3ff865 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 10:11:07 -0700 Subject: [PATCH 06/17] Revert "Ensure that client generated methods don't conflict with other properties" This reverts commit 6f34bad568e6db5f78a908fa63bd08a1304366fa. --- src/client.js | 27 ++++++++++++--------------- test/surface_test.js | 20 +------------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/client.js b/src/client.js index 405e2be6..f843669b 100644 --- a/src/client.js +++ b/src/client.js @@ -236,7 +236,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { deadline = Infinity; } var emitter = new EventEmitter(); - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -246,7 +246,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { emitter.getPeer = function getPeer() { return call.getPeer(); }; - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -309,12 +309,12 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientWritableStream(call, serialize); - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -383,12 +383,12 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientReadableStream(call, deserialize); - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -455,12 +455,12 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientDuplexStream(call, serialize, deserialize); - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -545,17 +545,14 @@ exports.makeClientConstructor = function(methods, serviceName) { options = {}; } options['grpc.primary_user_agent'] = 'grpc-node/' + version; - this.$channel = new grpc.Channel(address, options); - this.$server_address = address.replace(/\/$/, ''); - this.$auth_uri = this.$server_address + '/' + serviceName; - this.$updateMetadata = updateMetadata; + this.channel = new grpc.Channel(address, options); + this.server_address = address.replace(/\/$/, ''); + this.auth_uri = this.server_address + '/' + serviceName; + this.updateMetadata = updateMetadata; } _.each(methods, function(attrs, name) { var method_type; - if (_.startsWith(name, '$')) { - throw new Error('Method names cannot start with $'); - } if (attrs.requestStream) { if (attrs.responseStream) { method_type = 'bidi'; diff --git a/test/surface_test.js b/test/surface_test.js index 1afdb330..98f9b15b 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -110,24 +110,6 @@ describe('Server.prototype.addProtoService', function() { }); }); }); -describe('Client constructor building', function() { - var illegal_service_attrs = { - $method : { - path: '/illegal/$method', - requestStream: false, - responseStream: false, - requestSerialize: _.identity, - requestDeserialize: _.identity, - responseSerialize: _.identity, - responseDeserialize: _.identity - } - }; - it('Should reject method names starting with $', function() { - assert.throws(function() { - grpc.makeGenericClientConstructor(illegal_service_attrs); - }, /\$/); - }); -}); describe('Echo service', function() { var server; var client; @@ -362,7 +344,7 @@ describe('Other conditions', function() { server.shutdown(); }); it('channel.getTarget should be available', function() { - assert.strictEqual(typeof client.$channel.getTarget(), 'string'); + assert.strictEqual(typeof client.channel.getTarget(), 'string'); }); describe('Server recieving bad input', function() { var misbehavingClient; From 46d1b4143c1cc41c1c46ff3d46724e7c1a6912a9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 16:33:52 -0700 Subject: [PATCH 07/17] Eliminated some redundant checks in the Node interop client --- interop/interop_client.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/interop/interop_client.js b/interop/interop_client.js index e810e68e..d2f51296 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -69,9 +69,6 @@ function zeroBuffer(size) { function emptyUnary(client, done) { var call = client.emptyCall({}, function(err, resp) { assert.ifError(err); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -96,9 +93,6 @@ function largeUnary(client, done) { assert.ifError(err); assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -115,9 +109,6 @@ function clientStreaming(client, done) { var call = client.streamingInputCall(function(err, resp) { assert.ifError(err); assert.strictEqual(resp.aggregated_payload_size, 74922); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -308,9 +299,6 @@ function authTest(expected_user, scope, client, done) { assert.strictEqual(resp.payload.body.length, 314159); assert.strictEqual(resp.username, expected_user); assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -344,9 +332,6 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { assert.ifError(err); assert.strictEqual(resp.username, expected_user); assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -358,7 +343,6 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { client.updateMetadata = updateMetadata; makeTestCall(null, {}); } - }); }); } From eb6b0ede34c523212d2198c0ec6d205f7fb54c54 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Jul 2015 17:01:47 -0700 Subject: [PATCH 08/17] Update wrappers, tests to new create_call() --- ext/call.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index dc45c8d8..e4451c36 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -511,8 +511,9 @@ NAN_METHOD(Call::New) { double deadline = args[2]->NumberValue(); grpc_channel *wrapped_channel = channel->GetWrappedChannel(); grpc_call *wrapped_call = grpc_channel_create_call( - wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, - channel->GetHost(), MillisecondsToTimespec(deadline)); + wrapped_channel, NULL, GRPC_INHERIT_DEFAULTS, + CompletionQueueAsyncWorker::GetQueue(), *method, channel->GetHost(), + MillisecondsToTimespec(deadline)); call = new Call(wrapped_call); args.This()->SetHiddenValue(NanNew("channel_"), channel_object); } From 70c8bfd09e3adc67043c00f99ada83a6ac9b9a0a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 3 Aug 2015 08:06:50 -0700 Subject: [PATCH 09/17] Implement cancellation propagation, define auth propagation --- ext/call.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/call.cc b/ext/call.cc index e4451c36..fe585a0d 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -511,7 +511,7 @@ NAN_METHOD(Call::New) { double deadline = args[2]->NumberValue(); grpc_channel *wrapped_channel = channel->GetWrappedChannel(); grpc_call *wrapped_call = grpc_channel_create_call( - wrapped_channel, NULL, GRPC_INHERIT_DEFAULTS, + wrapped_channel, NULL, GRPC_PROPAGATE_DEFAULTS, CompletionQueueAsyncWorker::GetQueue(), *method, channel->GetHost(), MillisecondsToTimespec(deadline)); call = new Call(wrapped_call); From 1451651e8c3716ea2c50ea46499f5400cd378b40 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 3 Aug 2015 10:42:22 -0700 Subject: [PATCH 10/17] Rename grpc_server_add_http2_port to grpc_server_add_insecure_http2_port --- ext/server.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/server.cc b/ext/server.cc index 04fabc87..1dc179db 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -265,8 +265,8 @@ NAN_METHOD(Server::AddHttp2Port) { grpc_server_credentials *creds = creds_object->GetWrappedServerCredentials(); int port; if (creds == NULL) { - port = grpc_server_add_http2_port(server->wrapped_server, - *NanUtf8String(args[0])); + port = grpc_server_add_insecure_http2_port(server->wrapped_server, + *NanUtf8String(args[0])); } else { port = grpc_server_add_secure_http2_port(server->wrapped_server, *NanUtf8String(args[0]), From 8551cd9559b48f120a6144aa3b932956f86abf92 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 3 Aug 2015 10:56:00 -0700 Subject: [PATCH 11/17] Disabled deprecation warnings in Node build --- binding.gyp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/binding.gyp b/binding.gyp index 6ba23338..734dc841 100644 --- a/binding.gyp +++ b/binding.gyp @@ -11,7 +11,8 @@ '-pedantic', '-g', '-zdefs', - '-Werror' + '-Werror', + '-Wno-error=deprecated-declarations' ], 'ldflags': [ '-g' From ad3334967aa9ca57e271b117c2c7dcf73ac34ba7 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 3 Aug 2015 15:17:53 -0700 Subject: [PATCH 12/17] Exposed host parameter in Call constructor, don't save it in Channel object --- ext/call.cc | 14 ++++++++++++-- ext/channel.cc | 25 ++++++------------------- ext/channel.h | 6 +----- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index dc45c8d8..ab177e85 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -510,9 +510,19 @@ NAN_METHOD(Call::New) { NanUtf8String method(args[1]); double deadline = args[2]->NumberValue(); grpc_channel *wrapped_channel = channel->GetWrappedChannel(); - grpc_call *wrapped_call = grpc_channel_create_call( + grpc_call *wrapped_call; + if (args[3]->IsString()) { + NanUtf8String host_override(args[3]); + wrapped_call = grpc_channel_create_call( wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, - channel->GetHost(), MillisecondsToTimespec(deadline)); + *host_override, MillisecondsToTimespec(deadline)); + } else if (args[3]->IsUndefined() || args[3]->IsNull()) { + wrapped_call = grpc_channel_create_call( + wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, + NULL, MillisecondsToTimespec(deadline)); + } else { + return NanThrowTypeError("Call's fourth argument must be a string"); + } call = new Call(wrapped_call); args.This()->SetHiddenValue(NanNew("channel_"), channel_object); } diff --git a/ext/channel.cc b/ext/channel.cc index d02ed956..457a58c0 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -59,14 +59,12 @@ using v8::Value; NanCallback *Channel::constructor; Persistent Channel::fun_tpl; -Channel::Channel(grpc_channel *channel, NanUtf8String *host) - : wrapped_channel(channel), host(host) {} +Channel::Channel(grpc_channel *channel) : wrapped_channel(channel) {} Channel::~Channel() { if (wrapped_channel != NULL) { grpc_channel_destroy(wrapped_channel); } - delete host; } void Channel::Init(Handle exports) { @@ -91,8 +89,6 @@ bool Channel::HasInstance(Handle val) { grpc_channel *Channel::GetWrappedChannel() { return this->wrapped_channel; } -char *Channel::GetHost() { return **this->host; } - NAN_METHOD(Channel::New) { NanScope(); @@ -103,8 +99,7 @@ NAN_METHOD(Channel::New) { } grpc_channel *wrapped_channel; // Owned by the Channel object - NanUtf8String *host = new NanUtf8String(args[0]); - NanUtf8String *host_override = NULL; + NanUtf8String host(args[0]); grpc_credentials *creds; if (!Credentials::HasInstance(args[1])) { return NanThrowTypeError( @@ -116,12 +111,9 @@ NAN_METHOD(Channel::New) { grpc_channel_args *channel_args_ptr; if (args[2]->IsUndefined()) { channel_args_ptr = NULL; - wrapped_channel = grpc_insecure_channel_create(**host, NULL); + wrapped_channel = grpc_insecure_channel_create(*host, NULL); } else if (args[2]->IsObject()) { Handle 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))); - } Handle keys(args_hash->GetOwnPropertyNames()); grpc_channel_args channel_args; channel_args.num_args = keys->Length(); @@ -153,20 +145,15 @@ NAN_METHOD(Channel::New) { return NanThrowTypeError("Channel expects a string and an object"); } if (creds == NULL) { - wrapped_channel = grpc_insecure_channel_create(**host, channel_args_ptr); + wrapped_channel = grpc_insecure_channel_create(*host, channel_args_ptr); } else { wrapped_channel = - grpc_secure_channel_create(creds, **host, channel_args_ptr); + 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); - } else { - channel = new Channel(wrapped_channel, host_override); - } + Channel *channel = new Channel(wrapped_channel); channel->Wrap(args.This()); NanReturnValue(args.This()); } else { diff --git a/ext/channel.h b/ext/channel.h index 6725ebb0..e2182cb4 100644 --- a/ext/channel.h +++ b/ext/channel.h @@ -53,11 +53,8 @@ class Channel : public ::node::ObjectWrap { /* Returns the grpc_channel struct that this object wraps */ grpc_channel *GetWrappedChannel(); - /* Return the hostname that this channel connects to */ - char *GetHost(); - private: - explicit Channel(grpc_channel *channel, NanUtf8String *host); + explicit Channel(grpc_channel *channel); ~Channel(); // Prevent copying @@ -71,7 +68,6 @@ class Channel : public ::node::ObjectWrap { static v8::Persistent fun_tpl; grpc_channel *wrapped_channel; - NanUtf8String *host; }; } // namespace node From dff79ef1d848b630e2f7fd6e86164cbf14cf7595 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 3 Aug 2015 15:18:23 -0700 Subject: [PATCH 13/17] Added host override option for RPCs. Added optional params object --- examples/perf_test.js | 2 +- interop/interop_client.js | 3 +- src/client.js | 59 ++++++++++++++++++++------------------- test/call_test.js | 5 ++++ test/end_to_end_test.js | 8 ------ 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/examples/perf_test.js b/examples/perf_test.js index 0f38725f..214b9384 100644 --- a/examples/perf_test.js +++ b/examples/perf_test.js @@ -63,7 +63,7 @@ function runTest(iterations, callback) { var timeDiff = process.hrtime(startTime); intervals[i] = timeDiff[0] * 1000000 + timeDiff[1] / 1000; next(i+1); - }, {}, deadline); + }, {}, {deadline: deadline}); } } next(0); diff --git a/interop/interop_client.js b/interop/interop_client.js index 236b3661..68f47477 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -268,7 +268,7 @@ function cancelAfterFirstResponse(client, done) { function timeoutOnSleepingServer(client, done) { var deadline = new Date(); deadline.setMilliseconds(deadline.getMilliseconds() + 1); - var call = client.fullDuplexCall(null, deadline); + var call = client.fullDuplexCall(null, {deadline: deadline}); call.write({ payload: {body: zeroBuffer(27182)} }); @@ -409,6 +409,7 @@ function runTest(address, host_override, test_case, tls, test_ca, done) { creds = grpc.Credentials.createSsl(ca_data); if (host_override) { options['grpc.ssl_target_name_override'] = host_override; + options['grpc.default_authority'] = host_override; } } else { creds = grpc.Credentials.createInsecure(); diff --git a/src/client.js b/src/client.js index b2b44237..87c7690d 100644 --- a/src/client.js +++ b/src/client.js @@ -207,6 +207,25 @@ ClientReadableStream.prototype.getPeer = getPeer; ClientWritableStream.prototype.getPeer = getPeer; ClientDuplexStream.prototype.getPeer = getPeer; +/** + * Get a call object built with the provided options. Keys for options are + * 'deadline', which takes a date or number, and 'host', which takes a string + * and overrides the hostname to connect to. + * @param {Object} options Options map. + */ +function getCall(channel, method, options) { + var deadline; + var host; + if (options) { + deadline = options.deadline; + host = options.host; + } + if (deadline === undefined) { + deadline = Infinity; + } + return new grpc.Call(channel, method, deadline, host); +} + /** * Get a function that can make unary requests to the specified method. * @param {string} method The name of the method to request @@ -226,17 +245,13 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { * response is received * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Object=} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeUnaryRequest(argument, callback, metadata, deadline) { + function makeUnaryRequest(argument, callback, metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } var emitter = new EventEmitter(); - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -300,16 +315,12 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) { * response is received * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Object=} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeClientStreamRequest(callback, metadata, deadline) { + function makeClientStreamRequest(callback, metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -374,16 +385,12 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) { * serialize * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Object} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeServerStreamRequest(argument, metadata, deadline) { + function makeServerStreamRequest(argument, metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -446,16 +453,12 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) { * @this {SurfaceClient} Client object. Must have a channel member. * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Options} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeBidiStreamRequest(metadata, deadline) { + function makeBidiStreamRequest(metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } diff --git a/test/call_test.js b/test/call_test.js index 48d859a8..8d0f20b0 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -84,6 +84,11 @@ describe('call', function() { new grpc.Call(channel, 'method', 0); }); }); + it('should accept an optional fourth string parameter', function() { + assert.doesNotThrow(function() { + new grpc.Call(channel, 'method', new Date(), 'host_override'); + }); + }); it('should fail with a closed channel', function() { var local_channel = new grpc.Channel('hostname', insecureCreds); local_channel.close(); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index ea41dfc2..7574d98b 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -74,8 +74,6 @@ describe('end-to-end', function() { }); it('should start and end a request without error', function(complete) { var done = multiDone(complete, 2); - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'xyz'; var call = new grpc.Call(channel, 'dummy_method', @@ -126,8 +124,6 @@ describe('end-to-end', function() { }); it('should successfully send and receive metadata', function(complete) { var done = multiDone(complete, 2); - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'xyz'; var call = new grpc.Call(channel, 'dummy_method', @@ -184,8 +180,6 @@ describe('end-to-end', function() { var req_text = 'client_request'; var reply_text = 'server_response'; var done = multiDone(complete, 2); - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'success'; var call = new grpc.Call(channel, 'dummy_method', @@ -241,8 +235,6 @@ describe('end-to-end', function() { it('should send multiple messages', function(complete) { var done = multiDone(complete, 2); var requests = ['req1', 'req2']; - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'xyz'; var call = new grpc.Call(channel, 'dummy_method', From 33591d5e19f654dfd14b64106617266251ae7fa0 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 11 Aug 2015 17:30:05 -0700 Subject: [PATCH 14/17] Fixed lint errors --- interop/interop_client.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/interop/interop_client.js b/interop/interop_client.js index 55d243ca..27f6c19c 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -67,7 +67,7 @@ function zeroBuffer(size) { * primarily for use with mocha */ function emptyUnary(client, done) { - var call = client.emptyCall({}, function(err, resp) { + client.emptyCall({}, function(err, resp) { assert.ifError(err); if (done) { done(); @@ -89,7 +89,7 @@ function largeUnary(client, done) { body: zeroBuffer(271828) } }; - var call = client.unaryCall(arg, function(err, resp) { + client.unaryCall(arg, function(err, resp) { assert.ifError(err); assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); @@ -293,7 +293,7 @@ function authTest(expected_user, scope, client, done) { fill_username: true, fill_oauth_scope: true }; - var call = client.unaryCall(arg, function(err, resp) { + client.unaryCall(arg, function(err, resp) { assert.ifError(err); assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); @@ -328,7 +328,7 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { }; var makeTestCall = function(error, client_metadata) { assert.ifError(error); - var call = client.unaryCall(arg, function(err, resp) { + client.unaryCall(arg, function(err, resp) { assert.ifError(err); assert.strictEqual(resp.username, expected_user); assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); From 1e65e15085512221153ab2d2935f43988d8b062b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 12 Aug 2015 10:48:39 -0700 Subject: [PATCH 15/17] Fixed failing cloud-to-prod auth interop tests --- interop/interop_client.js | 6 ++++-- src/client.js | 11 +++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/interop/interop_client.js b/interop/interop_client.js index 221d69e2..6152d445 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -298,7 +298,9 @@ function authTest(expected_user, scope, client, done) { assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); assert.strictEqual(resp.username, expected_user); - assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); + if (scope) { + assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); + } if (done) { done(); } @@ -335,7 +337,7 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { if (done) { done(); } - }); + }, client_metadata); }; if (per_rpc) { updateMetadata('', {}, makeTestCall); diff --git a/src/client.js b/src/client.js index b2b44237..a253c860 100644 --- a/src/client.js +++ b/src/client.js @@ -523,7 +523,7 @@ var requester_makers = { * requestSerialize: function to serialize request objects * responseDeserialize: function to deserialize response objects * @param {Object} methods An object mapping method names to method attributes - * @param {string} serviceName The name of the service + * @param {string} serviceName The fully qualified name of the service * @return {function(string, Object)} New client constructor */ exports.makeClientConstructor = function(methods, serviceName) { @@ -548,8 +548,10 @@ exports.makeClientConstructor = function(methods, serviceName) { } options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, credentials, options); - this.server_address = address.replace(/\/$/, ''); - this.auth_uri = this.server_address + '/' + serviceName; + // Extract the DNS name from the address string + address = address.replace(/(\w+:\/\/)?([^:]+)(:\d+)?\/?$/, '$2'); + this.server_address = address; + this.auth_uri = 'https://' + this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; } @@ -587,7 +589,8 @@ exports.makeClientConstructor = function(methods, serviceName) { */ exports.makeProtobufClientConstructor = function(service) { var method_attrs = common.getProtobufServiceAttrs(service, service.name); - var Client = exports.makeClientConstructor(method_attrs); + var Client = exports.makeClientConstructor( + method_attrs, common.fullyQualifiedName(service)); Client.service = service; return Client; }; From 7970af5e1b681aa6a983e3b1753781a447c7a8c9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 12 Aug 2015 11:32:54 -0700 Subject: [PATCH 16/17] Fix scheme matching in auth URI --- src/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.js b/src/client.js index a253c860..4e631456 100644 --- a/src/client.js +++ b/src/client.js @@ -549,7 +549,7 @@ exports.makeClientConstructor = function(methods, serviceName) { options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, credentials, options); // Extract the DNS name from the address string - address = address.replace(/(\w+:\/\/)?([^:]+)(:\d+)?\/?$/, '$2'); + address = address.replace(/(\w+:\/\/+)?([^:]+)(:\d+)?\/?$/, '$2'); this.server_address = address; this.auth_uri = 'https://' + this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; From 02b7005b49380bacaba21dc112586c17e4303cf7 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 12 Aug 2015 11:54:41 -0700 Subject: [PATCH 17/17] Clarified address regex --- src/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index 4e631456..bbb45382 100644 --- a/src/client.js +++ b/src/client.js @@ -548,8 +548,8 @@ exports.makeClientConstructor = function(methods, serviceName) { } options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, credentials, options); - // Extract the DNS name from the address string - address = address.replace(/(\w+:\/\/+)?([^:]+)(:\d+)?\/?$/, '$2'); + // Remove the optional DNS scheme, trailing port, and trailing backslash + address = address.replace(/^(dns:\/{3})?([^:\/]+)(:\d+)?\/?$/, '$2'); this.server_address = address; this.auth_uri = 'https://' + this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata;