From a9172d2a8a48a41a1f7f0e2b397d37e2a31cae98 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 9 Dec 2015 16:12:37 -0800 Subject: [PATCH] Simplified some code and added tests to increase code coverage --- src/node/ext/channel_credentials.cc | 6 ++ src/node/ext/server_credentials.cc | 24 ++--- src/node/test/credentials_test.js | 147 ++++++++++++++++++++++++++-- src/node/test/server_test.js | 25 ++++- 4 files changed, 175 insertions(+), 27 deletions(-) diff --git a/src/node/ext/channel_credentials.cc b/src/node/ext/channel_credentials.cc index b77ff80af27..059bc8a8902 100644 --- a/src/node/ext/channel_credentials.cc +++ b/src/node/ext/channel_credentials.cc @@ -154,6 +154,12 @@ NAN_METHOD(ChannelCredentials::CreateSsl) { return Nan::ThrowTypeError( "createSSl's third argument must be a Buffer if provided"); } + if ((key_cert_pair.private_key == NULL) != + (key_cert_pair.cert_chain == NULL)) { + return Nan::ThrowError( + "createSsl's second and third arguments must be" + " provided or omitted together"); + } grpc_channel_credentials *creds = grpc_ssl_credentials_create( root_certs, key_cert_pair.private_key == NULL ? NULL : &key_cert_pair, NULL); diff --git a/src/node/ext/server_credentials.cc b/src/node/ext/server_credentials.cc index e6c55e263cb..5285d53df41 100644 --- a/src/node/ext/server_credentials.cc +++ b/src/node/ext/server_credentials.cc @@ -167,30 +167,18 @@ NAN_METHOD(ServerCredentials::CreateSsl) { return Nan::ThrowTypeError("Key/cert pairs must be objects"); } Local pair_obj = Nan::To(pair_val).ToLocalChecked(); - MaybeLocal maybe_key = Nan::Get(pair_obj, key_key); - if (maybe_key.IsEmpty()) { - delete key_cert_pairs; - return Nan::ThrowTypeError( - "Key/cert pairs must have a private_key and a cert_chain"); - } - MaybeLocal maybe_cert = Nan::Get(pair_obj, cert_key); - if (maybe_cert.IsEmpty()) { - delete key_cert_pairs; - return Nan::ThrowTypeError( - "Key/cert pairs must have a private_key and a cert_chain"); - } - if (!::node::Buffer::HasInstance(maybe_key.ToLocalChecked())) { + Local maybe_key = Nan::Get(pair_obj, key_key).ToLocalChecked(); + Local maybe_cert = Nan::Get(pair_obj, cert_key).ToLocalChecked(); + if (!::node::Buffer::HasInstance(maybe_key)) { delete key_cert_pairs; return Nan::ThrowTypeError("private_key must be a Buffer"); } - if (!::node::Buffer::HasInstance(maybe_cert.ToLocalChecked())) { + if (!::node::Buffer::HasInstance(maybe_cert)) { delete key_cert_pairs; return Nan::ThrowTypeError("cert_chain must be a Buffer"); } - key_cert_pairs[i].private_key = ::node::Buffer::Data( - maybe_key.ToLocalChecked()); - key_cert_pairs[i].cert_chain = ::node::Buffer::Data( - maybe_cert.ToLocalChecked()); + key_cert_pairs[i].private_key = ::node::Buffer::Data(maybe_key); + key_cert_pairs[i].cert_chain = ::node::Buffer::Data(maybe_cert); } grpc_server_credentials *creds = grpc_ssl_server_credentials_create( root_certs, key_cert_pairs, key_cert_pair_count, force_client_auth, NULL); diff --git a/src/node/test/credentials_test.js b/src/node/test/credentials_test.js index 647f648ca93..294600c85a9 100644 --- a/src/node/test/credentials_test.js +++ b/src/node/test/credentials_test.js @@ -76,6 +76,146 @@ var fakeFailingGoogleCredentials = { } }; +var key_data, pem_data, ca_data; + +before(function() { + var key_path = path.join(__dirname, './data/server1.key'); + var pem_path = path.join(__dirname, './data/server1.pem'); + var ca_path = path.join(__dirname, '../test/data/ca.pem'); + key_data = fs.readFileSync(key_path); + pem_data = fs.readFileSync(pem_path); + ca_data = fs.readFileSync(ca_path); +}); + +describe('channel credentials', function() { + describe('#createSsl', function() { + it('works with no arguments', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.credentials.createSsl(); + }); + assert.notEqual(creds, null); + }); + it('works with just one Buffer argument', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.credentials.createSsl(ca_data); + }); + assert.notEqual(creds, null); + }); + it('works with 3 Buffer arguments', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.credentials.createSsl(ca_data, key_data, pem_data); + }); + assert.notEqual(creds, null); + }); + it('works if the first argument is null', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.credentials.createSsl(null, key_data, pem_data); + }); + assert.notEqual(creds, null); + }); + it('fails if the first argument is a non-Buffer value', function() { + assert.throws(function() { + grpc.credentials.createSsl('test'); + }, TypeError); + }); + it('fails if the second argument is a non-Buffer value', function() { + assert.throws(function() { + grpc.credentials.createSsl(null, 'test', pem_data); + }, TypeError); + }); + it('fails if the third argument is a non-Buffer value', function() { + assert.throws(function() { + grpc.credentials.createSsl(null, key_data, 'test'); + }, TypeError); + }); + it('fails if only 1 of the last 2 arguments is provided', function() { + assert.throws(function() { + grpc.credentials.createSsl(null, key_data); + }); + assert.throws(function() { + grpc.credentials.createSsl(null, null, pem_data); + }); + }); + }); +}); + +describe('server credentials', function() { + describe('#createSsl', function() { + it('accepts a buffer and array as the first 2 arguments', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.ServerCredentials.createSsl(ca_data, []); + }); + assert.notEqual(creds, null); + }); + it('accepts a boolean as the third argument', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.ServerCredentials.createSsl(ca_data, [], true); + }); + assert.notEqual(creds, null); + }); + it('accepts an object with two buffers in the second argument', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.ServerCredentials.createSsl(null, + [{private_key: key_data, + cert_chain: pem_data}]); + }); + assert.notEqual(creds, null); + }); + it('accepts multiple objects in the second argument', function() { + var creds; + assert.doesNotThrow(function() { + creds = grpc.ServerCredentials.createSsl(null, + [{private_key: key_data, + cert_chain: pem_data}, + {private_key: key_data, + cert_chain: pem_data}]); + }); + assert.notEqual(creds, null); + }); + it('fails if the second argument is not an Array', function() { + assert.throws(function() { + grpc.ServerCredentials.createSsl(ca_data, 'test'); + }, TypeError); + }); + it('fails if the first argument is a non-Buffer value', function() { + assert.throws(function() { + grpc.ServerCredentials.createSsl('test', []); + }, TypeError); + }); + it('fails if the third argument is a non-boolean value', function() { + assert.throws(function() { + grpc.ServerCredentials.createSsl(ca_data, [], 'test'); + }, TypeError); + }); + it('fails if the array elements are not objects', function() { + assert.throws(function() { + grpc.ServerCredentials.createSsl(ca_data, 'test'); + }, TypeError); + }); + it('fails if the object does not have a Buffer private_key', function() { + assert.throws(function() { + grpc.ServerCredentials.createSsl(null, + [{private_key: 'test', + cert_chain: pem_data}]); + }, TypeError); + }); + it('fails if the object does not have a Buffer cert_chain', function() { + assert.throws(function() { + grpc.ServerCredentials.createSsl(null, + [{private_key: key_data, + cert_chain: 'test'}]); + }, TypeError); + }); + }); +}); + describe('client credentials', function() { var Client; var server; @@ -109,20 +249,13 @@ describe('client credentials', function() { }); } }); - var key_path = path.join(__dirname, './data/server1.key'); - var pem_path = path.join(__dirname, './data/server1.pem'); - var key_data = fs.readFileSync(key_path); - var pem_data = fs.readFileSync(pem_path); var creds = grpc.ServerCredentials.createSsl(null, [{private_key: key_data, cert_chain: pem_data}]); - //creds = grpc.ServerCredentials.createInsecure(); port = server.bind('localhost:0', creds); server.start(); Client = proto.TestService; - var ca_path = path.join(__dirname, '../test/data/ca.pem'); - var ca_data = fs.readFileSync(ca_path); client_ssl_creds = grpc.credentials.createSsl(ca_data); var host_override = 'foo.test.google.fr'; client_options['grpc.ssl_target_name_override'] = host_override; diff --git a/src/node/test/server_test.js b/src/node/test/server_test.js index 999a183b3cb..592f47e1456 100644 --- a/src/node/test/server_test.js +++ b/src/node/test/server_test.js @@ -45,9 +45,30 @@ describe('server', function() { new grpc.Server(); }); }); - it('should work with an empty list argument', function() { + it('should work with an empty object argument', function() { assert.doesNotThrow(function() { - new grpc.Server([]); + new grpc.Server({}); + }); + }); + it('should work without the new keyword', function() { + var server; + assert.doesNotThrow(function() { + server = grpc.Server(); + }); + assert(server instanceof grpc.Server); + }); + it('should only accept objects with string or int values', function() { + assert.doesNotThrow(function() { + new grpc.Server({'key' : 'value'}); + }); + assert.doesNotThrow(function() { + new grpc.Server({'key' : 5}); + }); + assert.throws(function() { + new grpc.Server({'key' : null}); + }); + assert.throws(function() { + new grpc.Server({'key' : new Date()}); }); }); });