Skip to content

Commit

Permalink
esm: improve check for ESM syntax
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@GeoffreyBooth @targos
GeoffreyBooth authored and targos committed Oct 23, 2023
1 parent 7740bf8 commit c76eb27
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 65 deletions.
7 changes: 4 additions & 3 deletions lib/internal/modules/cjs/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const {
makeContextifyScript,
runScriptInThisContext,
} = require('internal/vm');
const { containsModuleSyntax } = internalBinding('contextify');

const assert = require('internal/assert');
const fs = require('fs');
Expand All @@ -104,7 +105,6 @@ const {
const {
getCjsConditions,
initializeCjsConditions,
hasEsmSyntax,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
Expand Down Expand Up @@ -1315,7 +1315,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
} catch (err) {
if (process.mainModule === cjsModuleInstance) {
const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(err, content);
enrichCJSError(err, content, filename);
}
throw err;
}
Expand Down Expand Up @@ -1400,10 +1400,11 @@ Module._extensions['.js'] = function(module, filename) {
const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null };
// Function require shouldn't be used in ES modules.
if (pkg.data?.type === 'module') {
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
const parent = moduleParentCache.get(module);
const parentPath = parent?.filename;
const packageJsonPath = path.resolve(pkg.path, 'package.json');
const usesEsm = hasEsmSyntax(content);
const usesEsm = containsModuleSyntax(content, filename);
const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath,
packageJsonPath);
// Attempt to reconstruct the parent require frame.
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/modules/esm/translators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ function lazyTypes() {
return _TYPES = require('internal/util/types');
}

const { containsModuleSyntax } = internalBinding('contextify');
const assert = require('internal/assert');
const { readFileSync } = require('fs');
const { dirname, extname, isAbsolute } = require('path');
const {
hasEsmSyntax,
loadBuiltinModule,
stripBOM,
} = require('internal/modules/helpers');
Expand Down Expand Up @@ -166,11 +166,11 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
* Provide a more informative error for CommonJS imports.
* @param {Error | any} err
* @param {string} [content] Content of the file, if known.
* @param {string} [filename] Useful only if `content` is unknown.
* @param {string} [filename] The filename of the erroring module.
*/
function enrichCJSError(err, content, filename) {
if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) {
containsModuleSyntax(content, filename)) {
// Emit the warning synchronously because we are in the middle of handling
// a SyntaxError that will throw and likely terminate the process before an
// asynchronous warning would be emitted.
Expand Down Expand Up @@ -217,7 +217,7 @@ function loadCJSModule(module, source, url, filename) {
importModuleDynamically, // importModuleDynamically
).function;
} catch (err) {
enrichCJSError(err, source, url);
enrichCJSError(err, source, filename);
throw err;
}

Expand Down Expand Up @@ -344,7 +344,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
assert(module === CJSModule._cache[filename]);
CJSModule._load(filename);
} catch (err) {
enrichCJSError(err, source, url);
enrichCJSError(err, source, filename);
throw err;
}
} : loadCJSModule;
Expand Down
23 changes: 0 additions & 23 deletions lib/internal/modules/helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const {
ArrayPrototypeForEach,
ArrayPrototypeJoin,
ArrayPrototypeSome,
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
SafeMap,
Expand Down Expand Up @@ -299,32 +298,10 @@ function normalizeReferrerURL(referrer) {
return new URL(referrer).href;
}

/**
* For error messages only, check if ESM syntax is in use.
* @param {string} code
*/
function hasEsmSyntax(code) {
debug('Checking for ESM syntax');
const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser;
let root;
try {
root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' });
} catch {
return false;
}

return ArrayPrototypeSome(root.body, (stmt) =>
stmt.type === 'ExportDefaultDeclaration' ||
stmt.type === 'ExportNamedDeclaration' ||
stmt.type === 'ImportDeclaration' ||
stmt.type === 'ExportAllDeclaration');
}

module.exports = {
addBuiltinLibsToObject,
getCjsConditions,
initializeCjsConditions,
hasEsmSyntax,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
Expand Down
208 changes: 174 additions & 34 deletions src/node_contextify.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,13 +318,15 @@ void ContextifyContext::CreatePerIsolateProperties(
SetMethod(isolate, target, "makeContext", MakeContext);
SetMethod(isolate, target, "isContext", IsContext);
SetMethod(isolate, target, "compileFunction", CompileFunction);
SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
}

void ContextifyContext::RegisterExternalReferences(
ExternalReferenceRegistry* registry) {
registry->Register(MakeContext);
registry->Register(IsContext);
registry->Register(CompileFunction);
registry->Register(ContainsModuleSyntax);
registry->Register(PropertyGetterCallback);
registry->Register(PropertySetterCallback);
registry->Register(PropertyDescriptorCallback);
Expand Down Expand Up @@ -1204,33 +1206,18 @@ void ContextifyContext::CompileFunction(
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
}

// Set host_defined_options
Local<PrimitiveArray> host_defined_options =
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
host_defined_options->Set(
isolate, loader::HostDefinedOptions::kID, id_symbol);

ScriptOrigin origin(isolate,
filename,
line_offset, // line offset
column_offset, // column offset
true, // is cross origin
-1, // script id
Local<Value>(), // source map URL
false, // is opaque (?)
false, // is WASM
false, // is ES Module
host_defined_options);

ScriptCompiler::Source source(code, origin, cached_data);
ScriptCompiler::CompileOptions options;
if (source.GetCachedData() == nullptr) {
options = ScriptCompiler::kNoCompileOptions;
} else {
options = ScriptCompiler::kConsumeCodeCache;
}
GetHostDefinedOptions(isolate, id_symbol);
ScriptCompiler::Source source =
GetCommonJSSourceInstance(isolate,
code,
filename,
line_offset,
column_offset,
host_defined_options,
cached_data);
ScriptCompiler::CompileOptions options = GetCompileOptions(source);

TryCatchScope try_catch(env);
Context::Scope scope(parsing_context);

// Read context extensions from buffer
Expand All @@ -1255,9 +1242,83 @@ void ContextifyContext::CompileFunction(
}
}

TryCatchScope try_catch(env);
Local<Object> result = CompileFunctionAndCacheResult(env,
parsing_context,
&source,
params,
context_extensions,
options,
produce_cached_data,
id_symbol,
try_catch);

if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
try_catch.ReThrow();
return;
}

if (result.IsEmpty()) {
return;
}
args.GetReturnValue().Set(result);
}

Local<PrimitiveArray> ContextifyContext::GetHostDefinedOptions(
Isolate* isolate, Local<Symbol> id_symbol) {
Local<PrimitiveArray> host_defined_options =
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
host_defined_options->Set(
isolate, loader::HostDefinedOptions::kID, id_symbol);
return host_defined_options;
}

ScriptCompiler::Source ContextifyContext::GetCommonJSSourceInstance(
Isolate* isolate,
Local<String> code,
Local<String> filename,
int line_offset,
int column_offset,
Local<PrimitiveArray> host_defined_options,
ScriptCompiler::CachedData* cached_data) {
ScriptOrigin origin(isolate,
filename,
line_offset, // line offset
column_offset, // column offset
true, // is cross origin
-1, // script id
Local<Value>(), // source map URL
false, // is opaque (?)
false, // is WASM
false, // is ES Module
host_defined_options);
return ScriptCompiler::Source(code, origin, cached_data);
}

ScriptCompiler::CompileOptions ContextifyContext::GetCompileOptions(
const ScriptCompiler::Source& source) {
ScriptCompiler::CompileOptions options;
if (source.GetCachedData() != nullptr) {
options = ScriptCompiler::kConsumeCodeCache;
} else {
options = ScriptCompiler::kNoCompileOptions;
}
return options;
}

Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
Environment* env,
Local<Context> parsing_context,
ScriptCompiler::Source* source,
std::vector<Local<String>> params,
std::vector<Local<Object>> context_extensions,
ScriptCompiler::CompileOptions options,
bool produce_cached_data,
Local<Symbol> id_symbol,
const TryCatchScope& try_catch) {
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunction(
parsing_context,
&source,
source,
params.size(),
params.data(),
context_extensions.size(),
Expand All @@ -1269,24 +1330,26 @@ void ContextifyContext::CompileFunction(
if (!maybe_fn.ToLocal(&fn)) {
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
errors::DecorateErrorStack(env, try_catch);
try_catch.ReThrow();
return Object::New(env->isolate());
}
return;
}

Local<Context> context = env->context();
if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol)
.IsNothing()) {
return;
return Object::New(env->isolate());
}

Isolate* isolate = env->isolate();
Local<Object> result = Object::New(isolate);
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
return;
return Object::New(env->isolate());
if (result
->Set(parsing_context,
env->source_map_url_string(),
fn->GetScriptOrigin().SourceMapUrl())
.IsNothing())
return;
return Object::New(env->isolate());

std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
if (produce_cached_data) {
Expand All @@ -1295,14 +1358,91 @@ void ContextifyContext::CompileFunction(
if (StoreCodeCacheResult(env,
result,
options,
source,
*source,
produce_cached_data,
std::move(new_cached_data))
.IsNothing()) {
return;
return Object::New(env->isolate());
}

args.GetReturnValue().Set(result);
return result;
}

// When compiling as CommonJS source code that contains ESM syntax, the
// following error messages are returned:
// - `import` statements: "Cannot use import statement outside a module"
// - `export` statements: "Unexpected token 'export'"
// - `import.meta` references: "Cannot use 'import.meta' outside a module"
// Dynamic `import()` is permitted in CommonJS, so it does not error.
// While top-level `await` is not permitted in CommonJS, it returns the same
// error message as when `await` is used in a sync function, so we don't use it
// as a disambiguation.
constexpr std::array<std::string_view, 3> esm_syntax_error_messages = {
"Cannot use import statement outside a module", // `import` statements
"Unexpected token 'export'", // `export` statements
"Cannot use 'import.meta' outside a module"}; // `import.meta` references

void ContextifyContext::ContainsModuleSyntax(
const FunctionCallbackInfo<Value>& args) {
// Argument 1: source code
CHECK(args[0]->IsString());
Local<String> code = args[0].As<String>();

// Argument 2: filename
Local<String> filename = String::Empty(args.GetIsolate());
if (!args[1]->IsUndefined()) {
CHECK(args[1]->IsString());
filename = args[1].As<String>();
}

Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();
Local<Context> context = env->context();

// TODO(geoffreybooth): Centralize this rather than matching the logic in
// cjs/loader.js and translators.js
Local<String> script_id = String::Concat(
isolate, String::NewFromUtf8(isolate, "cjs:").ToLocalChecked(), filename);
Local<Symbol> id_symbol = Symbol::New(isolate, script_id);

Local<PrimitiveArray> host_defined_options =
GetHostDefinedOptions(isolate, id_symbol);
ScriptCompiler::Source source = GetCommonJSSourceInstance(
isolate, code, filename, 0, 0, host_defined_options, nullptr);
ScriptCompiler::CompileOptions options = GetCompileOptions(source);

std::vector<Local<String>> params = {
String::NewFromUtf8(isolate, "exports").ToLocalChecked(),
String::NewFromUtf8(isolate, "require").ToLocalChecked(),
String::NewFromUtf8(isolate, "module").ToLocalChecked(),
String::NewFromUtf8(isolate, "__filename").ToLocalChecked(),
String::NewFromUtf8(isolate, "__dirname").ToLocalChecked()};

TryCatchScope try_catch(env);

ContextifyContext::CompileFunctionAndCacheResult(env,
context,
&source,
params,
std::vector<Local<Object>>(),
options,
true,
id_symbol,
try_catch);

bool found_error_message_caused_by_module_syntax = false;
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
Utf8Value message_value(env->isolate(), try_catch.Message()->Get());
auto message = message_value.ToStringView();

for (const auto& error_message : esm_syntax_error_messages) {
if (message.find(error_message) != std::string_view::npos) {
found_error_message_caused_by_module_syntax = true;
break;
}
}
}
args.GetReturnValue().Set(found_error_message_caused_by_module_syntax);
}

static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {
Expand Down
Loading

0 comments on commit c76eb27

Please sign in to comment.