Open jcbhmr opened 11 months ago
Can you copy paste the output of your console instead of sending screen shots? Screenshots can be very unhelpful for e.g. folks using screen readers and/or folks with visual disabilities.
The following works on most[^1] runtimes but Node.js:
Promise.resolve(`import("data:text/javascript,")`).then(eval).then(console.log, console.error)
[^1]: I'd like to say all, but there are so many runtimes these days I surely missed at least one
Interesting that you don't even need a specifier! And it appears to also do the same Invalid host defined options
on .catch(eval)
too.
jcbhmr@PIG-2016:~$ node -e 'Promise.reject(`import("")`).catch(eval)'
undefined:1
import("")
^
TypeError: Invalid host defined options
at eval (eval at <anonymous> (unknown source), <anonymous>:1:1)
at eval (<anonymous>)
Node.js v20.7.0
The two spots I could find from a GitHub search for "Invalid host defined options" are these:
Interesting:
jcbhmr@PIG-2016:~$ node -e 'Promise.resolve().then(eval.bind(null,"import(``)"))'
undefined:1
import(``)
^
TypeError: Invalid host defined options
at eval (eval at <anonymous> (unknown source), <anonymous>:1:1)
at eval (<anonymous>)
Node.js v20.7.0
Even more interesting:
jcbhmr@PIG-2016:~$ node -e 'queueMicrotask(eval.bind(null,"import(``)"))'
undefined:1
import(``)
^
TypeError: Invalid host defined options
at eval (eval at <anonymous> (node:internal/process/task_queues:140:7), <anonymous>:1:1)
at eval (<anonymous>)
at node:internal/process/task_queues:140:7
at AsyncResource.runInAsyncScope (node:async_hooks:206:9)
at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)
Node.js v20.7.0
The following check is failing (options->Length()
is supposed to be 9, it's 0 here):
https://github.com/nodejs/node/blob/26c8858834756412267ebcbf668d60dc6a1031ea/src/module_wrap.cc#L568-L569
Here's a backtrace (maybe I need a debug build):
$ echo 'setImmediate(eval.bind(null, `import(null)`)' > entry.js
$ lldb ./node entry.js
(lldb) b module_wrap.cc:568
Breakpoint 1: where = node`node::loader::ImportModuleDynamically(v8::Local<v8::Context>, v8::Local<v8::Data>, v8::Local<v8::Value>, v8::Local<v8::String>, v8::Local<v8::FixedArray>) + 160 at module_wrap.cc:568:52, address = 0x00000001001cb534
(lldb) r
Process 88426 launched: '/Users/duhamean/Documents/node/node' (arm64)
Process 88426 stopped
Target 0: (node) stopped.
(lldb) n
Process 88426 stopped
(lldb) call options->Length()
(int) $0 = 0
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x00000001001cb534 node`node::loader::ImportModuleDynamically(context=(val_ = 0x000000010600ff38), host_defined_options=(val_ = 0x000000010600ff40), resource_name=(val_ = 0x000000010600ff48), specifier=(val_ = 0x000000016fdfdeb0), import_assertions=(val_ = 0x00000001080082d8)) at module_wrap.cc:568:52
frame #1: 0x000000010077373c node`v8::internal::Isolate::RunHostImportModuleDynamicallyCallback(this=0x0000000108008000, maybe_referrer=<unavailable>, specifier=<unavailable>, maybe_import_assertions_argument=<unavailable>) at isolate.cc:5123:5 [opt]
frame #2: 0x0000000100bd02ac node`v8::internal::Runtime_DynamicImportCall(int, unsigned long*, v8::internal::Isolate*) at runtime-module.cc:38:3 [opt]
frame #3: 0x0000000100bd01d8 node`v8::internal::Runtime_DynamicImportCall(args_length=2, args_object=<unavailable>, isolate=0x0000000108008000) at runtime-module.cc:24:1 [opt]
frame #4: 0x0000000100f3ca1c node`Builtins_DeoptimizationEntry_Eager + 608796
frame #5: 0x0000000100fefc3c node`Builtins_DeoptimizationEntry_Eager + 1342524
frame #6: 0x0000000100eb43e4 node`Builtins_DeoptimizationEntry_Eager + 50148
frame #7: 0x0000000100eb250c node`Builtins_DeoptimizationEntry_Eager + 42252
frame #8: 0x0000000100eb21f4 node`Builtins_DeoptimizationEntry_Eager + 41460
frame #9: 0x0000000100756e38 node`v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [inlined] v8::internal::GeneratedCode<unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, long, unsigned long**>::Call(this=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>) at simulator.h:154:12 [opt]
frame #10: 0x0000000100756e34 node`v8::internal::(anonymous namespace)::Invoke(isolate=<unavailable>, params=<unavailable>) at execution.cc:427:33 [opt]
frame #11: 0x00000001007566c8 node`v8::internal::Execution::Call(isolate=0x0000000108008000, callable=<unavailable>, receiver=<unavailable>, argc=0, argv=0x0000000000000000) at execution.cc:529:10 [opt]
frame #12: 0x000000010066b5e0 node`v8::internal::Builtin_GlobalEval(int, unsigned long*, v8::internal::Isolate*) at builtins-global.cc:110:3 [opt]
frame #13: 0x000000010066b51c node`v8::internal::Builtin_GlobalEval(args_length=<unavailable>, args_object=<unavailable>, isolate=0x0000000108008000) at builtins-global.cc:84:1 [opt]
frame #14: 0x0000000100f3cb24 node`Builtins_DeoptimizationEntry_Eager + 609060
frame #15: 0x0000000100f98fb8 node`Builtins_DeoptimizationEntry_Eager + 987064
frame #16: 0x0000000100edab94 node`Builtins_DeoptimizationEntry_Eager + 207764
frame #17: 0x0000000100eb23f4 node`Builtins_DeoptimizationEntry_Eager + 41972
frame #18: 0x0000000100756df0 node`v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [inlined] v8::internal::GeneratedCode<unsigned long, unsigned long, v8::internal::MicrotaskQueue*>::Call(this=<unavailable>, args=<unavailable>, args=<unavailable>) at simulator.h:154:12 [opt]
frame #19: 0x0000000100756dec node`v8::internal::(anonymous namespace)::Invoke(isolate=<unavailable>, params=<unavailable>) at execution.cc:443:33 [opt]
frame #20: 0x0000000100757738 node`v8::internal::(anonymous namespace)::InvokeWithTryCatch(isolate=0x0000000108008000, params=0x000000016fdfe5f0) at execution.cc:490:20 [opt]
frame #21: 0x0000000100757924 node`v8::internal::Execution::TryRunMicrotasks(isolate=<unavailable>, microtask_queue=<unavailable>) at execution.cc:601:10 [opt]
frame #22: 0x000000010078110c node`v8::internal::MicrotaskQueue::RunMicrotasks(this=0x0000000104a07400, isolate=0x0000000108008000) at microtask-queue.cc:174:22 [opt]
frame #23: 0x000000010078190c node`v8::internal::MicrotaskQueue::PerformCheckpoint(v8::Isolate*) [inlined] v8::internal::MicrotaskQueue::PerformCheckpointInternal(this=0x0000000104a07400, v8_isolate=0x0000000108008000) at microtask-queue.cc:126:3 [opt]
frame #24: 0x00000001007818d0 node`v8::internal::MicrotaskQueue::PerformCheckpoint(this=0x0000000104a07400, isolate=0x0000000108008000) at microtask-queue.h:46:5 [opt]
frame #25: 0x000000010006f1b0 node`node::InternalCallbackScope::Close(this=0x000000016fdfea90) at callback.cc:137:35
frame #26: 0x000000010006efcc node`node::InternalCallbackScope::~InternalCallbackScope(this=0x000000016fdfea90) at callback.cc:92:3
frame #27: 0x000000010006eae4 node`node::InternalCallbackScope::~InternalCallbackScope(this=0x000000016fdfea90) at callback.cc:91:49
frame #28: 0x00000001001d5cbc node`node::StartExecution(env=0x0000000105815e00, cb=node::StartExecutionCallback @ 0x000000016fdfeb28) at node.cc:372:1
frame #29: 0x000000010007d478 node`node::LoadEnvironment(env=0x0000000105815e00, cb=node::StartExecutionCallback @ 0x000000016fdfec28) at environment.cc:547:10
frame #30: 0x00000001002e3c30 node`node::NodeMainInstance::Run(this=0x000000016fdfed60, exit_code=0x000000016fdfecb4, env=0x0000000105815e00) at node_main_instance.cc:108:7
frame #31: 0x00000001002e386c node`node::NodeMainInstance::Run(this=0x000000016fdfed60) at node_main_instance.cc:88:3
frame #32: 0x00000001001d84b0 node`node::StartInternal(argc=2, argv=0x00000001049286c0) at node.cc:1369:24
frame #33: 0x00000001001d80f8 node`node::Start(argc=2, argv=0x000000016fdff170) at node.cc:1376:27
frame #34: 0x00000001012831c4 node`main(argc=2, argv=0x000000016fdff170) at node_main.cc:97:10
frame #35: 0x00000001a3663f28 dyld`start + 2236
(lldb) c
TypeError: Invalid host defined options
at eval (eval at <anonymous> (unknown source), <anonymous>:1:1)
at eval (<anonymous>)
Process 88426 exited with status = 0 (0x00000000)
host_defined_options
is empty because of this condition:
maybe_referrer
comes from here:
This is likely a V8 issue, I can also reproduce it with d8. The problem probably comes from that V8 is unable to retrieve the referrer/origin when code is eval
'ed directly from a microtask - if you wrap it in an anonymous function V8 is going to generate bytecode that passes the closure to a runtime call so that it can figure out the script and then the referrer/origin. If it's just eval directly with no JS frames above V8 uses an empty script as the wrapper and cannot infer host-defined options.
Also this does not only affect host-defined options, for example V8 would not be able to show the script name if an error is thrown from code being eval'ed directly from a microstask because of the same cause. I'll open an upstream issue.
Upstream issue https://bugs.chromium.org/p/v8/issues/detail?id=14336
@joyeecheung naive question: if it's a V8 problem then why doesn't Deno (also V8-based) have this issue? π€ is deno doing some magic or something?
jcbhmr@PIG-2016:~$ deno
Deno 1.37.0
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> Promise.resolve('import("node:fs")').then(eval).then(console.log)
[Module: null prototype] {
Dir: [class Dir],
Dirent: [class Dirent],
...
FWIW Chromium is also not affected. Even if it is not a V8 issue in the end, the patch that will fix d8 will probably contain all the needed info to fix it in Node.js, so I think our best course of action is to wait for V8 team for a fix.
My guess is, Deno or Chromium may not absolutely require the host defined options for the module resolution to work. If the referrer or host defined options are unknown they'll fall back to something else (for example, Chromium can fallback to a default referrer with an empty URL and somehow still work with that). Whereas Node.js enforces that it needs to know about the host defined options to look up for a callback and some data that was originally saved for the referrer script (because e.g. the vm APIs of Node.js allows customization of the dynamic import handling for each different vm.*Module
). If you run
Promise.resolve('throw new Error("test")')
.then(eval)
.catch(e => console.log(e.stack));
You can see that neither Deno nor Chromium is able to infer the origin (the script is listed as unknown source
instead of the file name). This means that they would also get empty host-defined options too. But their loaders can work without host-defined options probably because they don't support the kind of customizations that Node.js support.
One possible workaround might be, we can do basically what https://github.com/nodejs/node/pull/48655 does for shadow realms - when we don't have host-defined options at hand to look up for the callback and data, we just fall back to the default loader with the parentURL
passed as undefined
. This would allow us to load a module when there's no valid origin. Though that might be considered a bug itself (e.g. if the user expects that the custom importModuleDynamically
callback must be called for vm.Script
/vm.*Module
and there should not be some "fallback to default loader" behavior escaping their configurations). I am also not sure if falling back to an undefined parentURL
here can be considered a bug or not (for example this may allow bypassing of the policies - yet another Node.js feature that requires the origin to work).
Actually just realized that this bug may affect the integration of shadow realm too because we are operating under the assumption that an empty host defined options means that the import comes from ShadowRealm.prototype.importValue
...
Actually the Deno/Chromium behavior of falling back to an empty base URL may be quite surprising. If you create a directory structure:
- scripts/
- resolve.js
- mod.js (with `export const hello = 'world';`)
- mod.js (with `export const hello = 'oops';`)
and load scripts/resolve.js
from the base directory, with a wrapper to get the valid host-defined options:
Promise.resolve('import("./mod.js")').then((i) => eval(i)).then(i => console.log(i.hello));
It prints world
from scripts/mod.js
, but if you remove the wrapper and evaluate it directly from the microtask:
Promise.resolve('import("./mod.js")').then(eval).then(i => console.log(i.hello));
Due to the fallback, it's going to print oops
usingmod.js
from the base directory. It's probably not great that eval()
code differently lead to completely different files being loaded, or getting a surprising module-not-found error (e.g. if you remove the top-level mod.js
and expect resolve.js
to load ./mod.js
from the same directory). So even though it appears that Deno and Chromium works with eval()
from a microtask, once you have any kind of non-flat directory structure the lack of referrer information is still going to cause problems in module resolution.
I think printing oops
is the correct behavior? Idk. Since x => eval(x)
is a direct eval, it gets access to the current scope's import()
but an indirect eval (like globalThis.eval()
or .then(eval)
) should use the default location
/process.cwd()
-like base URL sorta like fetch()
does by default.
edit: after seeing the results of the test below idk what to think π΅
this is what I'm getting at:
// these should be different; direct vs indirect eval
Promise.resolve('import("./mod.js")').then((i) => eval(i)).then(i => console.log(i.hello));
Promise.resolve('import("./mod.js")').then((i) => globalThis.eval(i)).then(i => console.log(i.hello));
// these should act the same; both indirect eval
Promise.resolve('import("./mod.js")').then((i) => globalThis.eval(i)).then(i => console.log(i.hello));
Promise.resolve('import("./mod.js")').then(eval).then(i => console.log(i.hello));
// ...only problem is that this ^^^^ throws in current Node.js π’
csb: https://codesandbox.io/s/infallible-elbakyan-2ctpgp?file=/hello/world/main.js
console.clear();
Promise.resolve('import("./mod.js")')
.then((i) => eval(i))
.then((i) => console.log("eval()", i.default));
Promise.resolve('import("./mod.js")')
.then((i) => globalThis.eval(i))
.then((i) => console.log("globalThis.eval()", i.default));
Promise.resolve('import("./mod.js")')
.then(eval)
.then((i) => console.log(".then(eval)", i.default));
Promise.resolve('import("./mod.js")')
.then((x) => new Function("return " + x)())
.then((i) => console.log("new Function()", i.default));
actually now that i tried it I'm surprised that new Function()
keeps the context too! π±
I'm also surprised that Firefox and Node.js keep context even on globalThis.eval()
but chrome doesn't π€
idk if .then(eval)
should behave like .then(globalThis.eval)
which should behave like .then(x => globalThis.eval(x))
? π€·ββοΈ
this is what i read about direct/indirect eval: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#direct_and_indirect_eval
Direct eval call from a function sets the current execution context to be its surrounding module/script, which would be used as the referrer (i.e. host defined options) in the dynamic import calls. On the other hand, eval function call from a microtask sets the current execution context to be null
, as noted in the editor's notes (https://tc39.es/ecma262/#sec-HostLoadImportedModule):
An example of when referrer can be a Realm Record is in a web browser host. There, if a user clicks on a control given by
<button type="button" onclick="import('./foo.mjs')">Click me</button>
there will be no active script or module at the time the import() expression runs. More generally, this can happen in any situation where the host pushes execution contexts with null ScriptOrModule components onto the execution context stack.
I believe the condition here is identical to ShadowRealm.prototype.importValue
that when referrer script or module is null
in a dynamic import call, the module resolution should be performed relative to the origin of the process, like what a browser (and Deno) does.
This is not only limited to eval, for example this can also be reproduced with new Function
Promise.resolve(['import("./mod.mjs")'])
.then(Reflect.construct.bind(null, Function))
.then(Function.prototype.call.bind(Function.prototype.call))
.then(m => console.log(m.hello));
I think the answer to whether we should fallback comes down to:
importModuleDynamically
fallback in this case too? That would make escaping the configured hook possible, so at least warrants a note in the doc. This may also apply to custom loaders @nodejs/loaders On the other hand, eval function call from a microtask sets the current execution context to be null
Actually I think the editor notes does not apply to microtasks, from https://262.ecma-international.org/14.0/#sec-hostenqueuepromisejob the implementation should restore the active script from when the microtask is queued
Let scriptOrModule be GetActiveScriptOrModule() at the time HostEnqueuePromiseJob is invoked. If realm is not null, each time job is invoked the implementation must perform implementation-defined steps such that scriptOrModule is the active script or module at the time of job's invocation.
On the other hand, eval function call from a microtask sets the current execution context to be null
Actually I think the editor notes does not apply to microtasks, from https://262.ecma-international.org/14.0/#sec-hostenqueuepromisejob the implementation should restore the active script from when the microtask is queued
Let scriptOrModule be GetActiveScriptOrModule() at the time HostEnqueuePromiseJob is invoked. If realm is not null, each time job is invoked the implementation must perform implementation-defined steps such that scriptOrModule is the active script or module at the time of job's invocation.
@joyeecheung this is interesting. I tested on Chrome 117, Firefox 119, and Safari 17, none of them respected this statement. They resolve eval import calls relative to the origin of the document, rather than the initiating module. I believe either the ecma262 has to be corrected to reflect the web reality, or the browsers have to fix the behavior.
Iβm confused; why would anyone expect this to work? This would attempt to eval the toString of the module namespace object, and you simply canβt assume eval of any function will be portable.
@ljharb it's not about eval-ing the toString result of a module namespace. The problem here is how to resolve the specifier when the eval function is called with a string script containing dynamic imports and the eval function is invoked by built-in callbacks directly.
I just noticed the HTML spec deals with this exact situation in https://html.spec.whatwg.org/multipage/webappapis.html#hostmakejobcallback
Interesting, apparently WHATWG spec is explicitly saying that in this case the referrer should be the original script (and, on the contrary, if it's from a onclick
handler, there's no original script), while browsers don't actually implement that behavior..
@joyeecheung they took this to TC39 at one point but TC39 members had some concerns about the behavior because it could lead to an exploit by giving extra permissions to the eval'd string
From July 2020: https://docs.google.com/presentation/d/19S97ZqhibJABqzeP5ZU6Flk6TVgWzXuvJWFbNTTfpWs/edit#slide=id.p
Submitted https://github.com/tc39/ecma262/pull/3195 to propagate active scripts with arbitrary microtask callbacks (like FinalizationRegistry callbacks) in ecma262.
Version
v20.7.0
Platform
Linux PIG-2016 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
esm
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior? Why is that the expected behavior?
it shouldn't throw.
What do you see instead?
these DONT throw
these DO throw
Additional information
deno version of the same thing DOESN'T throw: