Skip to content

Commit c64490e

Browse files
GeoffreyBoothtargos
GeoffreyBooth
authored and
targos
committed
esm: improve check for ESM syntax
PR-URL: #50127 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 16a0798 commit c64490e

File tree

5 files changed

+207
-65
lines changed

5 files changed

+207
-65
lines changed

‎lib/internal/modules/cjs/loader.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const {
9090
makeContextifyScript,
9191
runScriptInThisContext,
9292
} = require('internal/vm');
93+
const { containsModuleSyntax } = internalBinding('contextify');
9394

9495
const assert = require('internal/assert');
9596
const fs = require('fs');
@@ -104,7 +105,6 @@ const {
104105
const {
105106
getCjsConditions,
106107
initializeCjsConditions,
107-
hasEsmSyntax,
108108
loadBuiltinModule,
109109
makeRequireFunction,
110110
normalizeReferrerURL,
@@ -1315,7 +1315,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
13151315
} catch (err) {
13161316
if (process.mainModule === cjsModuleInstance) {
13171317
const { enrichCJSError } = require('internal/modules/esm/translators');
1318-
enrichCJSError(err, content);
1318+
enrichCJSError(err, content, filename);
13191319
}
13201320
throw err;
13211321
}
@@ -1400,10 +1400,11 @@ Module._extensions['.js'] = function(module, filename) {
14001400
const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null };
14011401
// Function require shouldn't be used in ES modules.
14021402
if (pkg.data?.type === 'module') {
1403+
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
14031404
const parent = moduleParentCache.get(module);
14041405
const parentPath = parent?.filename;
14051406
const packageJsonPath = path.resolve(pkg.path, 'package.json');
1406-
const usesEsm = hasEsmSyntax(content);
1407+
const usesEsm = containsModuleSyntax(content, filename);
14071408
const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath,
14081409
packageJsonPath);
14091410
// Attempt to reconstruct the parent require frame.

‎lib/internal/modules/esm/translators.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ function lazyTypes() {
3030
return _TYPES = require('internal/util/types');
3131
}
3232

33+
const { containsModuleSyntax } = internalBinding('contextify');
3334
const assert = require('internal/assert');
3435
const { readFileSync } = require('fs');
3536
const { dirname, extname, isAbsolute } = require('path');
3637
const {
37-
hasEsmSyntax,
3838
loadBuiltinModule,
3939
stripBOM,
4040
} = require('internal/modules/helpers');
@@ -166,11 +166,11 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
166166
* Provide a more informative error for CommonJS imports.
167167
* @param {Error | any} err
168168
* @param {string} [content] Content of the file, if known.
169-
* @param {string} [filename] Useful only if `content` is unknown.
169+
* @param {string} [filename] The filename of the erroring module.
170170
*/
171171
function enrichCJSError(err, content, filename) {
172172
if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
173-
hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) {
173+
containsModuleSyntax(content, filename)) {
174174
// Emit the warning synchronously because we are in the middle of handling
175175
// a SyntaxError that will throw and likely terminate the process before an
176176
// asynchronous warning would be emitted.
@@ -217,7 +217,7 @@ function loadCJSModule(module, source, url, filename) {
217217
importModuleDynamically, // importModuleDynamically
218218
).function;
219219
} catch (err) {
220-
enrichCJSError(err, source, url);
220+
enrichCJSError(err, source, filename);
221221
throw err;
222222
}
223223

@@ -344,7 +344,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
344344
assert(module === CJSModule._cache[filename]);
345345
CJSModule._load(filename);
346346
} catch (err) {
347-
enrichCJSError(err, source, url);
347+
enrichCJSError(err, source, filename);
348348
throw err;
349349
}
350350
} : loadCJSModule;

‎lib/internal/modules/helpers.js

-23
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
const {
44
ArrayPrototypeForEach,
55
ArrayPrototypeJoin,
6-
ArrayPrototypeSome,
76
ObjectDefineProperty,
87
ObjectPrototypeHasOwnProperty,
98
SafeMap,
@@ -299,32 +298,10 @@ function normalizeReferrerURL(referrer) {
299298
return new URL(referrer).href;
300299
}
301300

302-
/**
303-
* For error messages only, check if ESM syntax is in use.
304-
* @param {string} code
305-
*/
306-
function hasEsmSyntax(code) {
307-
debug('Checking for ESM syntax');
308-
const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser;
309-
let root;
310-
try {
311-
root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' });
312-
} catch {
313-
return false;
314-
}
315-
316-
return ArrayPrototypeSome(root.body, (stmt) =>
317-
stmt.type === 'ExportDefaultDeclaration' ||
318-
stmt.type === 'ExportNamedDeclaration' ||
319-
stmt.type === 'ImportDeclaration' ||
320-
stmt.type === 'ExportAllDeclaration');
321-
}
322-
323301
module.exports = {
324302
addBuiltinLibsToObject,
325303
getCjsConditions,
326304
initializeCjsConditions,
327-
hasEsmSyntax,
328305
loadBuiltinModule,
329306
makeRequireFunction,
330307
normalizeReferrerURL,

‎src/node_contextify.cc

+174-34
Original file line numberDiff line numberDiff line change
@@ -318,13 +318,15 @@ void ContextifyContext::CreatePerIsolateProperties(
318318
SetMethod(isolate, target, "makeContext", MakeContext);
319319
SetMethod(isolate, target, "isContext", IsContext);
320320
SetMethod(isolate, target, "compileFunction", CompileFunction);
321+
SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
321322
}
322323

323324
void ContextifyContext::RegisterExternalReferences(
324325
ExternalReferenceRegistry* registry) {
325326
registry->Register(MakeContext);
326327
registry->Register(IsContext);
327328
registry->Register(CompileFunction);
329+
registry->Register(ContainsModuleSyntax);
328330
registry->Register(PropertyGetterCallback);
329331
registry->Register(PropertySetterCallback);
330332
registry->Register(PropertyDescriptorCallback);
@@ -1205,33 +1207,18 @@ void ContextifyContext::CompileFunction(
12051207
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
12061208
}
12071209

1208-
// Set host_defined_options
12091210
Local<PrimitiveArray> host_defined_options =
1210-
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
1211-
host_defined_options->Set(
1212-
isolate, loader::HostDefinedOptions::kID, id_symbol);
1213-
1214-
ScriptOrigin origin(isolate,
1215-
filename,
1216-
line_offset, // line offset
1217-
column_offset, // column offset
1218-
true, // is cross origin
1219-
-1, // script id
1220-
Local<Value>(), // source map URL
1221-
false, // is opaque (?)
1222-
false, // is WASM
1223-
false, // is ES Module
1224-
host_defined_options);
1225-
1226-
ScriptCompiler::Source source(code, origin, cached_data);
1227-
ScriptCompiler::CompileOptions options;
1228-
if (source.GetCachedData() == nullptr) {
1229-
options = ScriptCompiler::kNoCompileOptions;
1230-
} else {
1231-
options = ScriptCompiler::kConsumeCodeCache;
1232-
}
1211+
GetHostDefinedOptions(isolate, id_symbol);
1212+
ScriptCompiler::Source source =
1213+
GetCommonJSSourceInstance(isolate,
1214+
code,
1215+
filename,
1216+
line_offset,
1217+
column_offset,
1218+
host_defined_options,
1219+
cached_data);
1220+
ScriptCompiler::CompileOptions options = GetCompileOptions(source);
12331221

1234-
TryCatchScope try_catch(env);
12351222
Context::Scope scope(parsing_context);
12361223

12371224
// Read context extensions from buffer
@@ -1256,9 +1243,83 @@ void ContextifyContext::CompileFunction(
12561243
}
12571244
}
12581245

1246+
TryCatchScope try_catch(env);
1247+
Local<Object> result = CompileFunctionAndCacheResult(env,
1248+
parsing_context,
1249+
&source,
1250+
params,
1251+
context_extensions,
1252+
options,
1253+
produce_cached_data,
1254+
id_symbol,
1255+
try_catch);
1256+
1257+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
1258+
try_catch.ReThrow();
1259+
return;
1260+
}
1261+
1262+
if (result.IsEmpty()) {
1263+
return;
1264+
}
1265+
args.GetReturnValue().Set(result);
1266+
}
1267+
1268+
Local<PrimitiveArray> ContextifyContext::GetHostDefinedOptions(
1269+
Isolate* isolate, Local<Symbol> id_symbol) {
1270+
Local<PrimitiveArray> host_defined_options =
1271+
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
1272+
host_defined_options->Set(
1273+
isolate, loader::HostDefinedOptions::kID, id_symbol);
1274+
return host_defined_options;
1275+
}
1276+
1277+
ScriptCompiler::Source ContextifyContext::GetCommonJSSourceInstance(
1278+
Isolate* isolate,
1279+
Local<String> code,
1280+
Local<String> filename,
1281+
int line_offset,
1282+
int column_offset,
1283+
Local<PrimitiveArray> host_defined_options,
1284+
ScriptCompiler::CachedData* cached_data) {
1285+
ScriptOrigin origin(isolate,
1286+
filename,
1287+
line_offset, // line offset
1288+
column_offset, // column offset
1289+
true, // is cross origin
1290+
-1, // script id
1291+
Local<Value>(), // source map URL
1292+
false, // is opaque (?)
1293+
false, // is WASM
1294+
false, // is ES Module
1295+
host_defined_options);
1296+
return ScriptCompiler::Source(code, origin, cached_data);
1297+
}
1298+
1299+
ScriptCompiler::CompileOptions ContextifyContext::GetCompileOptions(
1300+
const ScriptCompiler::Source& source) {
1301+
ScriptCompiler::CompileOptions options;
1302+
if (source.GetCachedData() != nullptr) {
1303+
options = ScriptCompiler::kConsumeCodeCache;
1304+
} else {
1305+
options = ScriptCompiler::kNoCompileOptions;
1306+
}
1307+
return options;
1308+
}
1309+
1310+
Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
1311+
Environment* env,
1312+
Local<Context> parsing_context,
1313+
ScriptCompiler::Source* source,
1314+
std::vector<Local<String>> params,
1315+
std::vector<Local<Object>> context_extensions,
1316+
ScriptCompiler::CompileOptions options,
1317+
bool produce_cached_data,
1318+
Local<Symbol> id_symbol,
1319+
const TryCatchScope& try_catch) {
12591320
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunction(
12601321
parsing_context,
1261-
&source,
1322+
source,
12621323
params.size(),
12631324
params.data(),
12641325
context_extensions.size(),
@@ -1270,24 +1331,26 @@ void ContextifyContext::CompileFunction(
12701331
if (!maybe_fn.ToLocal(&fn)) {
12711332
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
12721333
errors::DecorateErrorStack(env, try_catch);
1273-
try_catch.ReThrow();
1334+
return Object::New(env->isolate());
12741335
}
1275-
return;
12761336
}
1337+
1338+
Local<Context> context = env->context();
12771339
if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol)
12781340
.IsNothing()) {
1279-
return;
1341+
return Object::New(env->isolate());
12801342
}
12811343

1344+
Isolate* isolate = env->isolate();
12821345
Local<Object> result = Object::New(isolate);
12831346
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
1284-
return;
1347+
return Object::New(env->isolate());
12851348
if (result
12861349
->Set(parsing_context,
12871350
env->source_map_url_string(),
12881351
fn->GetScriptOrigin().SourceMapUrl())
12891352
.IsNothing())
1290-
return;
1353+
return Object::New(env->isolate());
12911354

12921355
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
12931356
if (produce_cached_data) {
@@ -1296,14 +1359,91 @@ void ContextifyContext::CompileFunction(
12961359
if (StoreCodeCacheResult(env,
12971360
result,
12981361
options,
1299-
source,
1362+
*source,
13001363
produce_cached_data,
13011364
std::move(new_cached_data))
13021365
.IsNothing()) {
1303-
return;
1366+
return Object::New(env->isolate());
13041367
}
13051368

1306-
args.GetReturnValue().Set(result);
1369+
return result;
1370+
}
1371+
1372+
// When compiling as CommonJS source code that contains ESM syntax, the
1373+
// following error messages are returned:
1374+
// - `import` statements: "Cannot use import statement outside a module"
1375+
// - `export` statements: "Unexpected token 'export'"
1376+
// - `import.meta` references: "Cannot use 'import.meta' outside a module"
1377+
// Dynamic `import()` is permitted in CommonJS, so it does not error.
1378+
// While top-level `await` is not permitted in CommonJS, it returns the same
1379+
// error message as when `await` is used in a sync function, so we don't use it
1380+
// as a disambiguation.
1381+
constexpr std::array<std::string_view, 3> esm_syntax_error_messages = {
1382+
"Cannot use import statement outside a module", // `import` statements
1383+
"Unexpected token 'export'", // `export` statements
1384+
"Cannot use 'import.meta' outside a module"}; // `import.meta` references
1385+
1386+
void ContextifyContext::ContainsModuleSyntax(
1387+
const FunctionCallbackInfo<Value>& args) {
1388+
// Argument 1: source code
1389+
CHECK(args[0]->IsString());
1390+
Local<String> code = args[0].As<String>();
1391+
1392+
// Argument 2: filename
1393+
Local<String> filename = String::Empty(args.GetIsolate());
1394+
if (!args[1]->IsUndefined()) {
1395+
CHECK(args[1]->IsString());
1396+
filename = args[1].As<String>();
1397+
}
1398+
1399+
Environment* env = Environment::GetCurrent(args);
1400+
Isolate* isolate = env->isolate();
1401+
Local<Context> context = env->context();
1402+
1403+
// TODO(geoffreybooth): Centralize this rather than matching the logic in
1404+
// cjs/loader.js and translators.js
1405+
Local<String> script_id = String::Concat(
1406+
isolate, String::NewFromUtf8(isolate, "cjs:").ToLocalChecked(), filename);
1407+
Local<Symbol> id_symbol = Symbol::New(isolate, script_id);
1408+
1409+
Local<PrimitiveArray> host_defined_options =
1410+
GetHostDefinedOptions(isolate, id_symbol);
1411+
ScriptCompiler::Source source = GetCommonJSSourceInstance(
1412+
isolate, code, filename, 0, 0, host_defined_options, nullptr);
1413+
ScriptCompiler::CompileOptions options = GetCompileOptions(source);
1414+
1415+
std::vector<Local<String>> params = {
1416+
String::NewFromUtf8(isolate, "exports").ToLocalChecked(),
1417+
String::NewFromUtf8(isolate, "require").ToLocalChecked(),
1418+
String::NewFromUtf8(isolate, "module").ToLocalChecked(),
1419+
String::NewFromUtf8(isolate, "__filename").ToLocalChecked(),
1420+
String::NewFromUtf8(isolate, "__dirname").ToLocalChecked()};
1421+
1422+
TryCatchScope try_catch(env);
1423+
1424+
ContextifyContext::CompileFunctionAndCacheResult(env,
1425+
context,
1426+
&source,
1427+
params,
1428+
std::vector<Local<Object>>(),
1429+
options,
1430+
true,
1431+
id_symbol,
1432+
try_catch);
1433+
1434+
bool found_error_message_caused_by_module_syntax = false;
1435+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
1436+
Utf8Value message_value(env->isolate(), try_catch.Message()->Get());
1437+
auto message = message_value.ToStringView();
1438+
1439+
for (const auto& error_message : esm_syntax_error_messages) {
1440+
if (message.find(error_message) != std::string_view::npos) {
1441+
found_error_message_caused_by_module_syntax = true;
1442+
break;
1443+
}
1444+
}
1445+
}
1446+
args.GetReturnValue().Set(found_error_message_caused_by_module_syntax);
13071447
}
13081448

13091449
static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {

0 commit comments

Comments
 (0)