You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Promise.resolve('import("node:fs")').then(eval) throws TypeError: Invalid host defined options #49726
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 most1 runtimes but Node.js:
changed the title [-]`Promise.resolve(`import("node:fs")`).then(eval)` throws `TypeError: Invalid host defined options`[/-][+]``Promise.resolve(`import("node:fs")`).then(eval)`` throws `TypeError: Invalid host defined options`[/+]on Sep 20, 2023
changed the title [-]``Promise.resolve(`import("node:fs")`).then(eval)`` throws `TypeError: Invalid host defined options`[/-][+]`Promise.resolve('import("node:fs")').then(eval)` throws `TypeError: Invalid host defined options`[/+]on Sep 20, 2023
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.
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.
I think the answer to whether we should fallback comes down to:
Do we think falling back should be by-design - at least as far as what the spec currently says, that would not be non-conformant. On the other hand, throwing is probably not non-conformant either. This is ultimately implementation-defined.
If we should fall back, what is the base we should fallback to?
Should scripts/modules compiled using vm methods + 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
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.
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.
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
Activity
aduh95 commentedon Sep 20, 2023
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 most1 runtimes but Node.js:
Footnotes
I'd like to say all, but there are so many runtimes these days I surely missed at least one ↩
[-]`Promise.resolve(`import("node:fs")`).then(eval)` throws `TypeError: Invalid host defined options`[/-][+]``Promise.resolve(`import("node:fs")`).then(eval)`` throws `TypeError: Invalid host defined options`[/+][-]``Promise.resolve(`import("node:fs")`).then(eval)`` throws `TypeError: Invalid host defined options`[/-][+]`Promise.resolve('import("node:fs")').then(eval)` throws `TypeError: Invalid host defined options`[/+]jcbhmr commentedon Sep 20, 2023
jcbhmr commentedon Sep 20, 2023
The two spots I could find from a GitHub search for "Invalid host defined options" are these:
node/src/module_wrap.cc
Lines 572 to 576 in b64f620
node/deps/v8/src/d8/d8.cc
Lines 1252 to 1255 in b64f620
jcbhmr commentedon Sep 20, 2023
aduh95 commentedon Sep 21, 2023
The following check is failing (
options->Length()is supposed to be 9, it's 0 here):node/src/module_wrap.cc
Lines 568 to 569 in 26c8858
Here's a backtrace (maybe I need a debug build):
targos commentedon Sep 22, 2023
host_defined_optionsis empty because of this condition:node/deps/v8/src/execution/isolate.cc
Lines 5113 to 5116 in c2cd744
maybe_referrercomes from here:node/deps/v8/src/runtime/runtime-module.cc
Lines 36 to 37 in c2cd744
joyeecheung commentedon Sep 22, 2023
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.joyeecheung commentedon Sep 22, 2023
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.
joyeecheung commentedon Sep 22, 2023
Upstream issue https://bugs.chromium.org/p/v8/issues/detail?id=14336
jcbhmr commentedon Sep 22, 2023
@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?
aduh95 commentedon Sep 22, 2023
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.
6 remaining items
joyeecheung commentedon Sep 26, 2023
I think the answer to whether we should fallback comes down to:
importModuleDynamicallyfallback 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/loadersjoyeecheung commentedon Sep 27, 2023
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
legendecas commentedon Oct 9, 2023
@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.
ljharb commentedon Oct 9, 2023
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.
legendecas commentedon Oct 9, 2023
@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.
aduh95 commentedon Oct 10, 2023
I just noticed the HTML spec deals with this exact situation in https://html.spec.whatwg.org/multipage/webappapis.html#hostmakejobcallback
joyeecheung commentedon Oct 11, 2023
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
onclickhandler, there's no original script), while browsers don't actually implement that behavior..bmeck commentedon Oct 11, 2023
@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
legendecas commentedon Oct 13, 2023
Submitted tc39/ecma262#3195 to propagate active scripts with arbitrary microtask callbacks (like FinalizationRegistry callbacks) in ecma262.
vm: allow dynamic import with a referrer realm
vm: allow dynamic import with a referrer realm
vm: allow dynamic import with a referrer realm
vm: allow dynamic import with a referrer realm