<div dir="ltr"><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small">Brendan filed <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1437147">https://bugzilla.mozilla.org/show_bug.cgi?id=1437147</a> to add warnings.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 13, 2018 at 9:47 AM, Kris Maglione <span dir="ltr"><<a href="mailto:kmaglione@mozilla.com" target="_blank">kmaglione@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Feb 09, 2018 at 09:28:39AM -0800, Brendan Dahl wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The linter does pick up the error. This was something I was testing<br>
locally. I generally don't think of running the linter to figure out why<br>
something isn't working (maybe I should be as it becomes more powerful?).<br>
However, I don't imagine I'll be the last developer to try to a use an API<br>
that is a web standard and be confused why it doesn't work though.<br>
</blockquote>
<br></span>
At the time, getting this shipped quickly and quietly was more important than the risk of bad developer experiences. But given the possible confusion, I sent this email after it shipped to release so people would at least have some warning.<br>
<br>
I agree that a warning when nodes are removed by the sanitizer probably makes sense as a follow-up at this point, though.<div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Fri, Feb 9, 2018 at 3:35 AM, Gijs Kruitbosch <<a href="mailto:gijskruitbosch@gmail.com" target="_blank">gijskruitbosch@gmail.com</a>><br>
wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Sorry about the waste of time. :-(<br>
<br>
Re: difficulty: it depends on your measure of 'very'. Internally the<br>
sanitization is whitelist-based. It is used in many places (not just for<br>
chrome-privileged docs), where it would be wrong to show warnings (possibly<br>
very *many* warnings!). It may be possible to add a flag to the sanitizer<br>
to log warnings for every removed attribute/element, and to pass that<br>
explicitly from the callsites that Kris added. It'd be worth filing a bug<br>
for that.<br>
<br>
To be honest, I would have expected there to have been an eslint error if<br>
using innerHTML/createContextualFrag<wbr>ment with anything other than a fixed<br>
string, which might have saved you a lot of headache. If that linter isn't<br>
catching createContextualFragment, then we need to fix the linter so that<br>
it does. If you were using a fixed string that got sanitized, can you point<br>
me to the bug/patch that you're working on? I'm having trouble thinking of<br>
cases where we use innerHTML with fixed content that would get sanitized in<br>
this way, without any injection/replacement, but perhaps I'm missing<br>
something.<br>
<br>
~ Gijs<br>
<br>
On 09/02/2018 02:18, Brendan Dahl wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Would it be very difficult to warn when something is sanitized and<br>
removed?<br>
<br>
I wasted a good deal of time trying to figure out why<br>
createContextualFragment wasn't working.<br>
<br>
On Fri, Feb 2, 2018 at 2:10 AM, Gijs Kruitbosch <<a href="mailto:gijskruitbosch@gmail.com" target="_blank">gijskruitbosch@gmail.com</a><br>
><br>
wrote:<br>
<br>
FWIW, if you're running into this with the usecase "I have a localized<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
string that needs to have links (or other markup) in it" and were<br>
formerly<br>
using getFormattedString combined with innerHTML, we now have a utility<br>
method that can help a little bit. Rather than hand-rolling splitting the<br>
string etc., on nightly you can use BrowserUtils.getLocalizedFragm<wbr>ent as<br>
a replacement. Given a document, raw string (fetch using getString /<br>
GetStringFromName instead of the "formatted" APIs), and DOM nodes to<br>
insert, it'll produce a DocumentFragment that you can<br>
appendChild/insertBefore etc., take care of splitting your strings for<br>
you,<br>
and will work with both indexed (%1$S) and non-indexed (%S) replacement<br>
points in the localized string. In the further future, I expect this type<br>
of problem will go away entirely because of Fluent.<br>
<br>
~ Gijs<br>
<br>
<br>
On 02/02/2018 07:13, Kris Maglione wrote:<br>
<br>
As of bug 1432966, any HTML injected into chrome-privileged documents[1]<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
is automatically sanitized to remove any possibility of script<br>
execution.<br>
The sanitization is whitelist-based, and only allows a limited set of<br>
HTML<br>
elements and attributes. All scripts, XUL nodes, or privileged URLs will<br>
automatically be removed. This change has been uplifted all the way to<br>
58<br>
release.<br>
<br>
If you're thinking about writing new code that injects HTML strings into<br>
chrome-privileged documents, please think again. Unless it's extremely<br>
simple, it probably won't be compatible with these changes (and will<br>
also<br>
be rejected by our default ESLint rules).<br>
<br>
Existing HTML injection in chrome documents is being gradually removed.<br>
Once that's done, the sanitization may be replaced with an outright<br>
prohibition.<br>
<br>
<br>
-Kris<br>
<br>
[1]: Using the usual HTML fragment creation methods such as `innerHTML`,<br>
`outerHTML`, `insertAdjacentHTML`, and `createContextualFragment`. Not,<br>
notably, when using document.write().<br>
______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
<br>
<br>
</blockquote>
<br>
______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
<br>
<br>
</blockquote></blockquote>
<br>
</blockquote></blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
</blockquote>
<br>
<br></div></div><span class="HOEnZb"><font color="#888888">
-- <br>
Kris Maglione<br>
Senior Firefox Add-ons Engineer<br>
Mozilla Corporation<br>
<br>
In the case of good books, the point is not how many of them you can<br>
get through, but rather how many can get through to you.<br>
--Mortimer J. Adler</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
</div></div></blockquote></div><br></div>