Bug 199610 – script elements created by the transformToFragment method of XSLTProcessor are not executed on insertion into DOM tree

• Component: XML
• Status: NEW

[Martin Honnen]

This bug report is about the failure of WebKit to ensure that "script" elements generated by the "transformToFragment" method of the "XSLTProcessor" object are executed on insertion into the DOM tree.

There is no W3C or WhatWG specification of the XSLTProcessor object and its API methods like transformToFragment but as Mozilla introduced that API and other browsers like WebKit or Chrome decided to offer the same API the Mozilla implementation's behaviour can be regarded as a reference; furthermore, for the "transformToFragment" method both the W3C HTML5 spec (https://www.w3.org/TR/html50/scripting-1.html#scriptTagXSLT) as well as the WhatWG HTML spec (https://html.spec.whatwg.org/multipage/scripting.html#scriptTagXSLT) do include some notes on the exact behaviour of this method and generated script elements:

The XSLTProcessor.transformToFragment() method needs to create a fragment that is equivalent to one built manually by creating the elements using document.createElementNS(). For instance, it needs to create script elements that aren't "parser-inserted" and that don't have their "already started" flag set, so that they will execute when the fragment is inserted into a document.

Mozilla browsers implement this but neither WebKit/Safari nor Chromium/Chrome implement it; Chrome has an open bug https://bugs.chromium.org/p/chromium/issues/detail?id=266305 on this.

My own test cases are at https://martin-honnen.github.io/xslt/2019/transformToFragment-Test3.html (XSLTProcessor.transformToFragment creating an HTML fragment with an HTML "script" element with inline code) and https://martin-honnen.github.io/xslt/2019/transformToFragment-Test5.html (XSLTProcessor.transformToFragment creating an HTML fragment with an HTML "script" element referencing an internal script document).

These work in Mozilla, meaning the XSLT generated "script" elements are executed on insertion into the DOM tree, but fail in WebKit (and Chrome) where the scripts are not executed on insertion into the DOM tree.

There is also some ongoing work to integrate two test cases into the wpt test suite, the current pull requests result on Safari can be seen at https://staging.wpt.fyi/results/xslt/transformToFragment.window.html?diff&filter=ADC&run_id=212960008&run_id=248040010.

Looking at the implementation of "transformToFragment" in both Chromium and WebKit it seems the infrastructure to allow that generated "script" elements can be executed on insertion into the DOM tree already exists as the Range method "createContextualFragment" has the same demand and both Chromium and WebKit implement this with a particular ParserContentPolicy and the behaviour of createContextualFragment is compatible with Mozilla.

So the only thing missing in the WebKit source code seems to use the ParserContentPolicy AllowScriptingContentAndDoNotMarkAlreadyStarted explicitly in the method "createFragmentForTransformToFragment" where the "parseHTML" and "parseXML" methods of a fragment are called; so basically the fix would be to change the call

    fragment->parseHTML(sourceString, fakeBody.ptr());
to
    fragment->parseHTML(sourceString, fakeBody.ptr(), AllowScriptingContentAndDoNotMarkAlreadyStarted);

and the call 

    fragment->parseXML(sourceString, 0);
to
   fragment->parseXML(sourceString, 0, AllowScriptingContentAndDoNotMarkAlreadyStarted);

in that method https://github.com/WebKit/webkit/blob/master/Source/WebCore/editing/markup.cpp#L1217.

I am not set up to build Safari but in Chromium, as outlined in the relevant https://bugs.chromium.org/p/chromium/issues/detail?id=266305, I have tried the appropriate fix and then the test cases do work.

[Alexey Proskuryakov]

Would you be willing to try submitting a patch anyway, as it will be processed and tests will run automatically? It will certainly be more annoying than with a local build, but it's doable.

[Martin Honnen]

(In reply to Alexey Proskuryakov from comment #1)
> Would you be willing to try submitting a patch anyway, as it will be
> processed and tests will run automatically? It will certainly be more
> annoying than with a local build, but it's doable.

Does it suffice to clone the WebKit git repository, branch, edit the one file, commit locally and attach a git diff here?

Or would I need to use the WebKit tools like described in https://webkit.org/contributing-code/#create-the-patch (e.g. Tools/Scripts/webkit-patch upload)? 

I am currently not sure what kind of setup those tools rely on, the list in https://webkit.org/webkit-on-windows/#installing-developer-tools seems rather long with all the particular Perl, Ruby, Python etc installs needed.

[Alexey Proskuryakov]

> Does it suffice to clone the WebKit git repository, branch, edit the one file, commit locally and attach a git diff here?

Pretty much.

webkit-patch is not necessary, I personally never use it. A ChangeLog is needed though, and the prepare-ChangeLog script helps by adding a template to the top. But you can also make a similar looking entry manually.

[Alexey Proskuryakov]

The only concern that I have about this change - and will defer to others to answer - is whether it opens any new possibilities for script execution at inopportune time. It *seems* like it's OK, but the consequences of untimely script execution are so bad that someone better be sure.

[Martin Honnen]

Created attachment 373949 [details]
fixing WebCore::createFragmentForTransformToFragment to use AllowScriptingContentAndDoNotMarkAlreadyStarted in parseHTML and parseXML calls

I have tried my best to create a changelog entry manually but I not sure whether I am supposed to link to external test cases (i.e. the ones I mentioned in the bug report) and where that "rdar" link is created from, so I have left these two issues out of the change log entry for the time being.

[EWS Watchlist]

Attachment 373949 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 7 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Honnen]

Comment on attachment 373949 [details]
fixing WebCore::createFragmentForTransformToFragment to use AllowScriptingContentAndDoNotMarkAlreadyStarted in parseHTML and parseXML calls

Trying to fix the tab issues raised by style bot on the ChangeLog entry.

[Martin Honnen]

Created attachment 374000 [details]
script elements created by the transformToFragment method of XSLTProcessor are not executed on insertion into DOM tree

Trying to fix issues with tab characters raised by style bot.

[Ryosuke Niwa]

Comment on attachment 374000 [details]
script elements created by the transformToFragment method of XSLTProcessor are not executed on insertion into DOM tree

It seems like this would be a pretty serious XSS risk for any website or apps embedding WKWebView / UIWebView relying on the existing to not execute scripts.