node-api: type tag external values without v8::Private

legendecas · RafaelGSS · commit 3a492056e22b · 2024-01-02T15:05:25.000-03:00
v8::External can not have any properties and private properties. Type tag v8::External with a wrapper struct without setting a private property on the v8::External. PR-URL: #51149 Fixes: nodejs/node-v8#273 Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
@@ -825,6 +825,58 @@ void Reference::WeakCallback(const v8::WeakCallbackInfo<Reference>& data) {
   reference->env_->InvokeFinalizerFromGC(reference);
 }
 
+/**
+ * A wrapper for `v8::External` to support type-tagging. `v8::External` doesn't
+ * support defining any properties and private properties on it, even though it
+ * is an object. This wrapper is used to store the type tag and the data of the
+ * external value.
+ */
+class ExternalWrapper {
+ private:
+  explicit ExternalWrapper(void* data) : data_(data) {}
+
+  static void WeakCallback(const v8::WeakCallbackInfo<ExternalWrapper>& data) {
+    ExternalWrapper* wrapper = data.GetParameter();
+    delete wrapper;
+  }
+
+ public:
+  static v8::Local<v8::External> New(napi_env env, void* data) {
+    ExternalWrapper* wrapper = new ExternalWrapper(data);
+    v8::Local<v8::External> external = v8::External::New(env->isolate, wrapper);
+    wrapper->persistent_.Reset(env->isolate, external);
+    wrapper->persistent_.SetWeak(
+        wrapper, WeakCallback, v8::WeakCallbackType::kParameter);
+
+    return external;
+  }
+
+  static ExternalWrapper* From(v8::Local<v8::External> external) {
+    return static_cast<ExternalWrapper*>(external->Value());
+  }
+
+  void* Data() { return data_; }
+
+  bool TypeTag(const napi_type_tag* type_tag) {
+    if (type_tag_ != nullptr) {
+      return false;
+    }
+    type_tag_ = type_tag;
+    return true;
+  }
+
+  bool CheckTypeTag(const napi_type_tag* type_tag) {
+    return type_tag == type_tag_ ||
+           (type_tag_ && type_tag->lower == type_tag_->lower &&
+            type_tag->upper == type_tag_->upper);
+  }
+
+ private:
+  v8impl::Persistent<v8::Value> persistent_;
+  void* data_;
+  const napi_type_tag* type_tag_ = nullptr;
+};
+
 }  // end of namespace v8impl
 
 // Warning: Keep in-sync with napi_status enum
@@ -2525,9 +2577,8 @@ napi_create_external(napi_env env,
   NAPI_PREAMBLE(env);
   CHECK_ARG(env, result);
 
-  v8::Isolate* isolate = env->isolate;
-
-  v8::Local<v8::Value> external_value = v8::External::New(isolate, data);
+  v8::Local<v8::External> external_value =
+      v8impl::ExternalWrapper::New(env, data);
 
   if (finalize_cb) {
     // The Reference object will delete itself after invoking the finalizer
@@ -2547,12 +2598,24 @@ napi_create_external(napi_env env,
 }
 
 napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
-                                            napi_value object,
+                                            napi_value object_or_external,
                                             const napi_type_tag* type_tag) {
   NAPI_PREAMBLE(env);
   v8::Local<v8::Context> context = env->context();
+
+  CHECK_ARG(env, object_or_external);
+  v8::Local<v8::Value> val =
+      v8impl::V8LocalValueFromJsValue(object_or_external);
+  if (val->IsExternal()) {
+    v8impl::ExternalWrapper* wrapper =
+        v8impl::ExternalWrapper::From(val.As<v8::External>());
+    RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
+        env, wrapper->TypeTag(type_tag), napi_invalid_arg);
+    return GET_RETURN_STATUS(env);
+  }
+
   v8::Local<v8::Object> obj;
-  CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
+  CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
   CHECK_ARG_WITH_PREAMBLE(env, type_tag);
 
   auto key = NAPI_PRIVATE_KEY(context, type_tag);
@@ -2574,13 +2637,24 @@ napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
 }
 
 napi_status NAPI_CDECL napi_check_object_type_tag(napi_env env,
-                                                  napi_value object,
+                                                  napi_value object_or_external,
                                                   const napi_type_tag* type_tag,
                                                   bool* result) {
   NAPI_PREAMBLE(env);
   v8::Local<v8::Context> context = env->context();
+
+  CHECK_ARG(env, object_or_external);
+  v8::Local<v8::Value> obj_val =
+      v8impl::V8LocalValueFromJsValue(object_or_external);
+  if (obj_val->IsExternal()) {
+    v8impl::ExternalWrapper* wrapper =
+        v8impl::ExternalWrapper::From(obj_val.As<v8::External>());
+    *result = wrapper->CheckTypeTag(type_tag);
+    return GET_RETURN_STATUS(env);
+  }
+
   v8::Local<v8::Object> obj;
-  CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
+  CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
   CHECK_ARG_WITH_PREAMBLE(env, type_tag);
   CHECK_ARG_WITH_PREAMBLE(env, result);
 
@@ -2625,7 +2699,7 @@ napi_status NAPI_CDECL napi_get_value_external(napi_env env,
   RETURN_STATUS_IF_FALSE(env, val->IsExternal(), napi_invalid_arg);
 
   v8::Local<v8::External> external_value = val.As<v8::External>();
-  *result = external_value->Value();
+  *result = v8impl::ExternalWrapper::From(external_value)->Data();
 
   return napi_clear_last_error(env);
 }
diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc
@@ -250,7 +250,8 @@ napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) {
   napi_value key, value;
   CHECK_EQ(napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key),
            napi_ok);
-  CHECK_EQ(napi_create_external(env, instance_data, nullptr, nullptr, &value),
+  CHECK_EQ(napi_create_external_arraybuffer(
+               env, instance_data, 1, nullptr, nullptr, &value),
            napi_ok);
   CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok);
   return nullptr;
@@ -289,9 +290,9 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) {
             .ToLocalChecked();
     v8::Local<v8::Value> completion_value =
         script->Run(context).ToLocalChecked();
-    CHECK(completion_value->IsExternal());
+    CHECK(completion_value->IsArrayBuffer());
     instance_data =
-        static_cast<int*>(completion_value.As<v8::External>()->Value());
+        static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
     CHECK_NE(instance_data, nullptr);
     CHECK_EQ(*instance_data, 0);
   }
@@ -327,9 +328,9 @@ TEST_F(LinkedBindingTest,
             .ToLocalChecked();
     v8::Local<v8::Value> completion_value =
         script->Run(context).ToLocalChecked();
-    CHECK(completion_value->IsExternal());
+    CHECK(completion_value->IsArrayBuffer());
     instance_data =
-        static_cast<int*>(completion_value.As<v8::External>()->Value());
+        static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
     CHECK_NE(instance_data, nullptr);
     CHECK_EQ(*instance_data, 0);
   }
