Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ececd22

Browse filesBrowse files
joyeecheungmarco-ippolito
joyeecheung
authored and
marco-ippolito
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: #56927 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Refs: #52697
1 parent 4fba019 commit ececd22
Copy full SHA for ececd22

File tree

Expand file treeCollapse file tree

10 files changed

+114
-31
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+114
-31
lines changed
Open diff view settings
Collapse file

‎lib/buffer.js‎

Copy file name to clipboardExpand all lines: lib/buffer.js
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ const {
7777
ONLY_ENUMERABLE,
7878
},
7979
getOwnNonIndexProperties,
80+
isInsideNodeModules,
8081
} = internalBinding('util');
8182
const {
8283
customInspectSymbol,
83-
isInsideNodeModules,
8484
lazyDOMException,
8585
normalizeEncoding,
8686
kIsEncodingSymbol,
@@ -176,13 +176,15 @@ function showFlaggedDeprecation() {
176176
if (bufferWarningAlreadyEmitted ||
177177
++nodeModulesCheckCounter > 10000 ||
178178
(!require('internal/options').getOptionValue('--pending-deprecation') &&
179-
isInsideNodeModules())) {
179+
isInsideNodeModules(100, true))) {
180180
// We don't emit a warning, because we either:
181181
// - Already did so, or
182182
// - Already checked too many times whether a call is coming
183183
// from node_modules and want to stop slowing down things, or
184184
// - We aren't running with `--pending-deprecation` enabled,
185185
// and the code is inside `node_modules`.
186+
// - We found node_modules in up to the topmost 100 frames, or
187+
// there are more than 100 frames and we don't want to search anymore.
186188
return;
187189
}
188190

Collapse file

‎lib/internal/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/util.js
-29Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
const {
44
ArrayBufferPrototypeGetByteLength,
55
ArrayFrom,
6-
ArrayIsArray,
76
ArrayPrototypePush,
87
ArrayPrototypeSlice,
98
ArrayPrototypeSort,
@@ -35,9 +34,7 @@ const {
3534
SafeSet,
3635
SafeWeakMap,
3736
SafeWeakRef,
38-
StringPrototypeIncludes,
3937
StringPrototypeReplace,
40-
StringPrototypeStartsWith,
4138
StringPrototypeToLowerCase,
4239
StringPrototypeToUpperCase,
4340
Symbol,
@@ -509,31 +506,6 @@ function getStructuredStack() {
509506
return getStructuredStackImpl();
510507
}
511508

512-
function isInsideNodeModules() {
513-
const stack = getStructuredStack();
514-
515-
// Iterate over all stack frames and look for the first one not coming
516-
// from inside Node.js itself:
517-
if (ArrayIsArray(stack)) {
518-
for (const frame of stack) {
519-
const filename = frame.getFileName();
520-
521-
if (
522-
filename == null ||
523-
StringPrototypeStartsWith(filename, 'node:') === true ||
524-
(
525-
filename[0] !== '/' &&
526-
StringPrototypeIncludes(filename, '\\') === false
527-
)
528-
) {
529-
continue;
530-
}
531-
return isUnderNodeModules(filename);
532-
}
533-
}
534-
return false;
535-
}
536-
537509
function once(callback, { preserveReturnValue = false } = kEmptyObject) {
538510
let called = false;
539511
let returnValue;
@@ -916,7 +888,6 @@ module.exports = {
916888
guessHandleType,
917889
isArrayBufferDetached,
918890
isError,
919-
isInsideNodeModules,
920891
isUnderNodeModules,
921892
join,
922893
lazyDOMException,
Collapse file

‎src/node_util.cc‎

Copy file name to clipboardExpand all lines: src/node_util.cc
+50Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,54 @@ static void ParseEnv(const FunctionCallbackInfo<Value>& args) {
254254
args.GetReturnValue().Set(dotenv.ToObject(env));
255255
}
256256

257+
bool starts_with(std::string_view view, std::string_view prefix) {
258+
return view.size() >= prefix.size() &&
259+
view.substr(0, prefix.size()) == prefix;
260+
}
261+
262+
static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {
263+
Isolate* isolate = args.GetIsolate();
264+
CHECK_EQ(args.Length(), 2);
265+
CHECK(args[0]->IsInt32()); // frame_limit
266+
// The second argument is the default value.
267+
268+
int frames_limit = args[0].As<v8::Int32>()->Value();
269+
Local<StackTrace> stack =
270+
StackTrace::CurrentStackTrace(isolate, frames_limit);
271+
int frame_count = stack->GetFrameCount();
272+
273+
// If the search requires looking into more than |frames_limit| frames, give
274+
// up and return the specified default value.
275+
if (frame_count == frames_limit) {
276+
return args.GetReturnValue().Set(args[1]);
277+
}
278+
279+
bool result = false;
280+
for (int i = 0; i < frame_count; ++i) {
281+
Local<StackFrame> stack_frame = stack->GetFrame(isolate, i);
282+
Local<String> script_name = stack_frame->GetScriptName();
283+
284+
if (script_name.IsEmpty() || script_name->Length() == 0) {
285+
continue;
286+
}
287+
Utf8Value script_name_utf8(isolate, script_name);
288+
std::string_view script_name_str = script_name_utf8.ToStringView();
289+
if (starts_with(script_name_str,
290+
"node:")) { // Ported to work with C++17 on 20.x.
291+
continue;
292+
}
293+
if (script_name_str.find("/node_modules/") != std::string::npos ||
294+
script_name_str.find("\\node_modules\\") != std::string::npos ||
295+
script_name_str.find("/node_modules\\") != std::string::npos ||
296+
script_name_str.find("\\node_modules/") != std::string::npos) {
297+
result = true;
298+
break;
299+
}
300+
}
301+
302+
args.GetReturnValue().Set(result);
303+
}
304+
257305
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
258306
registry->Register(GetPromiseDetails);
259307
registry->Register(GetProxyDetails);
@@ -269,6 +317,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
269317
registry->Register(FastGuessHandleType);
270318
registry->Register(fast_guess_handle_type_.GetTypeInfo());
271319
registry->Register(ParseEnv);
320+
registry->Register(IsInsideNodeModules);
272321
}
273322

274323
void Initialize(Local<Object> target,
@@ -338,6 +387,7 @@ void Initialize(Local<Object> target,
338387
target->Set(context, env->constants_string(), constants).Check();
339388
}
340389

390+
SetMethod(context, target, "isInsideNodeModules", IsInsideNodeModules);
341391
SetMethodNoSideEffect(
342392
context, target, "getPromiseDetails", GetPromiseDetails);
343393
SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails);
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/warning_node_modules/new-buffer-cjs.js
+1Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/warning_node_modules/new-buffer-esm.mjs
+1Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/warning_node_modules/node_modules/new-buffer-cjs/index.js
+1Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/warning_node_modules/node_modules/new-buffer-cjs/package.json
+3Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/warning_node_modules/node_modules/new-buffer-esm/index.js
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/warning_node_modules/node_modules/new-buffer-esm/package.json
+4Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboard
+48Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fixtures = require('../common/fixtures');
5+
const { spawnSyncAndAssert } = require('../common/child_process');
6+
7+
if (process.env.NODE_PENDING_DEPRECATION)
8+
common.skip('test does not work when NODE_PENDING_DEPRECATION is set');
9+
10+
spawnSyncAndAssert(
11+
process.execPath,
12+
[ fixtures.path('warning_node_modules', 'new-buffer-cjs.js') ],
13+
{
14+
trim: true,
15+
stderr: '',
16+
}
17+
);
18+
19+
spawnSyncAndAssert(
20+
process.execPath,
21+
[ fixtures.path('warning_node_modules', 'new-buffer-esm.mjs') ],
22+
{
23+
trim: true,
24+
stderr: '',
25+
}
26+
);
27+
28+
spawnSyncAndAssert(
29+
process.execPath,
30+
[
31+
'--pending-deprecation',
32+
fixtures.path('warning_node_modules', 'new-buffer-cjs.js'),
33+
],
34+
{
35+
stderr: /DEP0005/
36+
}
37+
);
38+
39+
spawnSyncAndAssert(
40+
process.execPath,
41+
[
42+
'--pending-deprecation',
43+
fixtures.path('warning_node_modules', 'new-buffer-esm.mjs'),
44+
],
45+
{
46+
stderr: /DEP0005/
47+
}
48+
);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.