From ba444a949dbebbea7bca3d612d367b38ef3b12b5 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 16 Feb 2024 09:12:35 +0100
Subject: [PATCH] vm: implement isContext() directly in JS land with private
 symbol
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We are now directly checking the existence of a private symbol
in the object to determine if an object is a ContextifyContext
anyway, so there is no need to implement it in C++ anymore.

PR-URL: https://github.com/nodejs/node/pull/51685
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
---
 lib/internal/vm.js        | 12 ++++++------
 lib/internal/vm/module.js |  3 ++-
 lib/vm.js                 | 15 ++++++++++++++-
 src/node_contextify.cc    | 16 ----------------
 4 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/lib/internal/vm.js b/lib/internal/vm.js
index 39c818a2ce93c1..337a2cf0b1ee73 100644
--- a/lib/internal/vm.js
+++ b/lib/internal/vm.js
@@ -8,7 +8,6 @@ const {
 const {
   ContextifyScript,
   compileFunction,
-  isContext: _isContext,
 } = internalBinding('contextify');
 const {
   runInContext,
@@ -21,18 +20,19 @@ const {
 } = internalBinding('symbols');
 const {
   validateFunction,
-  validateObject,
-  kValidateObjectAllowArray,
 } = require('internal/validators');
 
 const {
   getOptionValue,
 } = require('internal/options');
+const {
+  privateSymbols: {
+    contextify_context_private_symbol,
+  },
+} = internalBinding('util');
 
 function isContext(object) {
-  validateObject(object, 'object', kValidateObjectAllowArray);
-
-  return _isContext(object);
+  return object[contextify_context_private_symbol] !== undefined;
 }
 
 function getHostDefinedOptionId(importModuleDynamically, hint) {
diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js
index a100c8beefc80a..c2ab2f258584da 100644
--- a/lib/internal/vm/module.js
+++ b/lib/internal/vm/module.js
@@ -17,7 +17,6 @@ const {
   TypeError,
 } = primordials;
 
-const { isContext } = internalBinding('contextify');
 const {
   isModuleNamespaceObject,
 } = require('internal/util/types');
@@ -75,6 +74,8 @@ const kContext = Symbol('kContext');
 const kPerContextModuleId = Symbol('kPerContextModuleId');
 const kLink = Symbol('kLink');
 
+const { isContext } = require('internal/vm');
+
 class Module {
   constructor(options) {
     emitExperimentalWarning('VM Modules');
diff --git a/lib/vm.js b/lib/vm.js
index d006bdfe2abfd2..34814c430d3253 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -49,6 +49,7 @@ const {
   validateString,
   validateStringArray,
   validateUint32,
+  kValidateObjectAllowArray,
   kValidateObjectAllowNullable,
 } = require('internal/validators');
 const {
@@ -59,7 +60,7 @@ const {
 const {
   getHostDefinedOptionId,
   internalCompileFunction,
-  isContext,
+  isContext: _isContext,
   registerImportModuleDynamically,
 } = require('internal/vm');
 const {
@@ -67,6 +68,18 @@ const {
 } = internalBinding('symbols');
 const kParsingContext = Symbol('script parsing context');
 
+/**
+ * Check if object is a context object created by vm.createContext().
+ * @throws {TypeError} If object is not an object in the first place, throws TypeError.
+ * @param {object} object Object to check.
+ * @returns {boolean}
+ */
+function isContext(object) {
+  validateObject(object, 'object', kValidateObjectAllowArray);
+
+  return _isContext(object);
+}
+
 class Script extends ContextifyScript {
   constructor(code, options = kEmptyObject) {
     code = `${code}`;
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index d22ca507614b1a..4585e759c0ec8d 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -342,7 +342,6 @@ void ContextifyContext::CreatePerIsolateProperties(
     IsolateData* isolate_data, Local<ObjectTemplate> target) {
   Isolate* isolate = isolate_data->isolate();
   SetMethod(isolate, target, "makeContext", MakeContext);
-  SetMethod(isolate, target, "isContext", IsContext);
   SetMethod(isolate, target, "compileFunction", CompileFunction);
   SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
 }
@@ -350,7 +349,6 @@ void ContextifyContext::CreatePerIsolateProperties(
 void ContextifyContext::RegisterExternalReferences(
     ExternalReferenceRegistry* registry) {
   registry->Register(MakeContext);
-  registry->Register(IsContext);
   registry->Register(CompileFunction);
   registry->Register(ContainsModuleSyntax);
   registry->Register(PropertyGetterCallback);
@@ -415,20 +413,6 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-
-void ContextifyContext::IsContext(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
-  CHECK(args[0]->IsObject());
-  Local<Object> sandbox = args[0].As<Object>();
-
-  Maybe<bool> result =
-      sandbox->HasPrivate(env->context(),
-                          env->contextify_context_private_symbol());
-  args.GetReturnValue().Set(result.FromJust());
-}
-
-
 void ContextifyContext::WeakCallback(
     const WeakCallbackInfo<ContextifyContext>& data) {
   ContextifyContext* context = data.GetParameter();