Merge pull request #2679 from murgatroid99/node_explicit_insecure_channel
Make insecure channel/stub construction explicit
diff --git a/src/node/examples/perf_test.js b/src/node/examples/perf_test.js
index da919ec..0f38725 100644
--- a/src/node/examples/perf_test.js
+++ b/src/node/examples/perf_test.js
@@ -41,7 +41,8 @@
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/src/node/examples/qps_test.js b/src/node/examples/qps_test.js
index 00293b4..1ce4dbe 100644
--- a/src/node/examples/qps_test.js
+++ b/src/node/examples/qps_test.js
@@ -61,7 +61,8 @@
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/src/node/examples/route_guide_client.js b/src/node/examples/route_guide_client.js
index 8cd532f..647f3ff 100644
--- a/src/node/examples/route_guide_client.js
+++ b/src/node/examples/route_guide_client.js
@@ -40,7 +40,8 @@
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/src/node/examples/stock_client.js b/src/node/examples/stock_client.js
index b37e66d..ab9b050 100644
--- a/src/node/examples/stock_client.js
+++ b/src/node/examples/stock_client.js
@@ -38,7 +38,8 @@
* 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/src/node/ext/channel.cc b/src/node/ext/channel.cc
index c43b55f..d02ed95 100644
--- a/src/node/ext/channel.cc
+++ b/src/node/ext/channel.cc
@@ -98,31 +98,30 @@
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 @@
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 @@
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 d6cff06..21d61f1 100644
--- a/src/node/ext/credentials.cc
+++ b/src/node/ext/credentials.cc
@@ -81,6 +81,8 @@
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 @@
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::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 @@
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 @@
}
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 @@
}
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 794736f..62957e6 100644
--- a/src/node/ext/credentials.h
+++ b/src/node/ext/credentials.h
@@ -68,6 +68,7 @@
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;
diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js
index e810e68..236b366 100644
--- a/src/node/interop/interop_client.js
+++ b/src/node/interop/interop_client.js
@@ -397,6 +397,7 @@
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 @@
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/node/src/client.js b/src/node/src/client.js
index f843669..b2b4423 100644
--- a/src/node/src/client.js
+++ b/src/node/src/client.js
@@ -531,11 +531,13 @@
* 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);
@@ -545,7 +547,7 @@
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/src/node/test/call_test.js b/src/node/test/call_test.js
index 942c31a..c5edab8 100644
--- a/src/node/test/call_test.js
+++ b/src/node/test/call_test.js
@@ -48,6 +48,8 @@
return deadline;
}
+var insecureCreds = grpc.Credentials.createInsecure();
+
describe('call', function() {
var channel;
var server;
@@ -55,7 +57,7 @@
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 @@
});
});
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/src/node/test/channel_test.js b/src/node/test/channel_test.js
index 3e61d3b..c991d7b 100644
--- a/src/node/test/channel_test.js
+++ b/src/node/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,49 @@
new grpc.Channel(5);
});
});
- it('should accept an object for the second parameter', function() {
+ it('should require 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);
});
+ assert.throws(function() {
+ new grpc.Channel('hostname');
+ });
+ });
+ 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 +102,7 @@
});
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/src/node/test/end_to_end_test.js b/src/node/test/end_to_end_test.js
index 5d3baf8..9133cec 100644
--- a/src/node/test/end_to_end_test.js
+++ b/src/node/test/end_to_end_test.js
@@ -57,6 +57,8 @@
};
}
+var insecureCreds = grpc.Credentials.createInsecure();
+
describe('end-to-end', function() {
var server;
var channel;
@@ -64,7 +66,7 @@
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/src/node/test/health_test.js b/src/node/test/health_test.js
index bb700cc..93b068b 100644
--- a/src/node/test/health_test.js
+++ b/src/node/test/health_test.js
@@ -56,7 +56,8 @@
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/src/node/test/math_client_test.js b/src/node/test/math_client_test.js
index f275185..1bd85ca 100644
--- a/src/node/test/math_client_test.js
+++ b/src/node/test/math_client_test.js
@@ -53,7 +53,8 @@
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/src/node/test/surface_test.js b/src/node/test/surface_test.js
index 98f9b15..18c99a5 100644
--- a/src/node/test/surface_test.js
+++ b/src/node/test/surface_test.js
@@ -124,7 +124,7 @@
});
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 @@
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 @@
});
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() {
@@ -337,7 +338,7 @@
});
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() {
@@ -382,7 +383,8 @@
};
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) {
@@ -602,7 +604,7 @@
});
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() {