From 893690f8608f0f523b0a7d397045a67adcc3f597 Mon Sep 17 00:00:00 2001
From: murgatroid99 <mlumish@google.com>
Date: Mon, 27 Jul 2015 14:56:40 -0700
Subject: [PATCH] Made credentials an explicit required argument to channels
---
src/node/examples/perf_test.js | 3 +-
src/node/examples/qps_test.js | 3 +-
src/node/examples/route_guide_client.js | 3 +-
src/node/examples/stock_client.js | 3 +-
src/node/ext/channel.cc | 52 +++++++++++++------------
src/node/ext/credentials.cc | 4 +-
src/node/interop/interop_client.js | 8 ++--
src/node/src/client.js | 6 ++-
src/node/test/call_test.js | 6 ++-
src/node/test/channel_test.js | 30 +++++++++-----
src/node/test/end_to_end_test.js | 4 +-
src/node/test/health_test.js | 3 +-
src/node/test/math_client_test.js | 3 +-
src/node/test/surface_test.js | 14 ++++---
14 files changed, 85 insertions(+), 57 deletions(-)
diff --git a/src/node/examples/perf_test.js b/src/node/examples/perf_test.js
index da919eced5..0f38725f72 100644
--- a/src/node/examples/perf_test.js
+++ b/src/node/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/src/node/examples/qps_test.js b/src/node/examples/qps_test.js
index 00293b464a..1ce4dbe070 100644
--- a/src/node/examples/qps_test.js
+++ b/src/node/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/src/node/examples/route_guide_client.js b/src/node/examples/route_guide_client.js
index 8cd532fef1..647f3ffaba 100644
--- a/src/node/examples/route_guide_client.js
+++ b/src/node/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/src/node/examples/stock_client.js b/src/node/examples/stock_client.js
index b37e66df07..ab9b050e9b 100644
--- a/src/node/examples/stock_client.js
+++ b/src/node/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/src/node/ext/channel.cc b/src/node/ext/channel.cc
index c43b55f115..d02ed95672 100644
--- a/src/node/ext/channel.cc
+++ b/src/node/ext/channel.cc
@@ -98,31 +98,30 @@ NAN_METHOD(Channel::New) {
if (args.IsConstructCall()) {
if (!args[0]->IsString()) {
- return NanThrowTypeError("Channel expects a string and an object");
+ return NanThrowTypeError(
+ "Channel expects a string, a credential and an object");
}
grpc_channel *wrapped_channel;
// Owned by the Channel object
NanUtf8String *host = new NanUtf8String(args[0]);
NanUtf8String *host_override = NULL;
- if (args[1]->IsUndefined()) {
+ grpc_credentials *creds;
+ if (!Credentials::HasInstance(args[1])) {
+ return NanThrowTypeError(
+ "Channel's second argument must be a credential");
+ }
+ Credentials *creds_object = ObjectWrap::Unwrap<Credentials>(
+ args[1]->ToObject());
+ creds = creds_object->GetWrappedCredentials();
+ grpc_channel_args *channel_args_ptr;
+ if (args[2]->IsUndefined()) {
+ channel_args_ptr = NULL;
wrapped_channel = grpc_insecure_channel_create(**host, NULL);
- } else if (args[1]->IsObject()) {
- grpc_credentials *creds = NULL;
- Handle<Object> args_hash(args[1]->ToObject()->Clone());
+ } else if (args[2]->IsObject()) {
+ Handle<Object> args_hash(args[2]->ToObject()->Clone());
if (args_hash->HasOwnProperty(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))) {
host_override = new NanUtf8String(args_hash->Get(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG)));
}
- if (args_hash->HasOwnProperty(NanNew("credentials"))) {
- Handle<Value> creds_value = args_hash->Get(NanNew("credentials"));
- if (!Credentials::HasInstance(creds_value)) {
- return NanThrowTypeError(
- "credentials arg must be a Credentials object");
- }
- Credentials *creds_object =
- ObjectWrap::Unwrap<Credentials>(creds_value->ToObject());
- creds = creds_object->GetWrappedCredentials();
- args_hash->Delete(NanNew("credentials"));
- }
Handle<Array> keys(args_hash->GetOwnPropertyNames());
grpc_channel_args channel_args;
channel_args.num_args = keys->Length();
@@ -149,16 +148,19 @@ NAN_METHOD(Channel::New) {
return NanThrowTypeError("Arg values must be strings");
}
}
- if (creds == NULL) {
- wrapped_channel = grpc_insecure_channel_create(**host, &channel_args);
- } else {
- wrapped_channel =
- grpc_secure_channel_create(creds, **host, &channel_args);
- }
- free(channel_args.args);
+ channel_args_ptr = &channel_args;
} else {
return NanThrowTypeError("Channel expects a string and an object");
}
+ if (creds == NULL) {
+ wrapped_channel = grpc_insecure_channel_create(**host, channel_args_ptr);
+ } else {
+ wrapped_channel =
+ grpc_secure_channel_create(creds, **host, channel_args_ptr);
+ }
+ if (channel_args_ptr != NULL) {
+ free(channel_args_ptr->args);
+ }
Channel *channel;
if (host_override == NULL) {
channel = new Channel(wrapped_channel, host);
@@ -168,8 +170,8 @@ NAN_METHOD(Channel::New) {
channel->Wrap(args.This());
NanReturnValue(args.This());
} else {
- const int argc = 2;
- Local<Value> argv[argc] = {args[0], args[1]};
+ const int argc = 3;
+ Local<Value> argv[argc] = {args[0], args[1], args[2]};
NanReturnValue(constructor->GetFunction()->NewInstance(argc, argv));
}
}
diff --git a/src/node/ext/credentials.cc b/src/node/ext/credentials.cc
index a695e3656d..21d61f1a7f 100644
--- a/src/node/ext/credentials.cc
+++ b/src/node/ext/credentials.cc
@@ -82,7 +82,7 @@ void Credentials::Init(Handle<Object> exports) {
ctr->Set(NanNew("createIam"),
NanNew<FunctionTemplate>(CreateIam)->GetFunction());
ctr->Set(NanNew("createInsecure"),
- NanNew<FunctionTemplate>(CreateIam)->GetFunction());
+ NanNew<FunctionTemplate>(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/src/node/interop/interop_client.js b/src/node/interop/interop_client.js
index e810e68e45..236b36616c 100644
--- a/src/node/interop/interop_client.js
+++ b/src/node/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/node/src/client.js b/src/node/src/client.js
index d89c656c07..50338a6e1e 100644
--- a/src/node/src/client.js
+++ b/src/node/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/src/node/test/call_test.js b/src/node/test/call_test.js
index 942c31ac68..c5edab8bcd 100644
--- a/src/node/test/call_test.js
+++ b/src/node/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/src/node/test/channel_test.js b/src/node/test/channel_test.js
index 3e61d3bbc6..3cb43334b0 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,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/src/node/test/end_to_end_test.js b/src/node/test/end_to_end_test.js
index 5d3baf823d..9133ceca92 100644
--- a/src/node/test/end_to_end_test.js
+++ b/src/node/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/src/node/test/health_test.js b/src/node/test/health_test.js
index bb700cc46c..93b068be31 100644
--- a/src/node/test/health_test.js
+++ b/src/node/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/src/node/test/math_client_test.js b/src/node/test/math_client_test.js
index f2751857ff..1bd85ca495 100644
--- a/src/node/test/math_client_test.js
+++ b/src/node/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/src/node/test/surface_test.js b/src/node/test/surface_test.js
index 9005cbd505..d63f48d64f 100644
--- a/src/node/test/surface_test.js
+++ b/src/node/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() {
--
GitLab