Skip to content
Permalink
Browse files
[JSC] Perform module specifier validation at parsing time
https://bugs.webkit.org/show_bug.cgi?id=178256

Reviewed by Darin Adler.

Source/JavaScriptCore:

This patch make module loader's `resolve` operation synchronous. And we validate
module's requested module names when instantiating the module instead of satisfying
module's dependencies. This change is not observable to users. But this is precise
to the spec and this optimizes & simplifies the current module loader a bit by
reducing object allocations.

Previously, we have an object called pair in the module loader. This is pair of
module's name and module's record. And we use it to link one module to dependent
modules. Now, it is replaced with module's registry entry.

We also change our loader functions to take a registry entry instead of a module key.
Previous design is due to the consideration that these APIs may be exposed to users
in whatwg/loader spec. However, this won't happen. This change removes unnecessary
repeatedly hash map lookups.

* builtins/ModuleLoaderPrototype.js:
(globalPrivate.newRegistryEntry):
(requestFetch):
(requestInstantiate):
(requestSatisfy):
(link):
(moduleEvaluation):
(loadModule):
* jsc.cpp:
(GlobalObject::moduleLoaderResolve):
* runtime/AbstractModuleRecord.cpp:
(JSC::AbstractModuleRecord::finishCreation):
(JSC::AbstractModuleRecord::hostResolveImportedModule):
* runtime/JSGlobalObject.h:
* runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::resolveSync):
(JSC::JSModuleLoader::resolve):
* runtime/JSModuleLoader.h:
* runtime/ModuleLoaderPrototype.cpp:
(JSC::moduleLoaderPrototypeResolveSync):

Source/WebCore:

No behavior change in the current implementation.

* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::moduleLoaderResolve):
* bindings/js/JSDOMWindowBase.h:
* bindings/js/ScriptModuleLoader.cpp:
(WebCore::ScriptModuleLoader::resolve):
* bindings/js/ScriptModuleLoader.h:


Canonical link: https://commits.webkit.org/194538@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223331 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Constellation committed Oct 16, 2017
1 parent 29c48a6 commit 67c1f0f56aa645c4be81febcd56cbe62e8013354
@@ -1,3 +1,46 @@
2017-10-15 Yusuke Suzuki <utatane.tea@gmail.com>

[JSC] Perform module specifier validation at parsing time
https://bugs.webkit.org/show_bug.cgi?id=178256

Reviewed by Darin Adler.

This patch make module loader's `resolve` operation synchronous. And we validate
module's requested module names when instantiating the module instead of satisfying
module's dependencies. This change is not observable to users. But this is precise
to the spec and this optimizes & simplifies the current module loader a bit by
reducing object allocations.

Previously, we have an object called pair in the module loader. This is pair of
module's name and module's record. And we use it to link one module to dependent
modules. Now, it is replaced with module's registry entry.

We also change our loader functions to take a registry entry instead of a module key.
Previous design is due to the consideration that these APIs may be exposed to users
in whatwg/loader spec. However, this won't happen. This change removes unnecessary
repeatedly hash map lookups.

* builtins/ModuleLoaderPrototype.js:
(globalPrivate.newRegistryEntry):
(requestFetch):
(requestInstantiate):
(requestSatisfy):
(link):
(moduleEvaluation):
(loadModule):
* jsc.cpp:
(GlobalObject::moduleLoaderResolve):
* runtime/AbstractModuleRecord.cpp:
(JSC::AbstractModuleRecord::finishCreation):
(JSC::AbstractModuleRecord::hostResolveImportedModule):
* runtime/JSGlobalObject.h:
* runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::resolveSync):
(JSC::JSModuleLoader::resolve):
* runtime/JSModuleLoader.h:
* runtime/ModuleLoaderPrototype.cpp:
(JSC::moduleLoaderPrototypeResolveSync):

2017-10-14 Devin Rousso <webkit@devinrousso.com>

Web Inspector: provide a way to enable/disable event listeners
@@ -99,6 +99,7 @@ function newRegistryEntry(key)
module: @undefined, // JSModuleRecord
linkError: @undefined,
linkSucceeded: true,
evaluated: false,
};
}

@@ -140,13 +141,12 @@ function fulfillFetch(entry, source)

// Loader.

function requestFetch(key, parameters, fetcher)
function requestFetch(entry, parameters, fetcher)
{
// https://whatwg.github.io/loader/#request-fetch

"use strict";

var entry = this.ensureRegistered(key);
if (entry.fetch)
return entry.fetch;

@@ -156,26 +156,26 @@ function requestFetch(key, parameters, fetcher)
// Take the key and fetch the resource actually.
// For example, JavaScriptCore shell can provide the hook fetching the resource
// from the local file system.
var fetchPromise = this.fetch(key, parameters, fetcher).then((source) => {
var fetchPromise = this.fetch(entry.key, parameters, fetcher).then((source) => {
@setStateToMax(entry, @ModuleInstantiate);
return source;
});
entry.fetch = fetchPromise;
return fetchPromise;
}

function requestInstantiate(key, parameters, fetcher)
function requestInstantiate(entry, parameters, fetcher)
{
// https://whatwg.github.io/loader/#request-instantiate

"use strict";

var entry = this.ensureRegistered(key);
if (entry.instantiate)
return entry.instantiate;

var instantiatePromise = this.requestFetch(key, parameters, fetcher).then((source) => {
var moduleRecord = this.parseModule(entry.key, source);
var instantiatePromise = this.requestFetch(entry, parameters, fetcher).then((source) => {
var key = entry.key;
var moduleRecord = this.parseModule(key, source);

// FIXME: Described in the draft,
// 4. Fulfill entry.[[Instantiate]] with instance.
@@ -184,18 +184,15 @@ function requestInstantiate(key, parameters, fetcher)
// fulfilled without this "force fulfill" operation.
// https://github.com/whatwg/loader/pull/67

var dependencies = [];
var dependenciesMap = moduleRecord.dependenciesMap;
moduleRecord.registryEntry = entry;
var requestedModules = this.requestedModules(moduleRecord);
var dependencies = @newArrayWithSize(requestedModules.length);
for (var i = 0, length = requestedModules.length; i < length; ++i) {
var depKey = requestedModules[i];
var pair = {
key: depKey,
value: @undefined
};
@putByValDirect(dependencies, dependencies.length, pair);
dependenciesMap.@set(depKey, pair);
var depName = requestedModules[i];
var depKey = this.resolveSync(depName, key, fetcher);
var depEntry = this.ensureRegistered(depKey);
@putByValDirect(dependencies, i, depEntry);
dependenciesMap.@set(depName, depEntry);
}
entry.dependencies = dependencies;
entry.dependenciesMap = dependenciesMap;
@@ -207,52 +204,37 @@ function requestInstantiate(key, parameters, fetcher)
return instantiatePromise;
}

function requestSatisfy(key, parameters, fetcher)
function requestSatisfy(entry, parameters, fetcher)
{
// https://whatwg.github.io/loader/#satisfy-instance

"use strict";

var entry = this.ensureRegistered(key);
if (entry.satisfy)
return entry.satisfy;

var satisfyPromise = this.requestInstantiate(key, parameters, fetcher).then((entry) => {
var depLoads = [];
var satisfyPromise = this.requestInstantiate(entry, parameters, fetcher).then((entry) => {
var depLoads = @newArrayWithSize(entry.dependencies.length);
for (var i = 0, length = entry.dependencies.length; i < length; ++i) {
let pair = entry.dependencies[i];

// Hook point.
// 1. Loader.resolve.
// https://whatwg.github.io/loader/#browser-resolve
// Take the name and resolve it to the unique identifier for the resource location.
// For example, take the "jquery" and return the URL for the resource.
var promise = this.resolve(pair.key, key, fetcher).then((depKey) => {
var depEntry = this.ensureRegistered(depKey);

// Recursive resolving. The dependencies of this entry is being resolved or already resolved.
// Stop tracing the circular dependencies.
// But to retrieve the instantiated module record correctly,
// we need to wait for the instantiation for the dependent module.
// For example, reaching here, the module is starting resolving the dependencies.
// But the module may or may not reach the instantiation phase in the loader's pipeline.
// If we wait for the Satisfy for this module, it construct the circular promise chain and
// rejected by the Promises runtime. Since only we need is the instantiated module, instead of waiting
// the Satisfy for this module, we just wait Instantiate for this.
if (depEntry.satisfy) {
return depEntry.instantiate.then((entry) => {
pair.value = entry.module;
return entry;
});
}

var depEntry = entry.dependencies[i];
var promise = @undefined;

// Recursive resolving. The dependencies of this entry is being resolved or already resolved.
// Stop tracing the circular dependencies.
// But to retrieve the instantiated module record correctly,
// we need to wait for the instantiation for the dependent module.
// For example, reaching here, the module is starting resolving the dependencies.
// But the module may or may not reach the instantiation phase in the loader's pipeline.
// If we wait for the Satisfy for this module, it construct the circular promise chain and
// rejected by the Promises runtime. Since only we need is the instantiated module, instead of waiting
// the Satisfy for this module, we just wait Instantiate for this.
if (depEntry.satisfy)
promise = depEntry.instantiate;
else {
// Currently, module loader do not pass any information for non-top-level module fetching.
return this.requestSatisfy(depKey, @undefined, fetcher).then((entry) => {
pair.value = entry.module;
return entry;
});
});
@putByValDirect(depLoads, depLoads.length, promise);
promise = this.requestSatisfy(depEntry, @undefined, fetcher);
}
@putByValDirect(depLoads, i, promise);
}

return @InternalPromise.internalAll(depLoads).then((modules) => {
@@ -284,10 +266,8 @@ function link(entry, fetcher)
// we can call moduleDeclarationInstantiation with the correct order
// without constructing the dependency graph by calling dependencyGraph.
var dependencies = entry.dependencies;
for (var i = 0, length = dependencies.length; i < length; ++i) {
var pair = dependencies[i];
this.link(pair.value.registryEntry, fetcher);
}
for (var i = 0, length = dependencies.length; i < length; ++i)
this.link(dependencies[i], fetcher);

this.moduleDeclarationInstantiation(entry.module, entry.key, fetcher);
} catch (error) {
@@ -299,26 +279,22 @@ function link(entry, fetcher)

// Module semantics.

function moduleEvaluation(moduleRecord, fetcher)
function moduleEvaluation(entry, fetcher)
{
// http://www.ecma-international.org/ecma-262/6.0/#sec-moduleevaluation

"use strict";

if (moduleRecord.evaluated)
if (entry.evaluated)
return;
moduleRecord.evaluated = true;

var entry = moduleRecord.registryEntry;
entry.evaluated = true;

// The contents of the [[RequestedModules]] is cloned into entry.dependencies.
var dependencies = entry.dependencies;
for (var i = 0, length = dependencies.length; i < length; ++i) {
var pair = dependencies[i];
var requiredModuleRecord = pair.value;
this.moduleEvaluation(requiredModuleRecord, fetcher);
}
this.evaluate(entry.key, moduleRecord, fetcher);
for (var i = 0, length = dependencies.length; i < length; ++i)
this.moduleEvaluation(dependencies[i], fetcher);

this.evaluate(entry.key, entry.module, fetcher);
}

// APIs to control the module loader.
@@ -343,7 +319,7 @@ function loadModule(moduleName, parameters, fetcher)
// Take the name and resolve it to the unique identifier for the resource location.
// For example, take the "jquery" and return the URL for the resource.
return this.resolve(moduleName, @undefined, fetcher).then((key) => {
return this.requestSatisfy(key, parameters, fetcher);
return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher);
}).then((entry) => {
return entry.key;
});
@@ -358,7 +334,7 @@ function linkAndEvaluateModule(key, fetcher)
@throwTypeError("Requested module is not instantiated yet.");

this.link(entry, fetcher);
return this.moduleEvaluation(entry.module, fetcher);
return this.moduleEvaluation(entry, fetcher);
}

function loadAndEvaluateModule(moduleName, parameters, fetcher)
@@ -374,7 +350,7 @@ function requestImportModule(key, parameters, fetcher)
{
"use strict";

return this.requestSatisfy(key, parameters, fetcher).then((entry) => {
return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher).then((entry) => {
this.linkAndEvaluateModule(entry.key, fetcher);
return this.getModuleNamespaceObject(entry.module);
});
@@ -1658,7 +1658,7 @@ class GlobalObject : public JSGlobalObject {
}

static JSInternalPromise* moduleLoaderImportModule(JSGlobalObject*, ExecState*, JSModuleLoader*, JSString*, JSValue, const SourceOrigin&);
static JSInternalPromise* moduleLoaderResolve(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
static Identifier moduleLoaderResolve(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
static JSInternalPromise* moduleLoaderFetch(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
static JSObject* moduleLoaderCreateImportMetaProperties(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSModuleRecord*, JSValue);
};
@@ -1840,57 +1840,46 @@ JSInternalPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* global
return result;
}

JSInternalPromise* GlobalObject::moduleLoaderResolve(JSGlobalObject* globalObject, ExecState* exec, JSModuleLoader*, JSValue keyValue, JSValue referrerValue, JSValue)
Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* globalObject, ExecState* exec, JSModuleLoader*, JSValue keyValue, JSValue referrerValue, JSValue)
{
VM& vm = globalObject->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);
auto scope = DECLARE_THROW_SCOPE(vm);

JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
scope.releaseAssertNoException();
const Identifier key = keyValue.toPropertyKey(exec);
if (UNLIKELY(scope.exception())) {
JSValue exception = scope.exception();
scope.clearException();
return deferred->reject(exec, exception);
}
RETURN_IF_EXCEPTION(scope, { });

if (key.isSymbol())
return key;

if (key.isSymbol()) {
auto result = deferred->resolve(exec, keyValue);
scope.releaseAssertNoException();
return result;
}
if (referrerValue.isUndefined()) {
auto directoryName = currentWorkingDirectory();
if (!directoryName)
return deferred->reject(exec, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
auto result = deferred->resolve(exec, jsString(exec, resolvePath(directoryName.value(), ModuleName(key.impl()))));
scope.releaseAssertNoException();
return result;
if (!directoryName) {
throwException(exec, scope, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
return { };
}
return Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(key.impl())));
}

const Identifier referrer = referrerValue.toPropertyKey(exec);
if (UNLIKELY(scope.exception())) {
JSValue exception = scope.exception();
scope.clearException();
return deferred->reject(exec, exception);
}
RETURN_IF_EXCEPTION(scope, { });

if (referrer.isSymbol()) {
auto directoryName = currentWorkingDirectory();
if (!directoryName)
return deferred->reject(exec, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
auto result = deferred->resolve(exec, jsString(exec, resolvePath(directoryName.value(), ModuleName(key.impl()))));
scope.releaseAssertNoException();
return result;
if (!directoryName) {
throwException(exec, scope, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
return { };
}
return Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(key.impl())));
}

// If the referrer exists, we assume that the referrer is the correct absolute path.
auto directoryName = extractDirectoryName(referrer.impl());
if (!directoryName)
return deferred->reject(exec, createError(exec, makeString("Could not resolve the referrer name '", String(referrer.impl()), "'.")));
auto result = deferred->resolve(exec, jsString(exec, resolvePath(directoryName.value(), ModuleName(key.impl()))));
scope.releaseAssertNoException();
return result;
if (!directoryName) {
throwException(exec, scope, createError(exec, makeString("Could not resolve the referrer name '", String(referrer.impl()), "'.")));
return { };
}
return Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(key.impl())));
}

static void convertShebangToJSComment(Vector<char>& buffer)
@@ -54,8 +54,6 @@ void AbstractModuleRecord::finishCreation(ExecState* exec, VM& vm)
{
Base::finishCreation(vm);
ASSERT(inherits(vm, info()));
putDirect(vm, Identifier::fromString(&vm, ASCIILiteral("registryEntry")), jsUndefined());
putDirect(vm, Identifier::fromString(&vm, ASCIILiteral("evaluated")), jsBoolean(false));

auto scope = DECLARE_THROW_SCOPE(vm);
JSMap* map = JSMap::create(exec, vm, globalObject()->mapStructure());
@@ -149,10 +147,10 @@ AbstractModuleRecord* AbstractModuleRecord::hostResolveImportedModule(ExecState*
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
JSValue moduleNameValue = identifierToJSValue(exec, moduleName);
JSValue pair = m_dependenciesMap->JSMap::get(exec, moduleNameValue);
JSValue entry = m_dependenciesMap->JSMap::get(exec, moduleNameValue);
RETURN_IF_EXCEPTION(scope, nullptr);
scope.release();
return jsCast<AbstractModuleRecord*>(pair.get(exec, Identifier::fromString(exec, "value")));
return jsCast<AbstractModuleRecord*>(entry.get(exec, Identifier::fromString(exec, "module")));
}

auto AbstractModuleRecord::resolveImport(ExecState* exec, const Identifier& localName) -> Resolution
@@ -189,7 +189,7 @@ struct GlobalObjectMethodTable {
typedef JSInternalPromise* (*ModuleLoaderImportModulePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSString*, JSValue, const SourceOrigin&);
ModuleLoaderImportModulePtr moduleLoaderImportModule;

typedef JSInternalPromise* (*ModuleLoaderResolvePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
typedef Identifier (*ModuleLoaderResolvePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
ModuleLoaderResolvePtr moduleLoaderResolve;

typedef JSInternalPromise* (*ModuleLoaderFetchPtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);

0 comments on commit 67c1f0f

Please sign in to comment.