Skip to content

Commit 5f8ccec

Browse files
committed
module: revert module._compile to original state if module is patched
PR-URL: #21573 Fixes: #17396 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent d345b0d commit 5f8ccec

13 files changed

+201
-49
lines changed

lib/assert.js

-11
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ let isDeepEqual;
3939
let isDeepStrictEqual;
4040
let parseExpressionAt;
4141
let findNodeAround;
42-
let columnOffset = 0;
4342
let decoder;
4443

4544
function lazyLoadComparison() {
@@ -256,16 +255,6 @@ function getErrMessage(message, fn) {
256255
const line = call.getLineNumber() - 1;
257256
let column = call.getColumnNumber() - 1;
258257

259-
// Line number one reports the wrong column due to being wrapped in a
260-
// function. Remove that offset to get the actual call.
261-
if (line === 0) {
262-
if (columnOffset === 0) {
263-
const { wrapper } = require('internal/modules/cjs/loader');
264-
columnOffset = wrapper[0].length;
265-
}
266-
column -= columnOffset;
267-
}
268-
269258
const identifier = `${filename}${line}${column}`;
270259

271260
if (errorCache.has(identifier)) {

lib/internal/modules/cjs/helpers.js

+11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict';
22

33
const { validateString } = require('internal/validators');
4+
const path = require('path');
5+
const { pathToFileURL } = require('internal/url');
6+
const { URL } = require('url');
47

58
const {
69
CHAR_LINE_FEED,
@@ -145,10 +148,18 @@ function addBuiltinLibsToObject(object) {
145148
});
146149
}
147150

151+
function normalizeReferrerURL(referrer) {
152+
if (typeof referrer === 'string' && path.isAbsolute(referrer)) {
153+
return pathToFileURL(referrer).href;
154+
}
155+
return new URL(referrer).href;
156+
}
157+
148158
module.exports = exports = {
149159
addBuiltinLibsToObject,
150160
builtinLibs,
151161
makeRequireFunction,
162+
normalizeReferrerURL,
152163
requireDepth: 0,
153164
stripBOM,
154165
stripShebang

lib/internal/modules/cjs/loader.js

+83-17
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ const assert = require('internal/assert');
2929
const fs = require('fs');
3030
const internalFS = require('internal/fs/utils');
3131
const path = require('path');
32-
const { URL } = require('url');
3332
const {
3433
internalModuleReadJSON,
3534
internalModuleStat
3635
} = internalBinding('fs');
3736
const { safeGetenv } = internalBinding('credentials');
3837
const {
3938
makeRequireFunction,
39+
normalizeReferrerURL,
4040
requireDepth,
4141
stripBOM,
4242
stripShebang
@@ -48,6 +48,7 @@ const experimentalModules = getOptionValue('--experimental-modules');
4848
const manifest = getOptionValue('--experimental-policy') ?
4949
require('internal/process/policy').manifest :
5050
null;
51+
const { compileFunction } = internalBinding('contextify');
5152

5253
const {
5354
ERR_INVALID_ARG_VALUE,
@@ -129,15 +130,52 @@ Module._extensions = Object.create(null);
129130
var modulePaths = [];
130131
Module.globalPaths = [];
131132

132-
Module.wrap = function(script) {
133+
let patched = false;
134+
135+
// eslint-disable-next-line func-style
136+
let wrap = function(script) {
133137
return Module.wrapper[0] + script + Module.wrapper[1];
134138
};
135139

136-
Module.wrapper = [
140+
const wrapper = [
137141
'(function (exports, require, module, __filename, __dirname) { ',
138142
'\n});'
139143
];
140144

145+
let wrapperProxy = new Proxy(wrapper, {
146+
set(target, property, value, receiver) {
147+
patched = true;
148+
return Reflect.set(target, property, value, receiver);
149+
},
150+
151+
defineProperty(target, property, descriptor) {
152+
patched = true;
153+
return Object.defineProperty(target, property, descriptor);
154+
}
155+
});
156+
157+
Object.defineProperty(Module, 'wrap', {
158+
get() {
159+
return wrap;
160+
},
161+
162+
set(value) {
163+
patched = true;
164+
wrap = value;
165+
}
166+
});
167+
168+
Object.defineProperty(Module, 'wrapper', {
169+
get() {
170+
return wrapperProxy;
171+
},
172+
173+
set(value) {
174+
patched = true;
175+
wrapperProxy = value;
176+
}
177+
});
178+
141179
const debug = util.debuglog('module');
142180

143181
Module._debug = util.deprecate(debug, 'Module._debug is deprecated.',
@@ -680,13 +718,6 @@ Module.prototype.require = function(id) {
680718
// (needed for setting breakpoint when called with --inspect-brk)
681719
var resolvedArgv;
682720

683-
function normalizeReferrerURL(referrer) {
684-
if (typeof referrer === 'string' && path.isAbsolute(referrer)) {
685-
return pathToFileURL(referrer).href;
686-
}
687-
return new URL(referrer).href;
688-
}
689-
690721

691722
// Run the file contents in the correct scope or sandbox. Expose
692723
// the correct helper variables (require, module, exports) to
@@ -700,13 +731,48 @@ Module.prototype._compile = function(content, filename) {
700731

701732
content = stripShebang(content);
702733

703-
const compiledWrapper = vm.compileFunction(content, [
704-
'exports',
705-
'require',
706-
'module',
707-
'__filename',
708-
'__dirname',
709-
], { filename });
734+
let compiledWrapper;
735+
if (patched) {
736+
const wrapper = Module.wrap(content);
737+
compiledWrapper = vm.runInThisContext(wrapper, {
738+
filename,
739+
lineOffset: 0,
740+
displayErrors: true,
741+
importModuleDynamically: experimentalModules ? async (specifier) => {
742+
if (asyncESM === undefined) lazyLoadESM();
743+
const loader = await asyncESM.loaderPromise;
744+
return loader.import(specifier, normalizeReferrerURL(filename));
745+
} : undefined,
746+
});
747+
} else {
748+
compiledWrapper = compileFunction(
749+
content,
750+
filename,
751+
0,
752+
0,
753+
undefined,
754+
false,
755+
undefined,
756+
[],
757+
[
758+
'exports',
759+
'require',
760+
'module',
761+
'__filename',
762+
'__dirname',
763+
]
764+
);
765+
if (experimentalModules) {
766+
const { callbackMap } = internalBinding('module_wrap');
767+
callbackMap.set(compiledWrapper, {
768+
importModuleDynamically: async (specifier) => {
769+
if (asyncESM === undefined) lazyLoadESM();
770+
const loader = await asyncESM.loaderPromise;
771+
return loader.import(specifier, normalizeReferrerURL(filename));
772+
}
773+
});
774+
}
775+
}
710776

711777
var inspectorWrapper = null;
712778
if (process._breakFirstLine && process._eval == null) {

src/env-inl.h

+3
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,9 @@ inline uint32_t Environment::get_next_module_id() {
461461
inline uint32_t Environment::get_next_script_id() {
462462
return script_id_counter_++;
463463
}
464+
inline uint32_t Environment::get_next_function_id() {
465+
return function_id_counter_++;
466+
}
464467

465468
Environment::ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope(
466469
Environment* env)

src/env.cc

+11
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ Environment::Environment(IsolateData* isolate_data,
252252
isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
253253
}
254254

255+
CompileFnEntry::CompileFnEntry(Environment* env, uint32_t id)
256+
: env(env), id(id) {
257+
env->compile_fn_entries.insert(this);
258+
}
259+
255260
Environment::~Environment() {
256261
isolate()->GetHeapProfiler()->RemoveBuildEmbedderGraphCallback(
257262
BuildEmbedderGraph, this);
@@ -260,6 +265,12 @@ Environment::~Environment() {
260265
// CleanupHandles() should have removed all of them.
261266
CHECK(file_handle_read_wrap_freelist_.empty());
262267

268+
// dispose the Persistent references to the compileFunction
269+
// wrappers used in the dynamic import callback
270+
for (auto& entry : compile_fn_entries) {
271+
delete entry;
272+
}
273+
263274
HandleScope handle_scope(isolate());
264275

265276
#if HAVE_INSPECTOR

src/env.h

+10
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,12 @@ struct ContextInfo {
450450
bool is_default = false;
451451
};
452452

453+
struct CompileFnEntry {
454+
Environment* env;
455+
uint32_t id;
456+
CompileFnEntry(Environment* env, uint32_t id);
457+
};
458+
453459
// Listing the AsyncWrap provider types first enables us to cast directly
454460
// from a provider type to a debug category.
455461
#define DEBUG_CATEGORY_NAMES(V) \
@@ -720,9 +726,12 @@ class Environment {
720726
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
721727
std::unordered_map<uint32_t, contextify::ContextifyScript*>
722728
id_to_script_map;
729+
std::unordered_set<CompileFnEntry*> compile_fn_entries;
730+
std::unordered_map<uint32_t, Persistent<v8::Function>> id_to_function_map;
723731

724732
inline uint32_t get_next_module_id();
725733
inline uint32_t get_next_script_id();
734+
inline uint32_t get_next_function_id();
726735

727736
std::unordered_map<std::string, const loader::PackageConfig>
728737
package_json_cache;
@@ -1006,6 +1015,7 @@ class Environment {
10061015

10071016
uint32_t module_id_counter_ = 0;
10081017
uint32_t script_id_counter_ = 0;
1018+
uint32_t function_id_counter_ = 0;
10091019

10101020
AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
10111021
int should_not_abort_scope_counter_ = 0;

src/module_wrap.cc

+2
Original file line numberDiff line numberDiff line change
@@ -752,6 +752,8 @@ static MaybeLocal<Promise> ImportModuleDynamically(
752752
} else if (type == ScriptType::kModule) {
753753
ModuleWrap* wrap = ModuleWrap::GetFromID(env, id);
754754
object = wrap->object();
755+
} else if (type == ScriptType::kFunction) {
756+
object = env->id_to_function_map.find(id)->second.Get(iso);
755757
} else {
756758
UNREACHABLE();
757759
}

src/module_wrap.h

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ enum PackageMainCheck : bool {
2020
enum ScriptType : int {
2121
kScript,
2222
kModule,
23+
kFunction,
2324
};
2425

2526
enum HostDefinedOptions : int {

src/node_contextify.cc

+47-8
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,15 @@ void ContextifyContext::WeakCallback(
285285
delete context;
286286
}
287287

288+
void ContextifyContext::WeakCallbackCompileFn(
289+
const WeakCallbackInfo<CompileFnEntry>& data) {
290+
CompileFnEntry* entry = data.GetParameter();
291+
if (entry->env->compile_fn_entries.erase(entry) != 0) {
292+
entry->env->id_to_function_map.erase(entry->id);
293+
delete entry;
294+
}
295+
}
296+
288297
// static
289298
ContextifyContext* ContextifyContext::ContextFromContextifiedSandbox(
290299
Environment* env,
@@ -1027,7 +1036,30 @@ void ContextifyContext::CompileFunction(
10271036
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
10281037
}
10291038

1030-
ScriptOrigin origin(filename, line_offset, column_offset, True(isolate));
1039+
// Get the function id
1040+
uint32_t id = env->get_next_function_id();
1041+
1042+
// Set host_defined_options
1043+
Local<PrimitiveArray> host_defined_options =
1044+
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
1045+
host_defined_options->Set(
1046+
isolate,
1047+
loader::HostDefinedOptions::kType,
1048+
Number::New(isolate, loader::ScriptType::kFunction));
1049+
host_defined_options->Set(
1050+
isolate, loader::HostDefinedOptions::kID, Number::New(isolate, id));
1051+
1052+
ScriptOrigin origin(filename,
1053+
line_offset, // line offset
1054+
column_offset, // column offset
1055+
True(isolate), // is cross origin
1056+
Local<Integer>(), // script id
1057+
Local<Value>(), // source map URL
1058+
False(isolate), // is opaque (?)
1059+
False(isolate), // is WASM
1060+
False(isolate), // is ES Module
1061+
host_defined_options);
1062+
10311063
ScriptCompiler::Source source(code, origin, cached_data);
10321064
ScriptCompiler::CompileOptions options;
10331065
if (source.GetCachedData() == nullptr) {
@@ -1061,38 +1093,45 @@ void ContextifyContext::CompileFunction(
10611093
}
10621094
}
10631095

1064-
MaybeLocal<Function> maybe_fun = ScriptCompiler::CompileFunctionInContext(
1096+
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunctionInContext(
10651097
parsing_context, &source, params.size(), params.data(),
10661098
context_extensions.size(), context_extensions.data(), options);
10671099

1068-
Local<Function> fun;
1069-
if (maybe_fun.IsEmpty() || !maybe_fun.ToLocal(&fun)) {
1100+
if (maybe_fn.IsEmpty()) {
10701101
DecorateErrorStack(env, try_catch);
10711102
try_catch.ReThrow();
10721103
return;
10731104
}
1105+
Local<Function> fn = maybe_fn.ToLocalChecked();
1106+
env->id_to_function_map.emplace(std::piecewise_construct,
1107+
std::make_tuple(id),
1108+
std::make_tuple(isolate, fn));
1109+
CompileFnEntry* gc_entry = new CompileFnEntry(env, id);
1110+
env->id_to_function_map[id].SetWeak(gc_entry,
1111+
WeakCallbackCompileFn,
1112+
v8::WeakCallbackType::kParameter);
10741113

10751114
if (produce_cached_data) {
10761115
const std::unique_ptr<ScriptCompiler::CachedData> cached_data(
1077-
ScriptCompiler::CreateCodeCacheForFunction(fun));
1116+
ScriptCompiler::CreateCodeCacheForFunction(fn));
10781117
bool cached_data_produced = cached_data != nullptr;
10791118
if (cached_data_produced) {
10801119
MaybeLocal<Object> buf = Buffer::Copy(
10811120
env,
10821121
reinterpret_cast<const char*>(cached_data->data),
10831122
cached_data->length);
1084-
if (fun->Set(
1123+
if (fn->Set(
10851124
parsing_context,
10861125
env->cached_data_string(),
10871126
buf.ToLocalChecked()).IsNothing()) return;
10881127
}
1089-
if (fun->Set(
1128+
if (fn->Set(
10901129
parsing_context,
10911130
env->cached_data_produced_string(),
10921131
Boolean::New(isolate, cached_data_produced)).IsNothing()) return;
10931132
}
10941133

1095-
args.GetReturnValue().Set(fun);
1134+
args.GetReturnValue().Set(fn);
10961135
}
10971136

10981137

src/node_contextify.h

+2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ class ContextifyContext {
6161
const v8::FunctionCallbackInfo<v8::Value>& args);
6262
static void WeakCallback(
6363
const v8::WeakCallbackInfo<ContextifyContext>& data);
64+
static void WeakCallbackCompileFn(
65+
const v8::WeakCallbackInfo<CompileFnEntry>& data);
6466
static void PropertyGetterCallback(
6567
v8::Local<v8::Name> property,
6668
const v8::PropertyCallbackInfo<v8::Value>& args);

0 commit comments

Comments
 (0)