From 110ead9abb6351bc9fffac1ed2cca99728e0477c Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna.henningsen@mongodb.com>
Date: Wed, 25 Jan 2023 21:06:55 +0100
Subject: [PATCH] vm: expose cachedDataRejected for vm.compileFunction

Having this information available is useful for functions just as
it is for scripts. Therefore, expose it in the same way that other
information related to code caching is reported.

As part of this, de-duplify the code for setting the properties on
the C++ side and add proper exception handling to it.

PR-URL: https://github.com/nodejs/node/pull/46320
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
---
 doc/api/vm.md                  |   6 ++
 lib/internal/vm.js             |   4 ++
 src/node_contextify.cc         | 125 ++++++++++++++++++++-------------
 src/node_contextify.h          |   8 +++
 test/parallel/test-vm-basic.js |  26 +++++--
 5 files changed, 114 insertions(+), 55 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 09081fad2240bf..481aded758150b 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -962,6 +962,12 @@ const vm = require('node:vm');
 <!-- YAML
 added: v10.10.0
 changes:
+  - version:
+    - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/46320
+    description: The return value now includes `cachedDataRejected`
+                 with the same semantics as the `vm.Script` version
+                 if the `cachedData` option was passed.
   - version:
     - v17.0.0
     - v16.12.0
diff --git a/lib/internal/vm.js b/lib/internal/vm.js
index 848e1ec5d559cd..0e2e275aa3c1a5 100644
--- a/lib/internal/vm.js
+++ b/lib/internal/vm.js
@@ -90,6 +90,10 @@ function internalCompileFunction(code, params, options) {
     result.function.cachedData = result.cachedData;
   }
 
+  if (typeof result.cachedDataRejected === 'boolean') {
+    result.function.cachedDataRejected = result.cachedDataRejected;
+  }
+
   if (importModuleDynamically !== undefined) {
     validateFunction(importModuleDynamically,
                      'options.importModuleDynamically');
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 53b947fd1bcf2d..ed552ddd559f51 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -49,6 +49,7 @@ using v8::HandleScope;
 using v8::IndexedPropertyHandlerConfiguration;
 using v8::Int32;
 using v8::Isolate;
+using v8::Just;
 using v8::Local;
 using v8::Maybe;
 using v8::MaybeLocal;
@@ -58,6 +59,7 @@ using v8::MicrotaskQueue;
 using v8::MicrotasksPolicy;
 using v8::Name;
 using v8::NamedPropertyHandlerConfiguration;
+using v8::Nothing;
 using v8::Number;
 using v8::Object;
 using v8::ObjectTemplate;
@@ -857,40 +859,75 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
   }
   contextify_script->script_.Reset(isolate, v8_script);
 
-  Local<Context> env_context = env->context();
-  if (compile_options == ScriptCompiler::kConsumeCodeCache) {
-    args.This()->Set(
-        env_context,
-        env->cached_data_rejected_string(),
-        Boolean::New(isolate, source.GetCachedData()->rejected)).Check();
-  } else if (produce_cached_data) {
-    std::unique_ptr<ScriptCompiler::CachedData> cached_data{
-        ScriptCompiler::CreateCodeCache(v8_script)};
-    bool cached_data_produced = cached_data != nullptr;
-    if (cached_data_produced) {
-      MaybeLocal<Object> buf = Buffer::Copy(
-          env,
-          reinterpret_cast<const char*>(cached_data->data),
-          cached_data->length);
-      args.This()->Set(env_context,
-                       env->cached_data_string(),
-                       buf.ToLocalChecked()).Check();
-    }
-    args.This()->Set(
-        env_context,
-        env->cached_data_produced_string(),
-        Boolean::New(isolate, cached_data_produced)).Check();
+  std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
+  if (produce_cached_data) {
+    new_cached_data.reset(ScriptCompiler::CreateCodeCache(v8_script));
   }
 
-  args.This()
-      ->Set(env_context,
-            env->source_map_url_string(),
-            v8_script->GetSourceMappingURL())
-      .Check();
+  if (StoreCodeCacheResult(env,
+                           args.This(),
+                           compile_options,
+                           source,
+                           produce_cached_data,
+                           std::move(new_cached_data))
+          .IsNothing()) {
+    return;
+  }
+
+  if (args.This()
+          ->Set(env->context(),
+                env->source_map_url_string(),
+                v8_script->GetSourceMappingURL())
+          .IsNothing())
+    return;
 
   TRACE_EVENT_END0(TRACING_CATEGORY_NODE2(vm, script), "ContextifyScript::New");
 }
 
+Maybe<bool> StoreCodeCacheResult(
+    Environment* env,
+    Local<Object> target,
+    ScriptCompiler::CompileOptions compile_options,
+    const v8::ScriptCompiler::Source& source,
+    bool produce_cached_data,
+    std::unique_ptr<ScriptCompiler::CachedData> new_cached_data) {
+  Local<Context> context;
+  if (!target->GetCreationContext().ToLocal(&context)) {
+    return Nothing<bool>();
+  }
+  if (compile_options == ScriptCompiler::kConsumeCodeCache) {
+    if (target
+            ->Set(
+                context,
+                env->cached_data_rejected_string(),
+                Boolean::New(env->isolate(), source.GetCachedData()->rejected))
+            .IsNothing()) {
+      return Nothing<bool>();
+    }
+  }
+  if (produce_cached_data) {
+    bool cached_data_produced = new_cached_data != nullptr;
+    if (cached_data_produced) {
+      MaybeLocal<Object> buf =
+          Buffer::Copy(env,
+                       reinterpret_cast<const char*>(new_cached_data->data),
+                       new_cached_data->length);
+      if (target->Set(context, env->cached_data_string(), buf.ToLocalChecked())
+              .IsNothing()) {
+        return Nothing<bool>();
+      }
+    }
+    if (target
+            ->Set(context,
+                  env->cached_data_produced_string(),
+                  Boolean::New(env->isolate(), cached_data_produced))
+            .IsNothing()) {
+      return Nothing<bool>();
+    }
+  }
+  return Just(true);
+}
+
 bool ContextifyScript::InstanceOf(Environment* env,
                                   const Local<Value>& value) {
   return !value.IsEmpty() &&
@@ -1242,28 +1279,18 @@ void ContextifyContext::CompileFunction(
           .IsNothing())
     return;
 
+  std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
   if (produce_cached_data) {
-    const std::unique_ptr<ScriptCompiler::CachedData> cached_data(
-        ScriptCompiler::CreateCodeCacheForFunction(fn));
-    bool cached_data_produced = cached_data != nullptr;
-    if (cached_data_produced) {
-      MaybeLocal<Object> buf = Buffer::Copy(
-          env,
-          reinterpret_cast<const char*>(cached_data->data),
-          cached_data->length);
-      if (result
-              ->Set(parsing_context,
-                    env->cached_data_string(),
-                    buf.ToLocalChecked())
-              .IsNothing())
-        return;
-    }
-    if (result
-            ->Set(parsing_context,
-                  env->cached_data_produced_string(),
-                  Boolean::New(isolate, cached_data_produced))
-            .IsNothing())
-      return;
+    new_cached_data.reset(ScriptCompiler::CreateCodeCacheForFunction(fn));
+  }
+  if (StoreCodeCacheResult(env,
+                           result,
+                           options,
+                           source,
+                           produce_cached_data,
+                           std::move(new_cached_data))
+          .IsNothing()) {
+    return;
   }
 
   args.GetReturnValue().Set(result);
diff --git a/src/node_contextify.h b/src/node_contextify.h
index 87440169503fbd..76c89318bb6cbf 100644
--- a/src/node_contextify.h
+++ b/src/node_contextify.h
@@ -199,6 +199,14 @@ class CompiledFnEntry final : public BaseObject {
   static void WeakCallback(const v8::WeakCallbackInfo<CompiledFnEntry>& data);
 };
 
+v8::Maybe<bool> StoreCodeCacheResult(
+    Environment* env,
+    v8::Local<v8::Object> target,
+    v8::ScriptCompiler::CompileOptions compile_options,
+    const v8::ScriptCompiler::Source& source,
+    bool produce_cached_data,
+    std::unique_ptr<v8::ScriptCompiler::CachedData> new_cached_data);
+
 }  // namespace contextify
 }  // namespace node
 
diff --git a/test/parallel/test-vm-basic.js b/test/parallel/test-vm-basic.js
index 13d212f0079ca7..f2424128b66e9f 100644
--- a/test/parallel/test-vm-basic.js
+++ b/test/parallel/test-vm-basic.js
@@ -323,13 +323,27 @@ const vm = require('vm');
     global
   );
 
-  // Test compileFunction produceCachedData option
-  const result = vm.compileFunction('console.log("Hello, World!")', [], {
-    produceCachedData: true,
-  });
+  {
+    const source = 'console.log("Hello, World!")';
+    // Test compileFunction produceCachedData option
+    const result = vm.compileFunction(source, [], {
+      produceCachedData: true,
+    });
 
-  assert.ok(result.cachedDataProduced);
-  assert.ok(result.cachedData.length > 0);
+    assert.ok(result.cachedDataProduced);
+    assert.ok(result.cachedData.length > 0);
+
+    // Test compileFunction cachedData consumption
+    const result2 = vm.compileFunction(source, [], {
+      cachedData: result.cachedData
+    });
+    assert.strictEqual(result2.cachedDataRejected, false);
+
+    const result3 = vm.compileFunction('console.log("wrong source")', [], {
+      cachedData: result.cachedData
+    });
+    assert.strictEqual(result3.cachedDataRejected, true);
+  }
 
   // Resetting value
   Error.stackTraceLimit = oldLimit;