From 7da21975a6e35fb29beb7901c5966b2b9c206058 Mon Sep 17 00:00:00 2001 From: guybedford Date: Tue, 6 Nov 2018 01:01:39 +0200 Subject: [PATCH] implement spec --- lib/internal/errors.js | 2 +- lib/internal/modules/esm/default_resolve.js | 17 +- src/module_wrap.cc | 200 ++++++++++---------- src/node_errors.h | 1 + 4 files changed, 106 insertions(+), 114 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4094a40f6b..4cbfc92048 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -764,7 +764,7 @@ E('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', 'dynamicInstantiate function was provided', Error); E('ERR_MISSING_MODULE', 'Cannot find module %s', Error); E('ERR_MODULE_RESOLUTION_LEGACY', - '%s not found by import in %s.' + + 'Cannot find module \'%s\' imported from %s.' + ' Legacy behavior in require() would have found it at %s', Error); E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times', Error); diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 5ea2470325..60bb97301e 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -14,7 +14,6 @@ const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); -const StringStartsWith = Function.call.bind(String.prototype.startsWith); const { pathToFileURL, fileURLToPath } = require('internal/url'); const realpathCache = new Map(); @@ -35,8 +34,8 @@ function search(target, base) { tmpMod.paths = CJSmodule._nodeModulePaths( new URL('./', questionedBase).pathname); const found = CJSmodule._resolveFilename(target, tmpMod); - error = new ERR_MODULE_RESOLUTION_LEGACY(target, base, found); - } catch (problemChecking) { + error = new ERR_MODULE_RESOLUTION_LEGACY(target, fileURLToPath(base), found); + } catch { // ignore } throw error; @@ -56,16 +55,8 @@ function resolve(specifier, parentURL) { }; } - let url; - try { - url = search(specifier, - parentURL || pathToFileURL(`${process.cwd()}/`).href); - } catch (e) { - if (typeof e.message === 'string' && - StringStartsWith(e.message, 'Cannot find module')) - e.code = 'MODULE_NOT_FOUND'; - throw e; - } + let url = search(specifier, + parentURL || pathToFileURL(`${process.cwd()}/`).href); const isMain = parentURL === undefined; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 551a650be6..fe7c2beb88 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -467,104 +467,108 @@ std::string ReadFile(uv_file file) { return contents; } -enum CheckFileOptions { - LEAVE_OPEN_AFTER_CHECK, - CLOSE_AFTER_CHECK +enum DescriptorType { + NONE, + FILE, + DIRECTORY }; -Maybe CheckFile(const std::string& path, - CheckFileOptions opt = CLOSE_AFTER_CHECK) { +DescriptorType CheckDescriptor(const std::string& path) { uv_fs_t fs_req; - if (path.empty()) { - return Nothing(); - } - - uv_file fd = uv_fs_open(nullptr, &fs_req, path.c_str(), O_RDONLY, 0, nullptr); - uv_fs_req_cleanup(&fs_req); - - if (fd < 0) { - return Nothing(); - } - - uv_fs_fstat(nullptr, &fs_req, fd, nullptr); - uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR; - uv_fs_req_cleanup(&fs_req); - - if (is_directory) { - CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); - uv_fs_req_cleanup(&fs_req); - return Nothing(); - } - - if (opt == CLOSE_AFTER_CHECK) { - CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); + int rc = uv_fs_stat(nullptr, &fs_req, path.c_str(), nullptr); + if (rc == 0) { + uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR; uv_fs_req_cleanup(&fs_req); + return is_directory ? DIRECTORY : FILE; } - - return Just(fd); + uv_fs_req_cleanup(&fs_req); + return NONE; } -enum ResolveExtensionsOptions { - TRY_EXACT_NAME, - ONLY_VIA_EXTENSIONS -}; - -inline bool ResolvesToFile(const URL& search) { - std::string filePath = search.ToFilePath(); - Maybe check = CheckFile(filePath); - return !check.IsNothing(); +inline Maybe PackageMainResolve(const URL& search) { + std::string path = search.path(); + std::string last_segment = path.substr(path.find_last_of('/') + 1); + URL main_url = URL("./" + last_segment + "/index.mjs", search); + if (CheckDescriptor(main_url.ToFilePath()) == FILE) + return Just(main_url); + return Nothing(); } -template -Maybe ResolveExtensions(const URL& search) { - if (options == TRY_EXACT_NAME) { - std::string filePath = search.ToFilePath(); - Maybe check = CheckFile(filePath); - if (!check.IsNothing()) { - return Just(search); - } - } - - for (const char* extension : EXTENSIONS) { - URL guess(search.path() + extension, &search); - Maybe check = CheckFile(guess.ToFilePath()); - if (!check.IsNothing()) { - return Just(guess); +Maybe PackageResolve(Environment* env, + const std::string& specifier, + const URL& base) { + if (specifier.length() == 0) return Nothing(); + size_t sep_index = specifier.find('/'); + if (specifier[0] == '@') { + if (sep_index == std::string::npos) { + std::string msg = "Invalid package name '" + specifier + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_PACKAGE_NAME(env, msg.c_str()); + return Nothing(); } + sep_index = specifier.find('/', sep_index + 1); + } + std::string pkg_name = specifier.substr(0, + sep_index == std::string::npos ? std::string::npos : sep_index); + std::string pkg_path; + if (sep_index == std::string::npos || + sep_index == specifier.length() - 1) { + pkg_path = ""; + } else { + pkg_path = specifier.substr(sep_index); } - - return Nothing(); -} - -inline Maybe ResolveIndex(const URL& search) { - return ResolveExtensions(URL("index", search)); -} - -Maybe ResolveModule(Environment* env, - const std::string& specifier, - const URL& base) { URL parent(".", base); - URL dir(""); + std::string last_path; do { - dir = parent; - Maybe check = - Resolve(env, "./node_modules/" + specifier, dir); - if (!check.IsNothing()) { - const size_t limit = specifier.find('/'); - const size_t spec_len = - limit == std::string::npos ? specifier.length() : - limit + 1; - std::string chroot = - dir.path() + "node_modules/" + specifier.substr(0, spec_len); - if (check.FromJust().path().substr(0, chroot.length()) != chroot) { - return Nothing(); + URL pkg_url("./node_modules/" + pkg_name, &parent); + URL url; + if (pkg_path.length()) { + url = URL("./node_modules/" + pkg_name + pkg_path, &parent); + } else { + url = pkg_url; + } + if (pkg_path.length()) { + DescriptorType check = CheckDescriptor(url.ToFilePath()); + if (check == FILE) { + return Just(url); + } else if (check == DIRECTORY) { + Maybe main_url = PackageMainResolve(url); + if (!main_url.IsNothing()) return main_url; } - return check; } else { - // TODO(bmeck) PREVENT FALLTHROUGH + Maybe main_url = PackageMainResolve(url); + if (!main_url.IsNothing()) return main_url; + } + // throw not found if we did match a package directory + DescriptorType check = CheckDescriptor(pkg_url.ToFilePath()); + if (check == DIRECTORY) { + std::string msg = "Cannot find module '" + url.ToFilePath() + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_MISSING_MODULE(env, msg.c_str()); + return Nothing(); } - parent = URL("..", &dir); - } while (parent.path() != dir.path()); + last_path = parent.path(); + parent = URL("..", &parent); + // cross-platform root check + } while (parent.path() != last_path); + return Nothing(); +} + +Maybe PathResolve (Environment* env, + const URL& url, + const URL& base) { + std::string path = url.ToFilePath(); + DescriptorType check = CheckDescriptor(path); + if (check == FILE) return Just(url); + + if (check == DIRECTORY) { + Maybe url_main = PackageMainResolve(url); + if (!url_main.IsNothing()) return url_main; + } + + std::string msg = "Cannot find module '" + path + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_MISSING_MODULE(env, msg.c_str()); return Nothing(); } @@ -575,24 +579,13 @@ Maybe Resolve(Environment* env, const URL& base) { URL pure_url(specifier); if (!(pure_url.flags() & URL_FLAGS_FAILED)) { - // just check existence, without altering - Maybe check = CheckFile(pure_url.ToFilePath()); - if (check.IsNothing()) { - return Nothing(); - } - return Just(pure_url); - } - if (specifier.length() == 0) { - return Nothing(); + return PathResolve(env, pure_url, base); } if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { URL resolved(specifier, base); - if (ResolvesToFile(resolved)) - return Just(resolved); - return Nothing(); - } else { - return ResolveModule(env, specifier, base); + return PathResolve(env, resolved, base); } + return PackageResolve(env, specifier, base); } void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { @@ -613,11 +606,18 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { return node::THROW_ERR_INVALID_ARG_TYPE( env, "second argument is not a URL string"); } - + + TryCatch try_catch(env->isolate()); Maybe result = node::loader::Resolve(env, specifier_std, url); - if (result.IsNothing() || (result.FromJust().flags() & URL_FLAGS_FAILED)) { - std::string msg = "Cannot find module " + specifier_std; - return node::THROW_ERR_MISSING_MODULE(env, msg.c_str()); + if (try_catch.HasCaught()) { + try_catch.ReThrow(); + return; + } else if (result.IsNothing() || (result.FromJust().flags() & URL_FLAGS_FAILED)) { + std::string msg = "Cannot find module '" + specifier_std + + "' imported from " + url.ToFilePath(); + node::THROW_ERR_MISSING_MODULE(env, msg.c_str()); + try_catch.ReThrow(); + return; } args.GetReturnValue().Set(result.FromJust().ToObject(env)); diff --git a/src/node_errors.h b/src/node_errors.h index 233a0f7532..cff3d55932 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -28,6 +28,7 @@ namespace node { V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \ V(ERR_INVALID_ARG_VALUE, TypeError) \ V(ERR_INVALID_ARG_TYPE, TypeError) \ + V(ERR_INVALID_PACKAGE_NAME, Error) \ V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \ V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MISSING_ARGS, TypeError) \