From e15d6adf990b9b61d48ef77a89a25e8f244b3cd2 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 16 May 2024 12:08:54 -0700 Subject: [PATCH] Provide the raw specifier for the module fallback service Fixes: https://github.com/cloudflare/workerd/issues/2112 --- src/workerd/io/worker.h | 3 ++- src/workerd/jsg/modules.c++ | 19 ++++++++++++------- src/workerd/jsg/modules.h | 32 ++++++++++++++++++++------------ src/workerd/jsg/setup.h | 3 ++- src/workerd/server/server.c++ | 8 +++++++- 5 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/workerd/io/worker.h b/src/workerd/io/worker.h index 9e83afaf346..9ea004bd1d5 100644 --- a/src/workerd/io/worker.h +++ b/src/workerd/io/worker.h @@ -478,7 +478,8 @@ class Worker::Api { kj::StringPtr, kj::Maybe, jsg::CompilationObserver&, - jsg::ModuleRegistry::ResolveMethod); + jsg::ModuleRegistry::ResolveMethod, + kj::Maybe); virtual void setModuleFallbackCallback( kj::Function&& callback) const { // By default does nothing. diff --git a/src/workerd/jsg/modules.c++ b/src/workerd/jsg/modules.c++ index 2c2cd036e3f..71dc0c42ae2 100644 --- a/src/workerd/jsg/modules.c++ +++ b/src/workerd/jsg/modules.c++ @@ -68,7 +68,6 @@ v8::MaybeLocal resolveCallback(v8::Local 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:") || @@ -81,7 +80,9 @@ v8::MaybeLocal resolveCallback(v8::Local 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 @@ -92,7 +93,8 @@ v8::MaybeLocal resolveCallback(v8::Local 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; } @@ -271,7 +273,8 @@ v8::Local 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(). @@ -621,7 +624,8 @@ v8::Local 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(). @@ -703,14 +707,15 @@ kj::Maybe> tryResolveFromFallb Lock& js, const kj::Path& specifier, kj::Maybe& referrer, CompilationObserver& observer, - ModuleRegistry::ResolveMethod method) { + ModuleRegistry::ResolveMethod method, + kj::Maybe rawSpecifier) { auto& isolateBase = IsolateBase::from(js.v8Isolate); KJ_IF_SOME(fallback, isolateBase.tryGetModuleFallback()) { kj::Maybe 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; } diff --git a/src/workerd/jsg/modules.h b/src/workerd/jsg/modules.h index be4bf30283d..f037d33d972 100644 --- a/src/workerd/jsg/modules.h +++ b/src/workerd/jsg/modules.h @@ -374,13 +374,15 @@ class ModuleRegistry { const kj::Path& specifier, kj::Maybe referrer = kj::none, ResolveOption option = ResolveOption::DEFAULT, - ResolveMethod method = ResolveMethod::IMPORT) = 0; + ResolveMethod method = ResolveMethod::IMPORT, + kj::Maybe rawSpecifier = kj::none) = 0; virtual kj::Maybe resolve(jsg::Lock& js, v8::Local module) = 0; virtual Promise 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; @@ -403,7 +405,8 @@ kj::Maybe> tryResolveFromFallb Lock& js, const kj::Path& specifier, kj::Maybe& referrer, CompilationObserver& observer, - ModuleRegistry::ResolveMethod method); + ModuleRegistry::ResolveMethod method, + kj::Maybe rawSpecifier); template class ModuleRegistryImpl final: public ModuleRegistry { @@ -556,7 +559,8 @@ class ModuleRegistryImpl final: public ModuleRegistry { const kj::Path& specifier, kj::Maybe referrer = kj::none, ResolveOption option = ResolveOption::DEFAULT, - ResolveMethod method = ResolveMethod::IMPORT) override { + ResolveMethod method = ResolveMethod::IMPORT, + kj::Maybe rawSpecifier = kj::none) override { using Key = typename Entry::Key; if (option == ResolveOption::INTERNAL_ONLY) { KJ_IF_SOME(entry, entries.find(Key(specifier, Type::INTERNAL))) { @@ -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) { @@ -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); } } } @@ -645,7 +650,8 @@ class ModuleRegistryImpl final: public ModuleRegistry { Promise 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 @@ -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); @@ -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); @@ -823,10 +831,10 @@ v8::MaybeLocal dynamicImportCallback(v8::Local context } })(); + auto spec = kj::str(specifier); auto maybeSpecifierPath = ([&]() -> kj::Maybe { // 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:")) { @@ -854,7 +862,7 @@ v8::MaybeLocal dynamicImportCallback(v8::Local 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 diff --git a/src/workerd/jsg/setup.h b/src/workerd/jsg/setup.h index 3cd91f5d021..be6c6cd9e2f 100644 --- a/src/workerd/jsg/setup.h +++ b/src/workerd/jsg/setup.h @@ -104,7 +104,8 @@ class IsolateBase { kj::StringPtr, kj::Maybe, jsg::CompilationObserver&, - jsg::ModuleRegistry::ResolveMethod); + jsg::ModuleRegistry::ResolveMethod, + kj::Maybe); inline void setModuleFallbackCallback(kj::Function&& callback) { maybeModuleFallbackCallback = kj::mv(callback); } diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++ index 353e0d0423e..116ecff3004 100644 --- a/src/workerd/server/server.c++ +++ b/src/workerd/server/server.c++ @@ -2715,7 +2715,8 @@ kj::Own Server::makeWorker(kj::StringPtr name, config::Worker:: kj::StringPtr specifier, kj::Maybe referrer, jsg::CompilationObserver& observer, - jsg::ModuleRegistry::ResolveMethod method) mutable + jsg::ModuleRegistry::ResolveMethod method, + kj::Maybe rawSpecifier) mutable -> kj::Maybe> { kj::Maybe jsonPayload; bool redirect = false; @@ -2753,6 +2754,11 @@ kj::Own 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) } + ); + } auto spec = url.toString(kj::Url::HTTP_REQUEST);