Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Trusted types attributes #1268

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Trusted types attributes #1268

Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
184fbb3
Draft integration with Trusted Types, take 2.
koto Jan 23, 2024
ebae0ea
Added integration in 'set an attribute value'.
koto Jan 24, 2024
3dc6eba
Removed stringification.
koto Jan 24, 2024
f88ea29
Fixed formatting.
koto Jan 25, 2024
966291f
Fixed copy-paste error in setAttribute() and indentation.
koto Jan 25, 2024
065ee2e
Fix build
lukewarlow Mar 28, 2024
19e8f54
Change code to pass sink values through to TT code.
lukewarlow Mar 28, 2024
964b8cd
Move validate and set attribute into append an attribute
lukewarlow Apr 10, 2024
8db6d26
Remove sink values from spec
lukewarlow Apr 11, 2024
271a670
Revert changes to setAttributeNode and setAttributeNodeNS method steps
lukewarlow Apr 22, 2024
2427ef4
Remove throw from set an attribute and move TT check down to replace …
lukewarlow Apr 22, 2024
b5de89b
Revert unneeded changes
lukewarlow Apr 22, 2024
cd0415b
Address comment
lukewarlow Apr 22, 2024
d2adea4
Add missing and
lukewarlow Apr 22, 2024
99ae585
Address some comments
lukewarlow May 7, 2024
f76755c
Re-add early return
lukewarlow May 7, 2024
e5be35d
Change dfn to use given rather than with
lukewarlow May 7, 2024
982ef71
Update how the enforcement is done
lukewarlow May 16, 2024
f584e23
Add tentative enforcement for toggleAttribute
lukewarlow May 16, 2024
9b74d93
Update enforcement for toggleAttribute to match behaviour observed in…
lukewarlow May 16, 2024
d836a23
Revert "Update enforcement for toggleAttribute to match behaviour obs…
lukewarlow Jun 11, 2024
b0ecd77
Revert "Add tentative enforcement for toggleAttribute"
lukewarlow Jun 11, 2024
d6b3b5a
Reintroduce infrastructure changes
lukewarlow Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 83 additions & 20 deletions dom.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ spec:html; type:element
<p>This specification depends on the Infra Standard. [[!INFRA]]

<p>Some of the terms used in this specification are defined in <cite>Encoding</cite>,
<cite>Selectors</cite>, <cite>Web IDL</cite>, <cite>XML</cite>, and <cite>Namespaces in XML</cite>.
<cite>Selectors</cite>, <cite>Trusted Types</cite>, <cite>Web IDL</cite>, <cite>XML</cite>, and
<cite>Namespaces in XML</cite>.
[[!ENCODING]]
[[!SELECTORS4]]
[[!TRUSTED-TYPES]]
[[!WEBIDL]]
[[!XML]]
[[!XML-NAMES]]
Expand Down Expand Up @@ -6133,8 +6135,8 @@ interface Element : Node {
sequence&lt;DOMString> getAttributeNames();
DOMString? getAttribute(DOMString qualifiedName);
DOMString? getAttributeNS(DOMString? namespace, DOMString localName);
[CEReactions] undefined setAttribute(DOMString qualifiedName, DOMString value);
[CEReactions] undefined setAttributeNS(DOMString? namespace, DOMString qualifiedName, DOMString value);
[CEReactions] undefined setAttribute(DOMString qualifiedName, (TrustedType or DOMString) value);
[CEReactions] undefined setAttributeNS(DOMString? namespace, DOMString qualifiedName, (TrustedType or DOMString) value);
[CEReactions] undefined removeAttribute(DOMString qualifiedName);
[CEReactions] undefined removeAttributeNS(DOMString? namespace, DOMString localName);
[CEReactions] boolean toggleAttribute(DOMString qualifiedName, optional boolean force);
Expand Down Expand Up @@ -6510,6 +6512,16 @@ steps:
<a for=Attr>value</a>.
</ol>

<p>To <dfn>verify attribute value</dfn>
{{TrustedType}} or string <var>value</var> for an <a>attribute</a> <var>attribute</var>, given an
<a for=/>Element</a> <var>element</var>:

<ol>
<li><p>Return the result of calling
<a abstract-op>get Trusted Types-compliant attribute value</a> for <var>attribute</var>, with
<var>element</var>, <var>value</var>. [[!TRUSTED-TYPES]]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this may throw. Do the callers of this method handle that in some way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will end up rethrowing the exception which is what's expected.

</ol>

<hr>

<div algorithm>
Expand Down Expand Up @@ -6572,6 +6584,11 @@ string <var>namespace</var> (default null):</p>

<li><p>If <var>oldAttr</var> is <var>attr</var>, return <var>attr</var>.
lukewarlow marked this conversation as resolved.
Show resolved Hide resolved

<li><p>Let <var>verifiedValue</var> be the result of calling <a>verify attribute value</a>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This covers, setAttributeNode, setAttributeNodeNS, setNamedItem and setNamedItemNS

<var>attr</var>'s <a for=Attr>value</a> for <var>attr</var>, with <var>element</var>.

<li><p>Set <var>attr</var>'s <a for=Attr>value</a> to <var>verifiedValue</var>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so doing the verification may run scripts. And that means oldAttr might not be anymore in the element it used to be. Could that cause issues? Could the value be validated first for certain kind of element but then used on some other kind of element?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default policy doesn't provide context about which element an attribute is set on only the name of the attribute. In this case this algorithm is triggered by APIs such as setAttributeNode or setNamedItem.

So I don't think there's anything that can happen here that's too bad. Also any mechanism you use inside of the default policy will itself trigger the default policy so it should be fine?

<li><p>If <var>oldAttr</var> is non-null, then <a lt="replace an attribute">replace</a>
<var>oldAttr</var> with <var>attr</var>.

Expand All @@ -6583,23 +6600,46 @@ string <var>namespace</var> (default null):</p>

<div algorithm>
<p>To <dfn export id=concept-element-attributes-set-value>set an attribute value</dfn> given an
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This algorithm covers setAttributeNS

<a for=/>element</a> <var>element</var>, a string <var>localName</var>, a string <var>value</var>,
an optional null or string <var>prefix</var> (default null), and an optional null or string
<var>namespace</var> (default null):
<a for=/>element</a> <var>element</var>, a string <var>localName</var>, a string or {{TrustedType}}
<var>value</var>, an optional null or string <var>prefix</var> (default null), an optional null
or string <var>namespace</var> (default null), and an optional boolean <var>verify</var>
(default false):

<ol>
<li>Let <var>attribute</var> be the result of
<a lt="get an attribute by namespace and local name">getting an attribute</a> given
<var>namespace</var>, <var>localName</var>, and <var>element</var>.

<li>If <var>attribute</var> is null, create an <a>attribute</a> whose <a for=Attr>namespace</a> is
<var>namespace</var>, <a for=Attr>namespace prefix</a> is <var>prefix</var>,
<a for=Attr>local name</a> is <var>localName</var>, <a for=Attr>value</a> is <var>value</var>, and
<a for=Node>node document</a> is <var>element</var>'s <a for=Node>node document</a>, then
<a lt="append an attribute">append</a> this <a>attribute</a> to <var>element</var>, and then
return.
<li><p>Let <var>attributeExists</var> be false if <var>attribute</var> is null, and true otherwise.

<li><p>If <var>attributeExists</var> is false, set <var>attribute</var> to an <a>attribute</a>
whose <a for=Attr>namespace</a> is <var>namespace</var>, <a for=Attr>namespace prefix</a> is
<var>prefix</var>, <a for=Attr>local name</a> is <var>localName</var>, <a for=Attr>value</a> is
<var>value</var>, and <a for=Node>node document</a> is <var>element</var>'s <a for=Node>node
document</a>.

<li><p>Let <var>verifiedValue</var> be <var>value</var>.

<li><p>If <var>verify</var> is true:
<ol>
<li><p>Set <var>verifiedValue</var> to the result of calling <a>verify attribute value</a>
<var>value</var> for <var>attribute</var>, with <var>element</var>.

<li><p>Set <var>attributeExists</var> to true if <var>element</var> <a lt="has an attribute">has
an attribute</a> <var>attribute</var>; otherwise false.
</ol>
</li>

<li><p><a lt="change an attribute">Change</a> <var>attribute</var> to <var>value</var>.
<li><p>If <var>attributeExists</var> is true, <a lt="change an attribute">change</a>
<var>attribute</var> to <var>verifiedValue</var>.

<li><p>Otherwise:
<ol>
<li><p>Set <var>attribute</var>'s <a for=Attr>value</a> to <var>verifiedValue</var>.

<li><p><a lt="append an attribute">Append</a> this <a>attribute</a> to
<var>element</var>.
</ol>
</ol>
</div>

Expand Down Expand Up @@ -6860,12 +6900,29 @@ method steps are:
and null otherwise.
<!-- This is step 2 of "get an attribute by name", modified as appropriate -->

<li><p>If <var>attribute</var> is null, create an <a>attribute</a> whose
<a for=Attr>local name</a> is <var>qualifiedName</var>, <a for=Attr>value</a> is
<var>value</var>, and <a for=Node>node document</a> is <a>this</a>'s <a for=Node>node document</a>,
then <a lt="append an attribute">append</a> this <a>attribute</a> to <a>this</a>, and then return.
<li><p>Let <var>attributeExists</var> be false if <var>attribute</var> is null, and true otherwise.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This algorithm covers setAttribute


<li><p>If <var>attributeExists</var> is false, set <var>attribute</var> to an <a>attribute</a>
whose <a for=Attr>local name</a> is <var>qualifiedName</var>, <a for=Attr>value</a> is
<var>value</var>, and <a for=Node>node document</a> is <a>this</a>'s <a for=Node>node
document</a>.

<li><p>Let <var>verifiedValue</var> be the result of calling <a>verify attribute value</a>
<var>value</var> for <var>attribute</var>, with <a>this</a>.

<li><p>Set <var>attributeExists</var> to true if <a>this</a> <a lt="has an attribute">has an
attribute</a> <var>attribute</var>; otherwise false.

<li><p><a lt="change an attribute">Change</a> <var>attribute</var> to <var>value</var>.
<li><p>If <var>attributeExists</var> is true, <a lt="change an attribute">change</a>
<var>attribute</var> to <var>verifiedValue</var>.

<li><p>Otherwise:
<ol>
<li><p>Set <var>attribute</var>'s <a for=Attr>value</a> to <var>verifiedValue</var>.

<li><p><a lt="append an attribute">Append</a> this <a>attribute</a> to
<var>element</var>.
</ol>
</ol>

<p>The
Expand All @@ -6877,7 +6934,7 @@ method steps are:
passing <var>namespace</var> and <var>qualifiedName</var> to <a>validate and extract</a>.

<li><p><a>Set an attribute value</a> for <a>this</a> using <var>localName</var>, <var>value</var>,
and also <var>prefix</var> and <var>namespace</var>.
<var>prefix</var>, <var>namespace</var> and true.
</ol>

<p>The
Expand Down Expand Up @@ -7435,7 +7492,13 @@ string <var>value</var>, run these steps:
<li><p>If <var>attribute</var>'s <a for=Attr>element</a> is null, then set <var>attribute</var>'s
<a for=Attr>value</a> to <var>value</var>.

<li><p>Otherwise, <a lt="change an attribute">change</a> <var>attribute</var> to <var>value</var>.
<li><p>Otherwise:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This covers attr.nodeValue, attr.value, and attr.textContent.

<ol>
<li><p>Let <var>verifiedValue</var> be the result of calling <a>verify attribute value</a>
<var>value</var> for <var>attribute</var>, with <a>this</a>.

<li><p><a lt="change an attribute">Change</a> <var>attribute</var> to <var>verifiedValue</var>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this lead to effectively null pointer crashes in algorithms if attribute has been moved out from an element by the tt callback?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess potentially if some sub algorithm of change an attribute is assuming that element must exist? Fwiw in practice this doesn't cause issues from what I can see.

Not sure what the best fix here would be?

</ol>
</ol>

<p>The {{Attr/value}} setter steps are to <a>set an existing attribute value</a> with <a>this</a>
Expand Down
Loading