Update Trusted Types enforcement for document.write/writeln #10328

lukewarlow · 2024-05-07T12:05:37Z

Update Trusted Types enforcement for document.write/writeln

This changes from using HTMLString to a TrustedHTML or DOMString union.

This also changes the timing of the default policy call.

At least two implementers are interested (and none opposed):
- Gecko Trusted Types mozilla/standards-positions#20
- Chromium (Update Trusted Types enforcement for document.write/writeln #10328 (comment))
Tests are written and can be reviewed and commented upon at:
- WebKit export of https://bugs.webkit.org/show_bug.cgi?id=273819 web-platform-tests/wpt#46141
- https://github.com/web-platform-tests/wpt/blob/91e42694d29c23fbc075e7c89af3c23d53101dd0/trusted-types/Document-write-exception-order.xhtml
Implementation bugs are filed:
- Chromium: https://issues.chromium.org/issues/339317628
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1508286 (general meta bug)
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=273819
MDN issue is filed: …
The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

/dom.html ( diff )
/dynamic-markup-insertion.html ( diff )
/infrastructure.html ( diff )

lukewarlow · 2024-05-07T12:07:03Z

One thing to note is that the behaviour difference for default policy call is actually aligning with the shipping implementation (Chromium)

source

annevk

All comments apply to both algorithms, though I've made some only on one of them. Apologies for the slight mess.

source

lukewarlow · 2024-05-07T14:45:29Z

Thanks for the reviews so far hopefully it's looking like you expect now. Definitely cleaner than the first draft.

annevk

Yeah this looks good, thanks. Although now I wonder if we should have a shared operation as they are essentially copies? Maybe pass text to the document write steps and add an argument for a trailing LF? I guess @otherdaniel should agree to this as well at least.

https://bugs.webkit.org/show_bug.cgi?id=273819 Reviewed by NOBODY (OOPS!). This patch switches document.write and writeln to use a union IDL type and single call to default policy. This brings it close to chromium behaviour. See whatwg/html#10328 * LayoutTests/imported/w3c/web-platform-tests/trusted-types/Document-write-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/trusted-types/Document-write.html: * LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Document-write-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Document-write.html: * Source/WebCore/dom/Document+HTML.idl: * Source/WebCore/dom/Document.cpp: (WebCore::Document::write): (WebCore::Document::writeln): * Source/WebCore/dom/Document.h:

otherdaniel · 2024-05-07T17:18:35Z

Thanks! We're happy with this, too.

lukewarlow · 2024-05-08T09:06:13Z

@annevk am I okay to say this has webkit implementor interest?

The webkit pr itself is being merged but I can do follow ups to change it. Mainly want it in to move away from the IDL attribute.

https://bugs.webkit.org/show_bug.cgi?id=273819 Reviewed by Ryosuke Niwa. This patch switches document.write and writeln to use a union IDL type and single call to default policy. This brings it close to chromium behaviour. See whatwg/html#10328 * LayoutTests/imported/w3c/web-platform-tests/trusted-types/Document-write-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/trusted-types/Document-write.html: * LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Document-write-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Document-write.html: * Source/WebCore/dom/Document+HTML.idl: * Source/WebCore/dom/Document.cpp: (WebCore::Document::write): (WebCore::Document::writeln): * Source/WebCore/dom/Document.h: Canonical link: https://commits.webkit.org/278501@main

annevk · 2024-05-08T12:48:09Z

I think the deduplication still needs to happen.

As for implementer interest, it's a bit tricky. WebKit hasn't established a position as of yet in part due to the many outstanding issues with Trusted Types. This refactoring on its own seems okay, but I would not want that to be taken as WebKit being okay with it in general.

lukewarlow · 2024-05-09T13:37:58Z

I've refactored this to move things into the document write steps.

source

https://bugs.webkit.org/show_bug.cgi?id=273819 Reviewed by NOBODY (OOPS!). This patch switches document.write and writeln to use a union IDL type and single call to default policy. See whatwg/html#10328 * LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Document-write-expected.txt: * Source/WebCore/dom/DOMImplementation.cpp: (WebCore::DOMImplementation::createHTMLDocument): * Source/WebCore/dom/Document+HTML.idl: * Source/WebCore/dom/Document.cpp: (WebCore::Document::write): (WebCore::Document::writeln): * Source/WebCore/dom/Document.h: * Source/WebKitLegacy/mac/DOM/DOMHTMLDocument.mm: (-[DOMHTMLDocument write:]): (-[DOMHTMLDocument writeln:]):

This changes from using HTMLString to a TrustedHTML or DOMString union. This also changes the timing of the default policy call.

zcorpan · 2024-05-28T22:02:33Z

No need for any change to MDN here as far as I can tell. Everything seems in order, so merging.

https://bugs.webkit.org/show_bug.cgi?id=273819 Reviewed by NOBODY (OOPS!). This patch switches document.write and writeln to use a union IDL type and single call to default policy. See whatwg/html#10328 * LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Document-write-expected.txt: * Source/WebCore/dom/DOMImplementation.cpp: (WebCore::DOMImplementation::createHTMLDocument): * Source/WebCore/dom/Document+HTML.idl: * Source/WebCore/dom/Document.cpp: (WebCore::Document::trustedWrite): (WebCore::Document::write): (WebCore::Document::writeln): * Source/WebCore/dom/Document.h: * Source/WebKitLegacy/mac/DOM/DOMHTMLDocument.mm: (-[DOMHTMLDocument write:]): (-[DOMHTMLDocument writeln:]):

lukewarlow mentioned this pull request May 7, 2024

Spec / implementation mismatch with document.write/writeln w3c/trusted-types#510

Closed

lukewarlow force-pushed the tt-document-write branch from 424f608 to 8290d72 Compare May 7, 2024 12:10

lukewarlow commented May 7, 2024

View reviewed changes

source Outdated Show resolved Hide resolved

annevk reviewed May 7, 2024

View reviewed changes

source Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Show resolved Hide resolved

source Outdated Show resolved Hide resolved

lukewarlow force-pushed the tt-document-write branch from 8290d72 to 2dc877c Compare May 7, 2024 14:13

annevk reviewed May 7, 2024

View reviewed changes

lukewarlow marked this pull request as ready for review May 7, 2024 15:13

lukewarlow mentioned this pull request May 7, 2024

Update implementation of TT enforcement for document.write(ln) WebKit/WebKit#28238

Merged

lukewarlow requested a review from annevk May 9, 2024 13:38

lukewarlow mentioned this pull request May 14, 2024

Remove StringContext attribute w3c/trusted-types#498

Draft

zcorpan reviewed May 15, 2024

View reviewed changes

source Show resolved Hide resolved

lukewarlow force-pushed the tt-document-write branch from b97d4a0 to b7dbca8 Compare May 15, 2024 13:38

zcorpan approved these changes May 15, 2024

View reviewed changes

lukewarlow mentioned this pull request May 28, 2024

Switch trusted types sinks to use union type #10286

Open

5 tasks

Update Trusted Types enforcement for document.write/writeln

ae1f8f1

This changes from using HTMLString to a TrustedHTML or DOMString union. This also changes the timing of the default policy call.

lukewarlow force-pushed the tt-document-write branch from b7dbca8 to ae1f8f1 Compare May 28, 2024 21:55

zcorpan merged commit feb379d into whatwg:main May 28, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Trusted Types enforcement for document.write/writeln #10328

Update Trusted Types enforcement for document.write/writeln #10328

lukewarlow commented May 7, 2024 •

edited by pr-preview bot

lukewarlow commented May 7, 2024

annevk left a comment

lukewarlow commented May 7, 2024

annevk left a comment

otherdaniel commented May 7, 2024

lukewarlow commented May 8, 2024 •

edited

annevk commented May 8, 2024

lukewarlow commented May 9, 2024

zcorpan commented May 28, 2024

Update Trusted Types enforcement for document.write/writeln #10328

Update Trusted Types enforcement for document.write/writeln #10328

Conversation

lukewarlow commented May 7, 2024 • edited by pr-preview bot

lukewarlow commented May 7, 2024

annevk left a comment

Choose a reason for hiding this comment

lukewarlow commented May 7, 2024

annevk left a comment

Choose a reason for hiding this comment

otherdaniel commented May 7, 2024

lukewarlow commented May 8, 2024 • edited

annevk commented May 8, 2024

lukewarlow commented May 9, 2024

zcorpan commented May 28, 2024

lukewarlow commented May 7, 2024 •

edited by pr-preview bot

lukewarlow commented May 8, 2024 •

edited