From 58ede19ac7b44435ecb78bc336a89b1ba893d5ca Mon Sep 17 00:00:00 2001 From: Jan Krems Date: Sun, 1 Oct 2017 10:31:04 -0700 Subject: [PATCH] module: Set dynamic import callback This is an initial implementation to support dynamic import in both scripts and modules. It's off by default since support for dynamic import is still flagged in V8. Without setting the V8 flag, this code won't be executed. Because of missing support in the V8 APIs, we can't support importing into the proper context yet. Without further changes this might allow code to break out of the context it's running in. --- .eslintignore | 1 + lib/internal/loader/Loader.js | 21 +++- lib/internal/loader/ModuleWrap.js | 6 +- lib/module.js | 1 + src/env.h | 1 + src/module_wrap.cc | 59 +++++++++++ src/module_wrap.h | 2 + test/es-module/test-esm-dynamic-import.js | 113 ++++++++++++++++++++++ 8 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 test/es-module/test-esm-dynamic-import.js diff --git a/.eslintignore b/.eslintignore index dc4e023866ff89..669c27ce89b57a 100644 --- a/.eslintignore +++ b/.eslintignore @@ -2,6 +2,7 @@ lib/internal/v8.js lib/internal/v8_prof_polyfill.js lib/punycode.js test/addons/??_* +test/es-module/test-esm-dynamic-import.js test/fixtures test/message/esm_display_syntax_error.mjs tools/eslint diff --git a/lib/internal/loader/Loader.js b/lib/internal/loader/Loader.js index 49c8699771e819..b8fe9ccfc614b8 100644 --- a/lib/internal/loader/Loader.js +++ b/lib/internal/loader/Loader.js @@ -1,8 +1,12 @@ 'use strict'; -const { getURLFromFilePath } = require('internal/url'); +const path = require('path'); +const { getURLFromFilePath, URL } = require('internal/url'); -const { createDynamicModule } = require('internal/loader/ModuleWrap'); +const { + createDynamicModule, + setImportModuleDynamicallyCallback +} = require('internal/loader/ModuleWrap'); const ModuleMap = require('internal/loader/ModuleMap'); const ModuleJob = require('internal/loader/ModuleJob'); @@ -24,6 +28,13 @@ function getURLStringForCwd() { } } +function normalizeReferrerURL(referrer) { + if (typeof referrer === 'string' && path.isAbsolute(referrer)) { + return getURLFromFilePath(referrer).href; + } + return new URL(referrer).href; +} + /* A Loader instance is used as the main entry point for loading ES modules. * Currently, this is a singleton -- there is only one used for loading * the main module and everything in its dependency graph. */ @@ -129,6 +140,12 @@ class Loader { const module = await job.run(); return module.namespace(); } + + static registerImportDynamicallyCallback(loader) { + setImportModuleDynamicallyCallback(async (referrer, specifier) => { + return loader.import(specifier, normalizeReferrerURL(referrer)); + }); + } } Loader.validFormats = ['esm', 'cjs', 'builtin', 'addon', 'json', 'dynamic']; Object.setPrototypeOf(Loader.prototype, null); diff --git a/lib/internal/loader/ModuleWrap.js b/lib/internal/loader/ModuleWrap.js index c22e149925caed..f0b9e2f757a7bd 100644 --- a/lib/internal/loader/ModuleWrap.js +++ b/lib/internal/loader/ModuleWrap.js @@ -1,6 +1,9 @@ 'use strict'; -const { ModuleWrap } = internalBinding('module_wrap'); +const { + ModuleWrap, + setImportModuleDynamicallyCallback +} = internalBinding('module_wrap'); const debug = require('util').debuglog('esm'); const ArrayJoin = Function.call.bind(Array.prototype.join); const ArrayMap = Function.call.bind(Array.prototype.map); @@ -59,5 +62,6 @@ const createDynamicModule = (exports, url = '', evaluate) => { module.exports = { createDynamicModule, + setImportModuleDynamicallyCallback, ModuleWrap }; diff --git a/lib/module.js b/lib/module.js index f404c4317d7816..4c4ceaf847fba2 100644 --- a/lib/module.js +++ b/lib/module.js @@ -472,6 +472,7 @@ Module._load = function(request, parent, isMain) { ESMLoader.hook(hooks); } } + Loader.registerImportDynamicallyCallback(ESMLoader); await ESMLoader.import(getURLFromFilePath(request).pathname); })() .catch((e) => { diff --git a/src/env.h b/src/env.h index 083c09a8134efb..d3c2b7f367ad54 100644 --- a/src/env.h +++ b/src/env.h @@ -332,6 +332,7 @@ class ModuleWrap; V(vm_parsing_context_symbol, v8::Symbol) \ V(url_constructor_function, v8::Function) \ V(write_wrap_constructor_function, v8::Function) \ + V(host_import_module_dynamically_callback, v8::Function) \ class Environment; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 0e1f7c9eaf8685..2474b362d7d858 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -558,6 +558,62 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result.FromJust().ToObject(env)); } +static MaybeLocal ImportModuleDynamically( + Local context, + Local referrer, + Local specifier) { + Isolate* iso = context->GetIsolate(); + Environment* env = Environment::GetCurrent(context); + v8::EscapableHandleScope handle_scope(iso); + + if (env->context() != context) { + auto maybe_resolver = Promise::Resolver::New(context); + Local resolver; + if (maybe_resolver.ToLocal(&resolver)) { + // TODO(jkrems): Turn into proper error object w/ code + Local error = v8::Exception::Error( + OneByteString(iso, "import() called outside of main context")); + if (resolver->Reject(context, error).IsJust()) { + return handle_scope.Escape(resolver.As()); + } + } + return MaybeLocal(); + } + + Local import_callback = + env->host_import_module_dynamically_callback(); + Local import_args[] = { + referrer->GetResourceName(), + Local(specifier) + }; + MaybeLocal maybe_result = import_callback->Call(context, + v8::Undefined(iso), + 2, + import_args); + + Local result; + if (maybe_result.ToLocal(&result)) { + return handle_scope.Escape(result.As()); + } + return MaybeLocal(); +} + +void ModuleWrap::SetImportModuleDynamicallyCallback( + const FunctionCallbackInfo& args) { + Isolate* iso = args.GetIsolate(); + Environment* env = Environment::GetCurrent(args); + HandleScope handle_scope(iso); + if (!args[0]->IsFunction()) { + env->ThrowError("first argument is not a function"); + return; + } + + Local import_callback = args[0].As(); + env->set_host_import_module_dynamically_callback(import_callback); + + iso->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically); +} + void ModuleWrap::Initialize(Local target, Local unused, Local context) { @@ -575,6 +631,9 @@ void ModuleWrap::Initialize(Local target, target->Set(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"), tpl->GetFunction()); env->SetMethod(target, "resolve", node::loader::ModuleWrap::Resolve); + env->SetMethod(target, + "setImportModuleDynamicallyCallback", + node::loader::ModuleWrap::SetImportModuleDynamicallyCallback); } } // namespace loader diff --git a/src/module_wrap.h b/src/module_wrap.h index a8dd89b790068a..ec4d6bf5772631 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -39,6 +39,8 @@ class ModuleWrap : public BaseObject { static void GetUrl(v8::Local property, const v8::PropertyCallbackInfo& info); static void Resolve(const v8::FunctionCallbackInfo& args); + static void SetImportModuleDynamicallyCallback( + const v8::FunctionCallbackInfo& args); static v8::MaybeLocal ResolveCallback( v8::Local context, v8::Local specifier, diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js new file mode 100644 index 00000000000000..a099a2ddb8a1ce --- /dev/null +++ b/test/es-module/test-esm-dynamic-import.js @@ -0,0 +1,113 @@ +// Flags: --experimental-modules --harmony-dynamic-import +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { URL } = require('url'); +const vm = require('vm'); + +common.crashOnUnhandledRejection(); + +const relativePath = './test-esm-ok.mjs'; +const absolutePath = require.resolve('./test-esm-ok.mjs'); +const targetURL = new URL('file:///'); +targetURL.pathname = absolutePath; + +function expectErrorProperty(result, propertyKey, value) { + Promise.resolve(result) + .catch(common.mustCall(error => { + assert.equal(error[propertyKey], value); + })); +} + +function expectMissingModuleError(result) { + expectErrorProperty(result, 'code', 'MODULE_NOT_FOUND'); +} + +function expectInvalidUrlError(result) { + expectErrorProperty(result, 'code', 'ERR_INVALID_URL'); +} + +function expectInvalidReferrerError(result) { + expectErrorProperty(result, 'code', 'ERR_INVALID_URL'); +} + +function expectInvalidProtocolError(result) { + expectErrorProperty(result, 'code', 'ERR_INVALID_PROTOCOL'); +} + +function expectInvalidContextError(result) { + expectErrorProperty(result, + 'message', 'import() called outside of main context'); +} + +function expectOkNamespace(result) { + Promise.resolve(result) + .then(common.mustCall(ns => { + // Can't deepStrictEqual because ns isn't a normal object + assert.deepEqual(ns, { default: true }); + })); +} + +function expectFsNamespace(result) { + Promise.resolve(result) + .then(common.mustCall(ns => { + assert.equal(typeof ns.default.writeFile, 'function'); + })); +} + +// For direct use of import expressions inside of CJS or ES modules, including +// via eval, all kinds of specifiers should work without issue. +(function testScriptOrModuleImport() { + // Importing another file, both direct & via eval + // expectOkNamespace(import(relativePath)); + expectOkNamespace(eval.call(null, `import("${relativePath}")`)); + expectOkNamespace(eval(`import("${relativePath}")`)); + expectOkNamespace(eval.call(null, `import("${targetURL}")`)); + + // Importing a built-in, both direct & via eval + expectFsNamespace(import("fs")); + expectFsNamespace(eval('import("fs")')); + expectFsNamespace(eval.call(null, 'import("fs")')); + + expectMissingModuleError(import("./not-an-existing-module.mjs")); + // TODO(jkrems): Right now this doesn't hit a protocol error because the + // module resolution step already rejects it. These arguably should be + // protocol errors. + expectMissingModuleError(import("node:fs")); + expectMissingModuleError(import('http://example.com/foo.js')); +})(); + +// vm.runInThisContext: +// * Supports built-ins, always +// * Supports imports if the script has a known defined origin +(function testRunInThisContext() { + // Succeeds because it's got an valid base url + expectFsNamespace(vm.runInThisContext(`import("fs")`, { + filename: __filename, + })); + expectOkNamespace(vm.runInThisContext(`import("${relativePath}")`, { + filename: __filename, + })); + // Rejects because it's got an invalid referrer URL. + // TODO(jkrems): Arguably the first two (built-in + absolute URL) could work + // with some additional effort. + expectInvalidReferrerError(vm.runInThisContext('import("fs")')); + expectInvalidReferrerError(vm.runInThisContext(`import("${targetURL}")`)); + expectInvalidReferrerError(vm.runInThisContext(`import("${relativePath}")`)); +})(); + +// vm.runInNewContext is currently completely unsupported, pending well-defined +// semantics for per-context/realm module maps in node. +(function testRunInNewContext() { + // Rejects because it's running in the wrong context + expectInvalidContextError( + vm.runInNewContext(`import("${targetURL}")`, undefined, { + filename: __filename, + }) + ); + + // Rejects because it's running in the wrong context + expectInvalidContextError(vm.runInNewContext(`import("fs")`, undefined, { + filename: __filename, + })); +})();