src: implement IsInsideNodeModules() in C++ · nodejs/node@2af72a7 · GitHub
Skip to content

Commit 2af72a7

Browse files
joyeecheungruyadorno
joyeecheung
authored and
ruyadorno
committed
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: #55286 Backport-PR-URL: #55217 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Refs: #52697
1 parent fcdd616 commit 2af72a7

10 files changed

Lines changed: 108 additions & 31 deletions

File tree

‎lib/buffer.js‎

Lines changed: 4 additions & 2 deletions

‎lib/internal/util.js‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
const {
44
ArrayFrom,
5-
ArrayIsArray,
65
ArrayPrototypePush,
76
ArrayPrototypeSlice,
87
ArrayPrototypeSort,
@@ -34,9 +33,7 @@ const {
3433
SafeSet,
3534
SafeWeakMap,
3635
SafeWeakRef,
37-
StringPrototypeIncludes,
3836
StringPrototypeReplace,
39-
StringPrototypeStartsWith,
4037
StringPrototypeToLowerCase,
4138
StringPrototypeToUpperCase,
4239
Symbol,
@@ -513,31 +510,6 @@ function getStructuredStack() {
513510
return getStructuredStackImpl();
514511
}
515512

516-
function isInsideNodeModules() {
517-
const stack = getStructuredStack();
518-
519-
// Iterate over all stack frames and look for the first one not coming
520-
// from inside Node.js itself:
521-
if (ArrayIsArray(stack)) {
522-
for (const frame of stack) {
523-
const filename = frame.getFileName();
524-
525-
if (
526-
filename == null ||
527-
StringPrototypeStartsWith(filename, 'node:') === true ||
528-
(
529-
filename[0] !== '/' &&
530-
StringPrototypeIncludes(filename, '\\') === false
531-
)
532-
) {
533-
continue;
534-
}
535-
return isUnderNodeModules(filename);
536-
}
537-
}
538-
return false;
539-
}
540-
541513
function once(callback, { preserveReturnValue = false } = kEmptyObject) {
542514
let called = false;
543515
let returnValue;
@@ -911,7 +883,6 @@ module.exports = {
911883
getSystemErrorName,
912884
guessHandleType,
913885
isError,
914-
isInsideNodeModules,
915886
isUnderNodeModules,
916887
isMacOS,
917888
isWindows,

‎src/node_util.cc‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,48 @@ static void GetCallSite(const FunctionCallbackInfo<Value>& args) {
298298
args.GetReturnValue().Set(callsites);
299299
}
300300

301+
static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {
302+
Isolate* isolate = args.GetIsolate();
303+
CHECK_EQ(args.Length(), 2);
304+
CHECK(args[0]->IsInt32()); // frame_limit
305+
// The second argument is the default value.
306+
307+
int frames_limit = args[0].As<v8::Int32>()->Value();
308+
Local<StackTrace> stack =
309+
StackTrace::CurrentStackTrace(isolate, frames_limit);
310+
int frame_count = stack->GetFrameCount();
311+
312+
// If the search requires looking into more than |frames_limit| frames, give
313+
// up and return the specified default value.
314+
if (frame_count == frames_limit) {
315+
return args.GetReturnValue().Set(args[1]);
316+
}
317+
318+
bool result = false;
319+
for (int i = 0; i < frame_count; ++i) {
320+
Local<StackFrame> stack_frame = stack->GetFrame(isolate, i);
321+
Local<String> script_name = stack_frame->GetScriptName();
322+
323+
if (script_name.IsEmpty() || script_name->Length() == 0) {
324+
continue;
325+
}
326+
Utf8Value script_name_utf8(isolate, script_name);
327+
std::string_view script_name_str = script_name_utf8.ToStringView();
328+
if (script_name_str.starts_with("node:")) {
329+
continue;
330+
}
331+
if (script_name_str.find("/node_modules/") != std::string::npos ||
332+
script_name_str.find("\\node_modules\\") != std::string::npos ||
333+
script_name_str.find("/node_modules\\") != std::string::npos ||
334+
script_name_str.find("\\node_modules/") != std::string::npos) {
335+
result = true;
336+
break;
337+
}
338+
}
339+
340+
args.GetReturnValue().Set(result);
341+
}
342+
301343
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
302344
registry->Register(GetPromiseDetails);
303345
registry->Register(GetProxyDetails);
@@ -313,6 +355,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
313355
registry->Register(FastGuessHandleType);
314356
registry->Register(fast_guess_handle_type_.GetTypeInfo());
315357
registry->Register(ParseEnv);
358+
registry->Register(IsInsideNodeModules);
316359
}
317360

318361
void Initialize(Local<Object> target,
@@ -396,6 +439,7 @@ void Initialize(Local<Object> target,
396439
target->Set(context, env->constants_string(), constants).Check();
397440
}
398441

442+
SetMethod(context, target, "isInsideNodeModules", IsInsideNodeModules);
399443
SetMethodNoSideEffect(
400444
context, target, "getPromiseDetails", GetPromiseDetails);
401445
SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails);

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

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/warning_node_modules/new-buffer-esm.mjs‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/warning_node_modules/node_modules/new-buffer-cjs/index.js‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/warning_node_modules/node_modules/new-buffer-cjs/package.json‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/warning_node_modules/node_modules/new-buffer-esm/index.js‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/warning_node_modules/node_modules/new-buffer-esm/package.json‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/parallel/test-buffer-constructor-node-modules.js‎

Lines changed: 48 additions & 0 deletions

0 commit comments

Comments
 (0)