Fix for Mixed binary roundtrip issue (#4278) (#4366)

* Fix for Mixed binary roundtrip issue (#4278)

CHANGELOG.md

@@ -5,7 +5,7 @@ x.x.x Release notes (yyyy-MM-dd)
 
 ### Fixed
 * <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-js/issues/????), since v?.?.?)
-* None.
+* Fixed issue that could cause mangling of binary data on a roundtrip to/from the database ([#4278](https://github.com/realm/realm-js/issues/4278), since v10.1.4).
 
 ### Compatibility
 * MongoDB Realm Cloud.

src/js_object_accessor.hpp

@@ -487,8 +487,8 @@ struct Unbox<JSEngine, Mixed> {
     static Mixed call(NativeAccessor<JSEngine>* ctx, typename JSEngine::Value const& value, realm::CreatePolicy,
                       ObjKey)
     {
-        return js::Value<JSEngine>::to_mixed(ctx->m_ctx, ctx->m_realm, value,
-                                             ctx->m_string_buffer); // no need to validate type for a mixed value
+        return js::Value<JSEngine>::to_mixed(ctx->m_ctx, ctx->m_realm, value, ctx->m_string_buffer,
+                                             ctx->m_owned_binary_data); // no need to validate type for a mixed value
     }
 };
 

src/js_types.hpp

@@ -204,7 +204,7 @@ struct Value {
     static OwnedBinaryData to_binary(ContextType, const ValueType&);
     static bson::Bson to_bson(ContextType, ValueType);
     static Mixed to_mixed(ContextType ctx, std::shared_ptr<Realm> realm, const ValueType& value,
-                          std::string& string_buffer);
+                          std::string& string_buffer, OwnedBinaryData& binary_buffer);
 
 #define VALIDATED(return_t, type)                                                                                    \
     static return_t validated_to_##type(ContextType ctx, const ValueType& value, const char* name = nullptr)         \
@@ -875,7 +875,7 @@ inline bson::Bson Value<T>::to_bson(typename T::Context ctx, ValueType value)
 
 template <typename T>
 typename realm::Mixed Value<T>::to_mixed(ContextType ctx, std::shared_ptr<Realm> realm, const ValueType& value,
-                                         std::string& string_buffer)
+                                         std::string& string_buffer, OwnedBinaryData& binary_buffer)
 {
     if (is_null(ctx, value) || is_undefined(ctx, value)) {
         return Mixed(realm::null());
@@ -901,7 +901,8 @@ typename realm::Mixed Value<T>::to_mixed(ContextType ctx, std::shared_ptr<Realm>
         return Mixed(string_buffer);
     }
     else if (is_binary(ctx, value)) {
-        return Mixed(to_binary(ctx, value).get());
+        binary_buffer = std::move(to_binary(ctx, value));
+        return Mixed(binary_buffer.get());
     }
     else if (is_decimal128(ctx, value)) {
         return Mixed(to_decimal128(ctx, value));

src/node/node_value.hpp

@@ -263,6 +263,7 @@ template <>
 inline OwnedBinaryData node::Value::to_binary(Napi::Env env, const Napi::Value& value)
 {
 
+    // TODO:  this pointer is no good :(  It is never de-allocated
     NodeBinary* node_binary = nullptr;
 
 

tests/js/mixed-tests.js

@@ -224,4 +224,35 @@ module.exports = {
 
     realm.close();
   },
+
+  // test Mixed datatype with binary data contents
+  testMixedData() {
+    const buffer1 = new Uint8Array([1, 2, 4, 8]).buffer;
+    const buffer2 = new Uint8Array([255, 128, 64, 32, 16, 8]).buffer;
+
+    const MixedSchema = {
+      name: "MixedWithData",
+      properties: { value: "mixed" },
+    };
+
+    const realm = new Realm({ schema: [MixedSchema] });
+
+    realm.write(() => {
+      realm.create("MixedWithData", { value: buffer1 });
+    });
+
+    let mixedObjects = realm.objects("MixedWithData");
+    let returnedData = [...new Uint8Array(mixedObjects[0].value)];
+    TestCase.assertArraysEqual(returnedData, [1, 2, 4, 8]);
+
+    realm.write(() => {
+      mixedObjects[0].value = buffer2;
+    });
+
+    mixedObjects = realm.objects("MixedWithData");
+    returnedData = [...new Uint8Array(mixedObjects[0].value)];
+    TestCase.assertArraysEqual(returnedData, [255, 128, 64, 32, 16, 8]);
+
+    realm.close();
+  },
 };