fix: clean up memory leaks related to mongocrypt_binary_t (#166)

* refactor: inline all persistent ctor references
* fix: wrap all mongocrypt_binary_t in smart pointers
This fixes two places where we weren't cleaning up memory after
creating binary instances
* fix: mongocrypt_binary_t is non-owning, don't copy buffer contents
BufferToBinary was needlessly making a copy of the data for the
passed buffer when creating a new `mongocrypt_binary_t`. Since that
type is a non-owning view, we should directly pass the contents of
the buffer.

NODE-2985

Author: mbroadst

bindings/node/src/mongocrypt.cc

@@ -37,11 +37,7 @@ std::string StringFromBinary(mongocrypt_binary_t* binary) {
 mongocrypt_binary_t* BufferToBinary(v8::Local<v8::Object> node_buffer) {
     uint8_t* buffer = (uint8_t*)node::Buffer::Data(node_buffer);
     size_t buffer_len = node::Buffer::Length(node_buffer);
-
-    uint8_t* buffer_copy = new uint8_t[buffer_len];
-    memcpy(buffer_copy, buffer, buffer_len);
-
-    return mongocrypt_binary_new_from_data(buffer_copy, buffer_len);
+    return mongocrypt_binary_new_from_data(buffer, buffer_len);
 }
 
 v8::Local<v8::Object> BufferFromBinary(mongocrypt_binary_t* binary) {
@@ -121,7 +117,6 @@ std::string errorStringFromStatus(mongocrypt_ctx_t* context) {
     return errorMessage;
 }
 
-Nan::Persistent<v8::Function> MongoCrypt::constructor;
 NAN_MODULE_INIT(MongoCrypt::Init) {
     v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
     tpl->SetClassName(Nan::New("MongoCrypt").ToLocalChecked());
@@ -138,7 +133,7 @@ NAN_MODULE_INIT(MongoCrypt::Init) {
 
     Nan::SetAccessor(itpl, Nan::New("status").ToLocalChecked(), Status);
 
-    constructor.Reset(Nan::GetFunction(tpl).ToLocalChecked());
+    constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
     Nan::Set(
         target, Nan::New("MongoCrypt").ToLocalChecked(), Nan::GetFunction(tpl).ToLocalChecked());
 }
@@ -400,7 +395,9 @@ NAN_METHOD(MongoCrypt::New) {
                     return;
                 }
 
-                if (!mongocrypt_setopt_kms_providers(crypt.get(), BufferToBinary(kmsProvidersOptions))) {
+                std::unique_ptr<mongocrypt_binary_t, MongoCryptBinaryDeleter> kmsProvidersBinary(
+                    BufferToBinary(kmsProvidersOptions));
+                if (!mongocrypt_setopt_kms_providers(crypt.get(), kmsProvidersBinary.get())) {
                     Nan::ThrowTypeError(errorStringFromStatus(crypt.get()));
                     return;
                 }
@@ -416,7 +413,9 @@ NAN_METHOD(MongoCrypt::New) {
                     return;
                 }
 
-                if (!mongocrypt_setopt_schema_map(crypt.get(), BufferToBinary(schemaMapBuffer))) {
+                std::unique_ptr<mongocrypt_binary_t, MongoCryptBinaryDeleter> schemaMapBinary(
+                    BufferToBinary(schemaMapBuffer));
+                if (!mongocrypt_setopt_schema_map(crypt.get(), schemaMapBinary.get())) {
                     Nan::ThrowTypeError(errorStringFromStatus(crypt.get()));
                     return;
                 }
@@ -493,7 +492,7 @@ NAN_METHOD(MongoCrypt::New) {
 
     const int argc = 1;
     v8::Local<v8::Value> argv[argc] = {info[0]};
-    v8::Local<v8::Function> ctor = Nan::New<v8::Function>(MongoCrypt::constructor);
+    v8::Local<v8::Function> ctor = Nan::New<v8::Function>(constructor());
     info.GetReturnValue().Set(Nan::NewInstance(ctor, argc, argv).ToLocalChecked());
 }
 
@@ -680,7 +679,6 @@ NAN_METHOD(MongoCrypt::MakeDataKeyContext) {
     info.GetReturnValue().Set(result);
 }
 
-Nan::Persistent<v8::Function> MongoCryptContext::constructor;
 NAN_MODULE_INIT(MongoCryptContext::Init) {
     v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>();
     tpl->SetClassName(Nan::New("MongoCryptContext").ToLocalChecked());
@@ -697,15 +695,15 @@ NAN_MODULE_INIT(MongoCryptContext::Init) {
     Nan::SetAccessor(itpl, Nan::New("status").ToLocalChecked(), Status);
     Nan::SetAccessor(itpl, Nan::New("state").ToLocalChecked(), State);
 
-    constructor.Reset(Nan::GetFunction(tpl).ToLocalChecked());
+    constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
     Nan::Set(target,
              Nan::New("MongoCryptContext").ToLocalChecked(),
              Nan::GetFunction(tpl).ToLocalChecked());
 }
 
 v8::Local<v8::Object> MongoCryptContext::NewInstance(mongocrypt_ctx_t* context) {
     Nan::EscapableHandleScope scope;
-    v8::Local<v8::Function> ctor = Nan::New<v8::Function>(MongoCryptContext::constructor);
+    v8::Local<v8::Function> ctor = Nan::New<v8::Function>(constructor());
     v8::Local<v8::Object> object = Nan::NewInstance(ctor).ToLocalChecked();
     MongoCryptContext* class_instance = new MongoCryptContext(context);
     class_instance->Wrap(object);
@@ -794,7 +792,6 @@ NAN_METHOD(MongoCryptContext::Finalize) {
     info.GetReturnValue().Set(buffer);
 }
 
-Nan::Persistent<v8::Function> MongoCryptKMSRequest::constructor;
 NAN_MODULE_INIT(MongoCryptKMSRequest::Init) {
     v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>();
     tpl->SetClassName(Nan::New("MongoCryptKMSRequest").ToLocalChecked());
@@ -808,15 +805,15 @@ NAN_MODULE_INIT(MongoCryptKMSRequest::Init) {
     Nan::SetAccessor(itpl, Nan::New("endpoint").ToLocalChecked(), Endpoint);
     Nan::SetAccessor(itpl, Nan::New("message").ToLocalChecked(), Message);
 
-    constructor.Reset(Nan::GetFunction(tpl).ToLocalChecked());
+    constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
     Nan::Set(target,
              Nan::New("MongoCryptKMSRequest").ToLocalChecked(),
              Nan::GetFunction(tpl).ToLocalChecked());
 }
 
 v8::Local<v8::Object> MongoCryptKMSRequest::NewInstance(mongocrypt_kms_ctx_t* kms_context) {
     Nan::EscapableHandleScope scope;
-    v8::Local<v8::Function> ctor = Nan::New<v8::Function>(MongoCryptKMSRequest::constructor);
+    v8::Local<v8::Function> ctor = Nan::New<v8::Function>(constructor());
     v8::Local<v8::Object> object = Nan::NewInstance(ctor).ToLocalChecked();
     MongoCryptKMSRequest* class_instance = new MongoCryptKMSRequest(kms_context);
     class_instance->Wrap(object);

bindings/node/src/mongocrypt.h

@@ -31,7 +31,10 @@ class MongoCrypt : public Nan::ObjectWrap {
     static NAN_MODULE_INIT(Init);
 
    private:
-    static Nan::Persistent<v8::Function> constructor;
+    static inline Nan::Persistent<v8::Function> & constructor() {
+        static Nan::Persistent<v8::Function> ctor;
+        return ctor;
+    }
 
     static NAN_METHOD(New);
     static NAN_METHOD(MakeEncryptionContext);
@@ -73,7 +76,10 @@ class MongoCryptContext : public Nan::ObjectWrap {
     static v8::Local<v8::Object> NewInstance(mongocrypt_ctx_t* context);
 
    private:
-    static Nan::Persistent<v8::Function> constructor;
+    static inline Nan::Persistent<v8::Function> & constructor() {
+        static Nan::Persistent<v8::Function> ctor;
+        return ctor;
+    }
 
     static NAN_METHOD(NextMongoOperation);
     static NAN_METHOD(AddMongoOperationResponse);
@@ -96,7 +102,10 @@ class MongoCryptKMSRequest : public Nan::ObjectWrap {
     static v8::Local<v8::Object> NewInstance(mongocrypt_kms_ctx_t* kms_context);
 
    private:
-    static Nan::Persistent<v8::Function> constructor;
+    static inline Nan::Persistent<v8::Function> & constructor() {
+        static Nan::Persistent<v8::Function> ctor;
+        return ctor;
+    }
 
     static NAN_METHOD(AddResponse);