var HTMLString =
"<div>Hi!</div>" +
"<script>alert('foo');</script>";
const component = {
view() {
return (m.trust(HTMLString));
}
};
m.mount(document.body, component);
Expected the alert to be shown as it did when used with mithril 0.2.3.
With mithril 1.1.6, the alert does not get shown even though the alert should not be escaped .
We wanted to allow certain scripts to be executed and had used m.trust in the component. It used to work with 0.2.3 and broke when we upgraded. I didn't find anything in the documentation regarding this breaking change. So, I'm not sure if we have to do this in a different way.
That's really odd @varshavaradarajan. I can find discussion in v0.2 about specifically enabling this functionality, and documentation explaining that it isn't intended functionality in v1, but you're right - there's nothing about it in the change log and I can't find any discussion about removing it.
What we could add is support for hooks directly defined on trusted nodes: m.trust(htmlSource, attrs). The same can be achieved with m.fragment([m.trust("...")], hooks), without modifications.
The code from #1051 can be adapted as more or less as is (I'd avoid running it on every node in the parent, and limit it to the actual nodes bound to the trusted vnode (using vnode.domand vnode.domSize).
I think the problem relies in the fact the innerHTML created script-nodes do not get evaluated.
I would say won't fix!
Yeah, script elements don't get evaluated in most dynamically-generated HTML code for security reasons. This is required per spec, and I don't see browsers wanting to change that behavior.
Closing.
Yeah, script elements don't get evaluated in most dynamically-generated HTML code for security reasons.
why was it working before, in mithril 0.2.3?
[Back then we used insertAdjacentHTML], which somehow did the trick? I don't see why we shouldn't reinstate this — it's a regression, right?
Yeah, it's a bug. It just happened to be a breaking change when an upgrade was performed. It makes sense to not go back to this. For security reasons. Just wanted to know why it was working before.
@varshavaradarajan how do you think we should proceed? Document the change in policy? …Or reintroduce the functionality with a warning?
We have a sort of legitimate usecase (see the context above) for why this could be useful. However, seeing as nobody else has reported this and that it has dangerous implications, I think that documenting the change should be fine. In addition, users have https://mithril.js.org/trust.html#scripts-that-do-not-run now to stop them from having scripts in the html strings. Perhaps, you can revisit this if more people report the issue?
@varshavaradarajan If you want the old behavior, try this:
m.fragment wrapping the m.trust.oncreate hook that starts with vnode.children[0].dom and for the next vnode.children[0].domSize elements, does elem.querySelectorAll("script") and for each element in that, do script.parentNode.replaceNode(script.cloneNode(true), script).This is wholly untested, so I'm not 100% sure it'll work, but I'm pretty sure it should work. This could easily be wrapped into a function or component, in case this thing is common.
@isiahmeadows -- Many thanks for the provided solution, We were able to evaluate script tags.
For further references to allow scripts, here is an example:
var HTMLString = "<div>" +
"<div>Click me!</div>" +
"<script>document.body.onclick = function() {alert('script works!'); }</script>" +
"</div>";
var oncreate = function (vnode) {
vnode.children.forEach((elem) => {
elem.dom.querySelectorAll("script").forEach((script) => {
var newScript = document.createElement('script');
newScript.innerHTML = script.innerHTML;
script.parentNode.replaceChild(newScript, script);
})
});
};
const component = {
view() {
return (m.fragment({oncreate: oncreate}, [m.trust(HTMLString)]));
}
};
m.mount(document.body, component);
Most helpful comment
@varshavaradarajan If you want the old behavior, try this:
m.fragmentwrapping them.trust.oncreatehook that starts withvnode.children[0].domand for the nextvnode.children[0].domSizeelements, doeselem.querySelectorAll("script")and for each element in that, doscript.parentNode.replaceNode(script.cloneNode(true), script).This is wholly untested, so I'm not 100% sure it'll work, but I'm pretty sure it should work. This could easily be wrapped into a function or component, in case this thing is common.