From 8da674df493e2ded8dc812252ac4f1f90c362807 Mon Sep 17 00:00:00 2001
From: Dara Hayes <dara.hayes@redhat.com>
Date: Mon, 1 Oct 2018 19:56:07 +0100
Subject: [PATCH] vm: pass parsing_context to
 ScriptCompiler::CompileFunctionInContext

ContextifyContext::CompileFunction in src/node_contextify.cc
was incorrectly passing the context variable to
ScriptCompiler::CompileFunctionInContext

This meant that the parsingContext option in vm.compileFunction
was not being applied properly to the compiled function.

fixes: https://github.com/nodejs/node/issues/23194

doc: clarify parsingContext option for vm.compileScript

test: usage of parsingContext in vm.compileFunction

PR-URL: https://github.com/nodejs/node/pull/23206
Fixes: https://github.com/nodejs/node/issues/23194
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
---
 doc/api/vm.md                  |  4 ++--
 src/node_contextify.cc         |  2 +-
 test/parallel/test-vm-basic.js | 40 ++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 07923de5856e7d..5bf09de37940fb 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -673,8 +673,8 @@ added: v10.10.0
     data for the supplied source.
   * `produceCachedData` {boolean} Specifies whether to produce new cache data.
     **Default:** `false`.
-  * `parsingContext` {Object} The sandbox/context in which the said function
-    should be compiled in.
+  * `parsingContext` {Object} The [contextified][] sandbox in which the said
+    function should be compiled in.
   * `contextExtensions` {Object[]} An array containing a collection of context
     extensions (objects wrapping the current scope) to be applied while
     compiling. **Default:** `[]`.
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 023a659ebb66a5..9559747467440b 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -1070,7 +1070,7 @@ void ContextifyContext::CompileFunction(
   }
 
   MaybeLocal<Function> maybe_fun = ScriptCompiler::CompileFunctionInContext(
-      context, &source, params.size(), params.data(),
+      parsing_context, &source, params.size(), params.data(),
       context_extensions.size(), context_extensions.data(), options);
 
   Local<Function> fun;
diff --git a/test/parallel/test-vm-basic.js b/test/parallel/test-vm-basic.js
index c5adde4e5b2c4b..54b7c45ff8043a 100644
--- a/test/parallel/test-vm-basic.js
+++ b/test/parallel/test-vm-basic.js
@@ -267,6 +267,46 @@ const vm = require('vm');
     stack: 'Error: Sample Error\n    at <anonymous>:1:10'
   });
 
+  assert.strictEqual(
+    vm.compileFunction(
+      'return varInContext',
+      [],
+      {
+        parsingContext: vm.createContext({ varInContext: 'abc' })
+      }
+    )(),
+    'abc'
+  );
+
+  common.expectsError(() => {
+    vm.compileFunction(
+      'return varInContext',
+      []
+    )();
+  }, {
+    message: 'varInContext is not defined',
+    stack: 'ReferenceError: varInContext is not defined\n    at <anonymous>:1:1'
+  });
+
+  assert.notDeepStrictEqual(
+    vm.compileFunction(
+      'return global',
+      [],
+      {
+        parsingContext: vm.createContext({ global: {} })
+      }
+    )(),
+    global
+  );
+
+  assert.deepStrictEqual(
+    vm.compileFunction(
+      'return global',
+      []
+    )(),
+    global
+  );
+
   // Resetting value
   Error.stackTraceLimit = oldLimit;
 }