Skip to content

Commit

Permalink
improve validation failure errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@guybedford
guybedford committed Aug 6, 2019
1 parent 9d95b1d commit a7e3e76
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 41 deletions.
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
}
}
// eslint-disable-next-line no-restricted-syntax
const e = new Error(`Package exports for '${basePath}' do not define ` +
`a '${mappingKey}' subpath`);
const e = new Error(`Package exports for '${basePath}' do not define a ` +
`valid '${mappingKey}' target${subpath ? 'for ' + subpath : ''}`);
e.code = 'MODULE_NOT_FOUND';
throw e;
}
Expand Down
153 changes: 114 additions & 39 deletions src/module_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,8 @@ Maybe<const PackageConfig*> GetPackageConfig(Environment* env,
if (existing != env->package_json_cache.end()) {
const PackageConfig* pcfg = &existing->second;
if (pcfg->is_valid == IsValid::No) {
std::string msg = "Invalid JSON in '" + path +
"' imported from " + base.ToFilePath();
std::string msg = "Invalid JSON in " + path +
" imported from " + base.ToFilePath();
node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str());
return Nothing<const PackageConfig*>();
}
Expand Down Expand Up @@ -579,8 +579,8 @@ Maybe<const PackageConfig*> GetPackageConfig(Environment* env,
env->package_json_cache.emplace(path,
PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "",
PackageType::None, Global<Value>() });
std::string msg = "Invalid JSON in '" + path +
"' imported from " + base.ToFilePath();
std::string msg = "Invalid JSON in " + path +
" imported from " + base.ToFilePath();
node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str());
return Nothing<const PackageConfig*>();
}
Expand Down Expand Up @@ -755,17 +755,17 @@ Maybe<URL> FinalizeResolution(Environment* env,
if (!file.IsNothing()) {
return file;
}
std::string msg = "Cannot find module '" + resolved.path() +
"' imported from " + base.ToFilePath();
std::string msg = "Cannot find module " + resolved.path() +
" imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}

const std::string& path = resolved.ToFilePath();
if (CheckDescriptorAtPath(path) != FILE) {
std::string msg = "Cannot find module '" +
std::string msg = "Cannot find module " +
(path.length() != 0 ? path : resolved.path()) +
"' imported from " + base.ToFilePath();
" imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}
Expand Down Expand Up @@ -799,51 +799,92 @@ Maybe<URL> PackageMainResolve(Environment* env,
}
}
}
std::string msg = "Cannot find main entry point for '" +
URL(".", pjson_url).ToFilePath() + "' imported from " +
std::string msg = "Cannot find main entry point for " +
URL(".", pjson_url).ToFilePath() + " imported from " +
base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}

void ThrowExportsNotFound(Environment* env,
const std::string& match,
const std::string& subpath,
const URL& pjson_url,
const URL& base) {
const std::string msg = "Package exports for '" +
URL(".", pjson_url).ToFilePath() + "' do not define a '" + match +
const std::string msg = "Package exports for " +
pjson_url.ToFilePath() + " do not define a '" + subpath +
"' subpath, imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
}

void ThrowExportsInvalid(Environment* env,
const std::string& subpath,
const std::string& target,
const URL& pjson_url,
const URL& base) {
const std::string msg = "Cannot resolve package exports target '" + target +
"' matched for '" + subpath + "' in " + pjson_url.ToFilePath() +
", imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
}

void ThrowExportsInvalid(Environment* env,
const std::string& subpath,
Local<Value> target,
const URL& pjson_url,
const URL& base) {
Local<String> target_string;
if (target->ToString(env->context()).ToLocal(&target_string)) {
Utf8Value target_utf8(env->isolate(), target_string);
std::string target_str(*target_utf8, target_utf8.length());
if (target->IsArray()) {
target_str = '[' + target_str + ']';
}
ThrowExportsInvalid(env, subpath, target_str, pjson_url, base);
}
}

Maybe<URL> ResolveExportsTarget(Environment* env,
const std::string& target,
const std::string& subpath,
const std::string& match,
const URL& pjson_url,
const URL& base) {
if (target.substr(0, 2) == "./") {
if (subpath.length() > 0 && target.back() != '/') {
return Nothing<URL>();
const URL& base,
bool throw_invalid = true) {
if (target.substr(0, 2) != "./") {
if (throw_invalid) {
ThrowExportsInvalid(env, match, target, pjson_url, base);
}
URL resolved(target, pjson_url);
std::string resolved_path = resolved.path();
std::string pkg_path = URL(".", pjson_url).path();
if (resolved_path.find(pkg_path) == 0 &&
resolved_path.find("/node_modules/", pkg_path.length() - 1) ==
std::string::npos) {
if (subpath.length() == 0) return Just(resolved);
URL subpath_resolved(subpath, resolved);
std::string subpath_resolved_path = subpath_resolved.path();
if (subpath_resolved_path.find(pkg_path) == 0 &&
subpath_resolved_path.find("/node_modules/", pkg_path.length() - 1)
== std::string::npos) {
return Just(subpath_resolved);
}
return Nothing<URL>();
}
if (subpath.length() > 0 && target.back() != '/') {
if (throw_invalid) {
ThrowExportsInvalid(env, match, target, pjson_url, base);
}
return Nothing<URL>();
}
// Note: std: / nodejs: support goes here
return Nothing<URL>();
URL resolved(target, pjson_url);
std::string resolved_path = resolved.path();
std::string pkg_path = URL(".", pjson_url).path();
if (resolved_path.find(pkg_path) != 0 ||
resolved_path.find("/node_modules/", pkg_path.length() - 1) !=
std::string::npos) {
if (throw_invalid) {
ThrowExportsInvalid(env, match, target, pjson_url, base);
}
return Nothing<URL>();
}
if (subpath.length() == 0) return Just(resolved);
URL subpath_resolved(subpath, resolved);
std::string subpath_resolved_path = subpath_resolved.path();
if (subpath_resolved_path.find(pkg_path) != 0 ||
subpath_resolved_path.find("/node_modules/", pkg_path.length() - 1)
!= std::string::npos) {
if (throw_invalid) {
ThrowExportsInvalid(env, match, target + subpath, pjson_url, base);
}
return Nothing<URL>();
}
return Just(subpath_resolved);
}

Maybe<URL> PackageExportsResolve(Environment* env,
Expand Down Expand Up @@ -871,27 +912,43 @@ Maybe<URL> PackageExportsResolve(Environment* env,
Maybe<URL> resolved = ResolveExportsTarget(env, target_str, "",
pkg_subpath, pjson_url, base);
if (resolved.IsNothing()) {
ThrowExportsNotFound(env, pkg_subpath, pjson_url, base);
ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
return Nothing<URL>();
}
return FinalizeResolution(env, resolved.FromJust(), base);
} else if (target->IsArray()) {
Local<Array> target_arr = target.As<Array>();
const uint32_t length = target_arr->Length();
if (length == 0) {
ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
return Nothing<URL>();
}
for (uint32_t i = 0; i < length; i++) {
auto target_item = target_arr->Get(context, i).ToLocalChecked();
if (target_item->IsString()) {
Utf8Value target_utf8(isolate, target_item.As<v8::String>());
std::string target(*target_utf8, target_utf8.length());
Maybe<URL> resolved = ResolveExportsTarget(env, target, "",
pkg_subpath, pjson_url, base);
pkg_subpath, pjson_url, base, false);
if (resolved.IsNothing()) continue;
return FinalizeResolution(env, resolved.FromJust(), base);
}
}
auto invalid = target_arr->Get(context, length - 1).ToLocalChecked();
if (!invalid->IsString()) {
ThrowExportsInvalid(env, pkg_subpath, invalid, pjson_url, base);
return Nothing<URL>();
}
Utf8Value invalid_utf8(isolate, invalid.As<v8::String>());
std::string invalid_str(*invalid_utf8, invalid_utf8.length());
Maybe<URL> resolved = ResolveExportsTarget(env, invalid_str, "",
pkg_subpath, pjson_url, base);
CHECK(resolved.IsNothing());
return Nothing<URL>();
} else {
ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
return Nothing<URL>();
}
ThrowExportsNotFound(env, pkg_subpath, pjson_url, base);
return Nothing<URL>();
}

Local<String> best_match;
Expand Down Expand Up @@ -919,24 +976,42 @@ Maybe<URL> PackageExportsResolve(Environment* env,
Maybe<URL> resolved = ResolveExportsTarget(env, target, subpath,
pkg_subpath, pjson_url, base);
if (resolved.IsNothing()) {
ThrowExportsNotFound(env, pkg_subpath, pjson_url, base);
ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
return Nothing<URL>();
}
return FinalizeResolution(env, URL(subpath, resolved.FromJust()), base);
} else if (target->IsArray()) {
Local<Array> target_arr = target.As<Array>();
const uint32_t length = target_arr->Length();
if (length == 0) {
ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
return Nothing<URL>();
}
for (uint32_t i = 0; i < length; i++) {
auto target_item = target_arr->Get(context, i).ToLocalChecked();
if (target_item->IsString()) {
Utf8Value target_utf8(isolate, target_item.As<v8::String>());
std::string target_str(*target_utf8, target_utf8.length());
Maybe<URL> resolved = ResolveExportsTarget(env, target_str, subpath,
pkg_subpath, pjson_url, base);
pkg_subpath, pjson_url, base, false);
if (resolved.IsNothing()) continue;
return FinalizeResolution(env, resolved.FromJust(), base);
}
}
auto invalid = target_arr->Get(context, length - 1).ToLocalChecked();
if (!invalid->IsString()) {
ThrowExportsInvalid(env, pkg_subpath, invalid, pjson_url, base);
return Nothing<URL>();
}
Utf8Value invalid_utf8(isolate, invalid.As<v8::String>());
std::string invalid_str(*invalid_utf8, invalid_utf8.length());
Maybe<URL> resolved = ResolveExportsTarget(env, invalid_str, subpath,
pkg_subpath, pjson_url, base);
CHECK(resolved.IsNothing());
return Nothing<URL>();
} else {
ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
return Nothing<URL>();
}
}

Expand Down
14 changes: 14 additions & 0 deletions test/es-module/test-esm-exports.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
// The file exists but isn't exported. The exports is a number which counts
// as a non-null value without any properties, just like `{}`.
['pkgexports-number/hidden.js', './hidden.js'],
]);

const invalidExports = new Map([
// Even though 'pkgexports/sub/asdf.js' works, alternate "path-like"
// variants do not to prevent confusion and accidental loopholes.
['pkgexports/sub/./../asdf.js', './sub/./../asdf.js'],
Expand Down Expand Up @@ -67,6 +70,17 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
}));
}

for (const [specifier, subpath] of invalidExports) {
loadFixture(specifier).catch(mustCall((err) => {
strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');
assertStartsWith(err.message, (isRequire ? 'Package exports' :
'Cannot resolve'));
assertIncludes(err.message, isRequire ?
`do not define a valid '${subpath}' subpath` :
`matched for '${subpath}'`);
}));
}

// There's no main field so we won't find anything when importing the name.
// The fact that "." is mapped is ignored, it's not a valid main config.
loadFixture('pkgexports').catch(mustCall((err) => {
Expand Down

0 comments on commit a7e3e76

Please sign in to comment.