Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide the raw specifier for the module fallback service #2131

Merged
merged 1 commit into from
May 22, 2024
Merged

Provide the raw specifier for the module fallback service #2131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/workerd/io/worker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,8 @@ class Worker::Api {
kj::StringPtr,
kj::Maybe<kj::String>,
jsg::CompilationObserver&,
jsg::ModuleRegistry::ResolveMethod);
jsg::ModuleRegistry::ResolveMethod,
kj::Maybe<kj::StringPtr>);
virtual void setModuleFallbackCallback(
kj::Function<ModuleFallbackCallback>&& callback) const {
// By default does nothing.
Expand Down
19 changes: 12 additions & 7 deletions src/workerd/jsg/modules.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ v8::MaybeLocal<v8::Module> resolveCallback(v8::Local<v8::Context> context,
kj::Path targetPath = ([&] {
// If the specifier begins with one of our known prefixes, let's not resolve
// it against the referrer.
auto spec = kj::str(specifier);
if (internalOnly ||
spec.startsWith("node:") ||
spec.startsWith("cloudflare:") ||
Expand All @@ -81,7 +80,9 @@ v8::MaybeLocal<v8::Module> resolveCallback(v8::Local<v8::Context> context,
KJ_IF_SOME(resolved, registry->resolve(js, targetPath, ref.specifier,
internalOnly ?
ModuleRegistry::ResolveOption::INTERNAL_ONLY :
ModuleRegistry::ResolveOption::DEFAULT)) {
ModuleRegistry::ResolveOption::DEFAULT,
ModuleRegistry::ResolveMethod::IMPORT,
spec.asPtr())) {
result = resolved.module.getHandle(js);
} else {
// This is a bit annoying. If the module was not found, then
Expand All @@ -92,7 +93,8 @@ v8::MaybeLocal<v8::Module> resolveCallback(v8::Local<v8::Context> context,
// We only need to do this if internalOnly is false.
if (!internalOnly && (spec.startsWith("node:") || spec.startsWith("cloudflare:"))) {
KJ_IF_SOME(resolve, registry->resolve(js, kj::Path::parse(spec), ref.specifier,
ModuleRegistry::ResolveOption::DEFAULT)) {
ModuleRegistry::ResolveOption::DEFAULT,
ModuleRegistry::ResolveMethod::IMPORT, spec.asPtr())) {
result = resolve.module.getHandle(js);
return;
}
Expand Down Expand Up @@ -271,7 +273,8 @@ v8::Local<v8::Value> CommonJsModuleContext::require(jsg::Lock& js, kj::String sp
auto& info = JSG_REQUIRE_NONNULL(
modulesForResolveCallback->resolve(js, targetPath, path,
ModuleRegistry::ResolveOption::DEFAULT,
ModuleRegistry::ResolveMethod::REQUIRE),
ModuleRegistry::ResolveMethod::REQUIRE,
specifier.asPtr()),
Error, "No such module \"", targetPath.toString(), "\".");
// Adding imported from suffix here not necessary like it is for resolveCallback, since we have a
// js stack that will include the parent module's name and location of the failed require().
Expand Down Expand Up @@ -621,7 +624,8 @@ v8::Local<v8::Value> NodeJsModuleContext::require(jsg::Lock& js, kj::String spec
// excluded.
auto& info = JSG_REQUIRE_NONNULL(
modulesForResolveCallback->resolve(js, targetPath, path, resolveOption,
ModuleRegistry::ResolveMethod::REQUIRE),
ModuleRegistry::ResolveMethod::REQUIRE,
specifier.asPtr()),
Error, "No such module \"", targetPath.toString(), "\".");
// Adding imported from suffix here not necessary like it is for resolveCallback, since we have a
// js stack that will include the parent module's name and location of the failed require().
Expand Down Expand Up @@ -703,14 +707,15 @@ kj::Maybe<kj::OneOf<kj::String, ModuleRegistry::ModuleInfo>> tryResolveFromFallb
Lock& js, const kj::Path& specifier,
kj::Maybe<const kj::Path&>& referrer,
CompilationObserver& observer,
ModuleRegistry::ResolveMethod method) {
ModuleRegistry::ResolveMethod method,
kj::Maybe<kj::StringPtr> rawSpecifier) {
auto& isolateBase = IsolateBase::from(js.v8Isolate);
KJ_IF_SOME(fallback, isolateBase.tryGetModuleFallback()) {
kj::Maybe<kj::String> maybeRef;
KJ_IF_SOME(ref, referrer) {
maybeRef = ref.toString(true);
}
return fallback(js, specifier.toString(true), kj::mv(maybeRef), observer, method);
return fallback(js, specifier.toString(true), kj::mv(maybeRef), observer, method, rawSpecifier);
}
return kj::none;
}
Expand Down
32 changes: 20 additions & 12 deletions src/workerd/jsg/modules.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,13 +374,15 @@ class ModuleRegistry {
const kj::Path& specifier,
kj::Maybe<const kj::Path&> referrer = kj::none,
ResolveOption option = ResolveOption::DEFAULT,
ResolveMethod method = ResolveMethod::IMPORT) = 0;
ResolveMethod method = ResolveMethod::IMPORT,
kj::Maybe<kj::StringPtr> rawSpecifier = kj::none) = 0;

virtual kj::Maybe<ModuleRef> resolve(jsg::Lock& js, v8::Local<v8::Module> module) = 0;

virtual Promise<Value> resolveDynamicImport(jsg::Lock& js,
const kj::Path& specifier,
const kj::Path& referrer) = 0;
const kj::Path& referrer,
kj::StringPtr rawSpecifier) = 0;

virtual Value resolveInternalImport(jsg::Lock& js, const kj::StringPtr specifier) = 0;

Expand All @@ -403,7 +405,8 @@ kj::Maybe<kj::OneOf<kj::String, ModuleRegistry::ModuleInfo>> tryResolveFromFallb
Lock& js, const kj::Path& specifier,
kj::Maybe<const kj::Path&>& referrer,
CompilationObserver& observer,
ModuleRegistry::ResolveMethod method);
ModuleRegistry::ResolveMethod method,
kj::Maybe<kj::StringPtr> rawSpecifier);

template <typename TypeWrapper>
class ModuleRegistryImpl final: public ModuleRegistry {
Expand Down Expand Up @@ -556,7 +559,8 @@ class ModuleRegistryImpl final: public ModuleRegistry {
const kj::Path& specifier,
kj::Maybe<const kj::Path&> referrer = kj::none,
ResolveOption option = ResolveOption::DEFAULT,
ResolveMethod method = ResolveMethod::IMPORT) override {
ResolveMethod method = ResolveMethod::IMPORT,
kj::Maybe<kj::StringPtr> rawSpecifier = kj::none) override {
using Key = typename Entry::Key;
if (option == ResolveOption::INTERNAL_ONLY) {
KJ_IF_SOME(entry, entries.find(Key(specifier, Type::INTERNAL))) {
Expand Down Expand Up @@ -587,9 +591,10 @@ class ModuleRegistryImpl final: public ModuleRegistry {
// let's use it to try to resolve. Make sure we're using DEFAULT resolution so BUNDLE-typed
// modules from the fallback service can be used.
option = ResolveOption::DEFAULT;
return resolve(js, specifier.parent().eval(found), referrer, option, method);
return resolve(js, specifier.parent().eval(found), referrer, option, method, rawSpecifier);
}
KJ_IF_SOME(info, tryResolveFromFallbackService(js, specifier, referrer, observer, method)) {
KJ_IF_SOME(info, tryResolveFromFallbackService(js, specifier, referrer, observer,
method, rawSpecifier)) {
// If we resolved a module from the fallback service, we have to be sure
// to add it to the registry...
KJ_SWITCH_ONEOF(info) {
Expand All @@ -615,7 +620,7 @@ class ModuleRegistryImpl final: public ModuleRegistry {
// Make sure we're using DEFAULT resolution so BUNDLE-typed modules from the fallback
// service can be used.
option = ResolveOption::DEFAULT;
return resolve(js, specifier.parent().eval(s), referrer, option, method);
return resolve(js, specifier.parent().eval(s), referrer, option, method, rawSpecifier);
}
}
}
Expand Down Expand Up @@ -645,7 +650,8 @@ class ModuleRegistryImpl final: public ModuleRegistry {

Promise<Value> resolveDynamicImport(jsg::Lock& js,
const kj::Path& specifier,
const kj::Path& referrer) override {
const kj::Path& referrer,
kj::StringPtr rawSpecifier) override {
// Here, we first need to determine if the referrer is a built-in module
// or not. If it is a built-in, then we are only permitted to resolve
// internal modules. If the worker bundle provided an override for the
Expand All @@ -657,7 +663,8 @@ class ModuleRegistryImpl final: public ModuleRegistry {
resolveOption = ModuleRegistry::ResolveOption::INTERNAL_ONLY;
}

KJ_IF_SOME(info, resolve(js, specifier, referrer, resolveOption)) {
KJ_IF_SOME(info, resolve(js, specifier, referrer, resolveOption,
ResolveMethod::IMPORT, rawSpecifier)) {
KJ_IF_SOME(func, dynamicImportHandler) {
auto handler = [&info, isolate = js.v8Isolate]() -> Value {
auto& js = Lock::from(isolate);
Expand All @@ -679,7 +686,8 @@ class ModuleRegistryImpl final: public ModuleRegistry {
Value resolveInternalImport(jsg::Lock& js, const kj::StringPtr specifier) override {
auto specifierPath = kj::Path(specifier);
auto resolveOption = jsg::ModuleRegistry::ResolveOption::INTERNAL_ONLY;
auto maybeModuleInfo = resolve(js, specifierPath, kj::none, resolveOption);
auto maybeModuleInfo = resolve(js, specifierPath, kj::none, resolveOption,
ResolveMethod::IMPORT, specifier);
auto moduleInfo = &KJ_REQUIRE_NONNULL(maybeModuleInfo, "No such module \"", specifier, "\".");
auto handle = moduleInfo->module.getHandle(js);
jsg::instantiateModule(js, handle);
Expand Down Expand Up @@ -823,10 +831,10 @@ v8::MaybeLocal<v8::Promise> dynamicImportCallback(v8::Local<v8::Context> context
}
})();

auto spec = kj::str(specifier);
auto maybeSpecifierPath = ([&]() -> kj::Maybe<kj::Path> {
// If the specifier begins with one of our known prefixes, let's not resolve
// it against the referrer.
auto spec = kj::str(specifier);
if (spec.startsWith("node:") ||
spec.startsWith("cloudflare:") ||
spec.startsWith("workerd:")) {
Expand Down Expand Up @@ -854,7 +862,7 @@ v8::MaybeLocal<v8::Promise> dynamicImportCallback(v8::Local<v8::Context> context

try {
return wrapper.wrap(context, kj::none,
registry->resolveDynamicImport(js, specifierPath, referrerPath));
registry->resolveDynamicImport(js, specifierPath, referrerPath, spec));
} catch (JsExceptionThrown&) {
// If the tryCatch.Exception().IsEmpty() here is true, no JavaScript error
// was scheduled which can happen in a few edge cases. Treat it as if
Expand Down
3 changes: 2 additions & 1 deletion src/workerd/jsg/setup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ class IsolateBase {
kj::StringPtr,
kj::Maybe<kj::String>,
jsg::CompilationObserver&,
jsg::ModuleRegistry::ResolveMethod);
jsg::ModuleRegistry::ResolveMethod,
kj::Maybe<kj::StringPtr>);
inline void setModuleFallbackCallback(kj::Function<ModuleFallbackCallback>&& callback) {
maybeModuleFallbackCallback = kj::mv(callback);
}
Expand Down
8 changes: 7 additions & 1 deletion src/workerd/server/server.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2715,7 +2715,8 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name, config::Worker::
kj::StringPtr specifier,
kj::Maybe<kj::String> referrer,
jsg::CompilationObserver& observer,
jsg::ModuleRegistry::ResolveMethod method) mutable
jsg::ModuleRegistry::ResolveMethod method,
kj::Maybe<kj::StringPtr> rawSpecifier) mutable
-> kj::Maybe<kj::OneOf<kj::String, jsg::ModuleRegistry::ModuleInfo>> {
kj::Maybe<kj::String> jsonPayload;
bool redirect = false;
Expand Down Expand Up @@ -2753,6 +2754,11 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name, config::Worker::
kj::Url::QueryParam { kj::str("referrer"), kj::mv(ref) }
);
}
KJ_IF_SOME(raw, rawSpecifier) {
url.query.add(
kj::Url::QueryParam { kj::str("raw"), kj::str(raw) }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
kj::Url::QueryParam { kj::str("raw"), kj::str(raw) }
kj::Url::QueryParam { kj::str("rawSpecifier"), kj::str(raw) }

);
}

auto spec = url.toString(kj::Url::HTTP_REQUEST);

Expand Down
Loading