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.

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

What shoud script element's textContent's sink string be? #572

Closed
mbrodesser-Igalia opened this issue Jan 1, 2025 · 7 comments
Closed

Comments

@mbrodesser-Igalia
Copy link
Collaborator

@fred-wang
Copy link
Collaborator

https://w3c.github.io/trusted-types/dist/spec/#dom-htmlscriptelement-textcontent states "HTMLScriptElement textContent".

I think the spec is quite clear, it should be "HTMLScriptElement textContent".

A WPT requires "HTMLScriptElement text": https://searchfox.org/mozilla-central/rev/1f2c929f93eabe6d753096e47641ebba68b16601/testing/web-platform/tests/trusted-types/default-policy-callback-arguments.html#27-28,45-46

This test checks the correct sink names here:

assert_equals(args[2], current_case[2], "Expecting the sink name.");

The switch statement you mention is a noop: args[1] is supposed to be the expectedType’s type (see https://w3c.github.io/trusted-types/dist/spec/#process-value-with-a-default-policy-algorithm and https://w3c.github.io/trusted-types/dist/spec/#abstract-opdef-get-trusted-type-policy-value) which is a string while TrustedHTML, TrustedScript and TrustedScriptURL are objects.

The same mistake seems to happen for the switch statement added in web-platform-tests/wpt#46089

I'll try and fix that.

cc @ziransun @lukewarlow

@fred-wang
Copy link
Collaborator

This is fixed by web-platform-tests/wpt#49909

(I cannot close the issue because my github account does not have proper permission for this repo)

@evilpie
Copy link

evilpie commented Jan 6, 2025

I think the spec is quite clear, it should be "HTMLScriptElement textContent".

To me it looks like the test that was updated by web-platform-tests/wpt#49909 still uses "HTMLScriptElement text"

https://github.com/web-platform-tests/wpt/blob/9a0b9963e02f06989a4f63a80b7091956a1d23c9/trusted-types/default-policy-report-only.html#L77

@lukewarlow
Copy link
Member

That test should be text because that's the sink that's tested in it.

@fred-wang
Copy link
Collaborator

Yeah basically when setting HTMLScriptElement's textContent, the sink string should be textContent. And when setting HTMLScriptElement's text the sink string should be text.

The confusion was with default-policy-callback-arguments.html, which was setting HTMLScriptElement's textContent but had two contradictory assertions for the sink string (checking it's both textContent and text).

As explained above, the second assertion was actually not reachable (and wrong) so I removed it. I took the opportunity to fix unreachable assertions in other tests.

@evilpie
Copy link

evilpie commented Jan 6, 2025

Sorry, makes sense!

@lukewarlow
Copy link
Member

Great catch btw sorry I didn't spot that mistake when reviewing/writing the relevant tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants