From 0e008b11f426ea6fa0987d49e4267538d7a5226d Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 28 Sep 2015 16:31:16 -0700 Subject: [PATCH] Fixed some issues with new credential code --- ext/call.cc | 6 +- ext/credentials.cc | 15 +++-- interop/interop_client.js | 4 +- test/channel_test.js | 3 +- test/credentials_test.js | 113 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 129 insertions(+), 12 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index e2a0bafe..fccb30f5 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -729,13 +729,17 @@ NAN_METHOD(Call::GetPeer) { NAN_METHOD(Call::SetCredentials) { Nan::HandleScope scope; + if (!HasInstance(info.This())) { + return Nan::ThrowTypeError( + "setCredentials can only be called on Call objects"); + } if (!Credentials::HasInstance(info[0])) { return Nan::ThrowTypeError( "setCredentials' first argument must be a credential"); } Call *call = ObjectWrap::Unwrap(info.This()); Credentials *creds_object = ObjectWrap::Unwrap( - Nan::To(info[1]).ToLocalChecked()); + Nan::To(info[0]).ToLocalChecked()); grpc_credentials *creds = creds_object->GetWrappedCredentials(); grpc_call_error error = GRPC_CALL_ERROR; if (creds) { diff --git a/ext/credentials.cc b/ext/credentials.cc index eb8c3ea8..ec5cc5cb 100644 --- a/ext/credentials.cc +++ b/ext/credentials.cc @@ -288,12 +288,16 @@ NAN_METHOD(PluginCallback) { return Nan::ThrowTypeError( "The callback's third argument must be an object"); } + shared_ptr resources(new Resources); grpc_status_code code = static_cast( Nan::To(info[0]).FromJust()); - char *details = *Nan::Utf8String(info[1]); + //Utf8String details_str(info[1]); + //char *details = static_cast(calloc(details_str.length(), sizeof(char))); + //memcpy(details, *details_str, details_str.length()); + char *details = *Utf8String(info[1]); grpc_metadata_array array; if (!CreateMetadataArray(Nan::To(info[2]).ToLocalChecked(), - &array, shared_ptr(new Resources))){ + &array, resources)){ return Nan::ThrowError("Failed to parse metadata"); } grpc_credentials_plugin_metadata_cb cb = @@ -305,7 +309,6 @@ NAN_METHOD(PluginCallback) { Nan::Get(info.Callee(), Nan::New("user_data").ToLocalChecked() ).ToLocalChecked().As()->Value(); - gpr_log(GPR_DEBUG, "Calling plugin metadata callback"); cb(user_data, array.metadata, array.count, code, details); } @@ -329,13 +332,13 @@ NAUV_WORK_CB(SendPluginCallback) { callback->Call(argc, argv); delete data; uv_unref((uv_handle_t *)async); - delete async; + uv_close((uv_handle_t *)async, (uv_close_cb)free); } void plugin_get_metadata(void *state, const char *service_url, grpc_credentials_plugin_metadata_cb cb, void *user_data) { - uv_async_t *async = new uv_async_t; + uv_async_t *async = static_cast(malloc(sizeof(uv_async_t))); uv_async_init(uv_default_loop(), async, SendPluginCallback); @@ -345,6 +348,8 @@ void plugin_get_metadata(void *state, const char *service_url, data->cb = cb; data->user_data = user_data; async->data = data; + /* libuv says that it will coalesce calls to uv_async_send. If there is ever a + * problem with a callback not getting called, that is probably the reason */ uv_async_send(async); } diff --git a/interop/interop_client.js b/interop/interop_client.js index 0fae0faa..84cd7aff 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -345,7 +345,9 @@ function perRpcAuthTest(expected_user, scope, per_rpc, client, done) { fill_username: true, fill_oauth_scope: true }; - credential = credential.createScoped(scope); + if (credential.createScopedRequired() && scope) { + credential = credential.createScoped(scope); + } var creds = grpc.credentials.createFromGoogleCredential(credential); client.unaryCall(arg, function(err, resp) { assert.ifError(err); diff --git a/test/channel_test.js b/test/channel_test.js index d81df2a3..2e436222 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -149,12 +149,13 @@ describe('channel', function() { afterEach(function() { channel.close(); }); - it('should time out if called alone', function(done) { + it.only('should time out if called alone', function(done) { var old_state = channel.getConnectivityState(); var deadline = new Date(); deadline.setSeconds(deadline.getSeconds() + 1); channel.watchConnectivityState(old_state, deadline, function(err, value) { assert(err); + console.log('Callback from watchConnectivityState'); done(); }); }); diff --git a/test/credentials_test.js b/test/credentials_test.js index b18c267a..23f01f86 100644 --- a/test/credentials_test.js +++ b/test/credentials_test.js @@ -39,7 +39,28 @@ var path = require('path'); var grpc = require('..'); -describe('client credentials', function() { +/** + * This is used for testing functions with multiple asynchronous calls that + * can happen in different orders. This should be passed the number of async + * function invocations that can occur last, and each of those should call this + * function's return value + * @param {function()} done The function that should be called when a test is + * complete. + * @param {number} count The number of calls to the resulting function if the + * test passes. + * @return {function()} The function that should be called at the end of each + * sequence of asynchronous functions. + */ +function multiDone(done, count) { + return function() { + count -= 1; + if (count <= 0) { + done(); + } + }; +} + +describe.only('client credentials', function() { var Client; var server; var port; @@ -94,7 +115,15 @@ describe('client credentials', function() { after(function() { server.forceShutdown(); }); - it.only('Should update metadata with SSL creds', function(done) { + it('Should accept SSL creds for a client', function(done) { + var client = new Client('localhost:' + port, client_ssl_creds, + client_options); + client.unary({}, function(err, data) { + assert.ifError(err); + done(); + }); + }); + it('Should update metadata with SSL creds', function(done) { var metadataUpdater = function(service_url, callback) { var metadata = new grpc.Metadata(); metadata.set('plugin_key', 'plugin_value'); @@ -103,16 +132,92 @@ describe('client credentials', function() { var creds = grpc.credentials.createFromMetadataGenerator(metadataUpdater); var combined_creds = grpc.credentials.combineCredentials(client_ssl_creds, creds); - //combined_creds = grpc.credentials.createInsecure(); var client = new Client('localhost:' + port, combined_creds, client_options); var call = client.unary({}, function(err, data) { assert.ifError(err); - console.log('Received response'); }); call.on('metadata', function(metadata) { assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); done(); }); }); + it('Should update metadata for two simultaneous calls', function(done) { + done = multiDone(done, 2); + var metadataUpdater = function(service_url, callback) { + var metadata = new grpc.Metadata(); + metadata.set('plugin_key', 'plugin_value'); + callback(null, metadata); + }; + var creds = grpc.credentials.createFromMetadataGenerator(metadataUpdater); + var combined_creds = grpc.credentials.combineCredentials(client_ssl_creds, + creds); + var client = new Client('localhost:' + port, combined_creds, + client_options); + var call = client.unary({}, function(err, data) { + assert.ifError(err); + }); + call.on('metadata', function(metadata) { + assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); + done(); + }); + var call2 = client.unary({}, function(err, data) { + assert.ifError(err); + }); + call2.on('metadata', function(metadata) { + assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); + done(); + }); + }); + describe('Per-rpc creds', function() { + var client; + var updater_creds; + before(function() { + client = new Client('localhost:' + port, client_ssl_creds, + client_options); + var metadataUpdater = function(service_url, callback) { + var metadata = new grpc.Metadata(); + metadata.set('plugin_key', 'plugin_value'); + callback(null, metadata); + }; + updater_creds = grpc.credentials.createFromMetadataGenerator( + metadataUpdater); + }); + it('Should update metadata on a unary call', function(done) { + var call = client.unary({}, function(err, data) { + assert.ifError(err); + }, null, {credentials: updater_creds}); + call.on('metadata', function(metadata) { + assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); + done(); + }); + }); + it('should update metadata on a client streaming call', function(done) { + var call = client.clientStream(function(err, data) { + assert.ifError(err); + }, null, {credentials: updater_creds}); + call.on('metadata', function(metadata) { + assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); + done(); + }); + call.end(); + }); + it('should update metadata on a server streaming call', function(done) { + var call = client.serverStream({}, null, {credentials: updater_creds}); + call.on('data', function() {}); + call.on('metadata', function(metadata) { + assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); + done(); + }); + }); + it('should update metadata on a bidi streaming call', function(done) { + var call = client.bidiStream(null, {credentials: updater_creds}); + call.on('data', function() {}); + call.on('metadata', function(metadata) { + assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); + done(); + }); + call.end(); + }); + }); });