src: implement IsInsideNodeModules() in C++

This previously compiles a script and run it in a new context
to avoid global pollution, which is more complex than necessary
and can be too slow for it to be reused in other cases. The
new implementation just checks the frames in C++ which is safe
from global pollution, faster and simpler.

The previous implementation also had a bug when the call site
is in a ESM, because ESM have URLs as their script names,
which don't start with '/' or '\' and will be skipped. The new
implementation removes the skipping to fix it for ESM.

PR-URL: nodejs#55286
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>

Author: joyeecheung

lib/buffer.js

@@ -77,10 +77,10 @@ const {
     ONLY_ENUMERABLE,
   },
   getOwnNonIndexProperties,
+  isInsideNodeModules,
 } = internalBinding('util');
 const {
   customInspectSymbol,
-  isInsideNodeModules,
   lazyDOMException,
   normalizeEncoding,
   kIsEncodingSymbol,
@@ -176,13 +176,15 @@ function showFlaggedDeprecation() {
   if (bufferWarningAlreadyEmitted ||
       ++nodeModulesCheckCounter > 10000 ||
       (!require('internal/options').getOptionValue('--pending-deprecation') &&
-       isInsideNodeModules())) {
+       isInsideNodeModules(100, true))) {
     // We don't emit a warning, because we either:
     // - Already did so, or
     // - Already checked too many times whether a call is coming
     //   from node_modules and want to stop slowing down things, or
     // - We aren't running with `--pending-deprecation` enabled,
     //   and the code is inside `node_modules`.
+    // - We found node_modules in up to the topmost 100 frames, or
+    //   there are more than 100 frames and we don't want to search anymore.
     return;
   }
 

lib/internal/util.js

@@ -3,7 +3,6 @@
 const {
   ArrayBufferPrototypeGetByteLength,
   ArrayFrom,
-  ArrayIsArray,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSort,
@@ -35,9 +34,7 @@ const {
   SafeSet,
   SafeWeakMap,
   SafeWeakRef,
-  StringPrototypeIncludes,
   StringPrototypeReplace,
-  StringPrototypeStartsWith,
   StringPrototypeToLowerCase,
   StringPrototypeToUpperCase,
   Symbol,
@@ -509,31 +506,6 @@ function getStructuredStack() {
   return getStructuredStackImpl();
 }
 
-function isInsideNodeModules() {
-  const stack = getStructuredStack();
-
-  // Iterate over all stack frames and look for the first one not coming
-  // from inside Node.js itself:
-  if (ArrayIsArray(stack)) {
-    for (const frame of stack) {
-      const filename = frame.getFileName();
-
-      if (
-        filename == null ||
-        StringPrototypeStartsWith(filename, 'node:') === true ||
-        (
-          filename[0] !== '/' &&
-          StringPrototypeIncludes(filename, '\\') === false
-        )
-      ) {
-        continue;
-      }
-      return isUnderNodeModules(filename);
-    }
-  }
-  return false;
-}
-
 function once(callback, { preserveReturnValue = false } = kEmptyObject) {
   let called = false;
   let returnValue;
@@ -916,7 +888,6 @@ module.exports = {
   guessHandleType,
   isArrayBufferDetached,
   isError,
-  isInsideNodeModules,
   isUnderNodeModules,
   join,
   lazyDOMException,

src/node_util.cc

@@ -254,6 +254,52 @@ static void ParseEnv(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(dotenv.ToObject(env));
 }
 
+bool starts_with(std::string_view view, std::string_view prefix) {
+    return view.size() >= prefix.size() && view.substr(0, prefix.size()) == prefix;
+}
+
+static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  CHECK_EQ(args.Length(), 2);
+  CHECK(args[0]->IsInt32());  // frame_limit
+  // The second argument is the default value.
+
+  int frames_limit = args[0].As<v8::Int32>()->Value();
+  Local<StackTrace> stack =
+      StackTrace::CurrentStackTrace(isolate, frames_limit);
+  int frame_count = stack->GetFrameCount();
+
+  // If the search requires looking into more than |frames_limit| frames, give
+  // up and return the specified default value.
+  if (frame_count == frames_limit) {
+    return args.GetReturnValue().Set(args[1]);
+  }
+
+  bool result = false;
+  for (int i = 0; i < frame_count; ++i) {
+    Local<StackFrame> stack_frame = stack->GetFrame(isolate, i);
+    Local<String> script_name = stack_frame->GetScriptName();
+
+    if (script_name.IsEmpty() || script_name->Length() == 0) {
+      continue;
+    }
+    Utf8Value script_name_utf8(isolate, script_name);
+    std::string_view script_name_str = script_name_utf8.ToStringView();
+    if (starts_with(script_name_str, "node:")) {  // Ported to work with C++17 on 20.x.
+      continue;
+    }
+    if (script_name_str.find("/node_modules/") != std::string::npos ||
+        script_name_str.find("\\node_modules\\") != std::string::npos ||
+        script_name_str.find("/node_modules\\") != std::string::npos ||
+        script_name_str.find("\\node_modules/") != std::string::npos) {
+      result = true;
+      break;
+    }
+  }
+
+  args.GetReturnValue().Set(result);
+}
+
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(GetPromiseDetails);
   registry->Register(GetProxyDetails);
@@ -269,6 +315,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(FastGuessHandleType);
   registry->Register(fast_guess_handle_type_.GetTypeInfo());
   registry->Register(ParseEnv);
+  registry->Register(IsInsideNodeModules);
 }
 
 void Initialize(Local<Object> target,
@@ -338,6 +385,7 @@ void Initialize(Local<Object> target,
     target->Set(context, env->constants_string(), constants).Check();
   }
 
+  SetMethod(context, target, "isInsideNodeModules", IsInsideNodeModules);
   SetMethodNoSideEffect(
       context, target, "getPromiseDetails", GetPromiseDetails);
   SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails);

test/fixtures/warning_node_modules/new-buffer-cjs.js

@@ -0,0 +1,48 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+const { spawnSyncAndAssert } = require('../common/child_process');
+
+if (process.env.NODE_PENDING_DEPRECATION)
+  common.skip('test does not work when NODE_PENDING_DEPRECATION is set');
+
+spawnSyncAndAssert(
+  process.execPath,
+  [ fixtures.path('warning_node_modules', 'new-buffer-cjs.js') ],
+  {
+    trim: true,
+    stderr: '',
+  }
+);
+
+spawnSyncAndAssert(
+  process.execPath,
+  [ fixtures.path('warning_node_modules', 'new-buffer-esm.mjs') ],
+  {
+    trim: true,
+    stderr: '',
+  }
+);
+
+spawnSyncAndAssert(
+  process.execPath,
+  [
+    '--pending-deprecation',
+    fixtures.path('warning_node_modules', 'new-buffer-cjs.js'),
+  ],
+  {
+    stderr: /DEP0005/
+  }
+);
+
+spawnSyncAndAssert(
+  process.execPath,
+  [
+    '--pending-deprecation',
+    fixtures.path('warning_node_modules', 'new-buffer-esm.mjs'),
+  ],
+  {
+    stderr: /DEP0005/
+  }
+);