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

Stringification of TrustedHTML with null-data needs to be specified #469

Open
mbrodesser-Igalia opened this issue Mar 6, 2024 · 21 comments · Fixed by #527
Open

Stringification of TrustedHTML with null-data needs to be specified #469

mbrodesser-Igalia opened this issue Mar 6, 2024 · 21 comments · Fixed by #527
Milestone

Comments

@mbrodesser-Igalia
Copy link
Collaborator

https://w3c.github.io/trusted-types/dist/spec/#abstract-opdef-create-a-trusted-type

@mbrodesser-Igalia
Copy link
Collaborator Author

The policyValue may be null.

@mbrodesser-Igalia
Copy link
Collaborator Author

Chrome stringifies null to "".

@mbrodesser-Igalia mbrodesser-Igalia changed the title Section 3.2. "Create a Trusted Type" doesn't need to stringify because policyValue already is a string Section 3.2. "Create a Trusted Type" should specify how a policyValue=null/undefined is stringified Mar 6, 2024
@annevk
Copy link
Member

annevk commented Mar 6, 2024

Why does IDL not take care of this?

@mbrodesser-Igalia
Copy link
Collaborator Author

Why does IDL not take care of this?

Please elaborate.

@annevk
Copy link
Member

annevk commented Mar 6, 2024

If IDL expects a string any kind of other value will have been coerced by the time you get to specification algorithms.

@mbrodesser-Igalia
Copy link
Collaborator Author

If IDL expects a string any kind of other value will have been coerced by the time you get to specification algorithms.

The callback defined at the IDL level allows to return null: https://w3c.github.io/trusted-types/dist/spec/#trusted-type-policy-options.

@mbrodesser-Igalia
Copy link
Collaborator Author

Example: https://jsfiddle.net/hbp7mzov/.

@mbrodesser-Igalia
Copy link
Collaborator Author

If IDL expects a string any kind of other value will have been coerced by the time you get to specification algorithms.

The callback defined at the IDL level allows to return null: https://w3c.github.io/trusted-types/dist/spec/#trusted-type-policy-options.

So IDL shouldn't take care of it?

@annevk
Copy link
Member

annevk commented Mar 6, 2024

Yeah, I misunderstood. It seems this comes down to "stringifying" not being defined in step 3 of https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm.

@mbrodesser-Igalia mbrodesser-Igalia added this to the v1 milestone Jun 12, 2024
@mbrodesser-Igalia mbrodesser-Igalia changed the title Section 3.2. "Create a Trusted Type" should specify how a policyValue=null/undefined is stringified Stringification of TrustedHTML with null-data needs to be specified Jun 12, 2024
@mbrodesser-Igalia
Copy link
Collaborator Author

Yes, stringification of null isn't specified.

https://www.w3.org/TR/trusted-types/#trustedhtml-stringification-behavior mentions the associated date value is returned.
Chrome's behavior is to return the empty string in that case. According to #414 (comment) that's the desired behavior.

That seems acceptable, but there might be undesirable implications.

This also affects someTrustedHTML.toString(), e.g. https://jsfiddle.net/yqphfx0r/.

@petervanderbeken: WDYT?

CC @lukewarlow

@mbrodesser-Igalia
Copy link
Collaborator Author

mbrodesser-Igalia commented Jun 12, 2024

Firefox currently returns "<empty string>", one can check above example with Firefox Nightly and the pref "dom.security.trusted_types.enabled" set to true. In debug mode, an assertion is violated.

@petervanderbeken
Copy link

Yes, stringification of null isn't specified.

https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm probably needs to handle null explicitly, by setting dataString to an empty string. That's assuming #414 (comment) is what we want. @koto should be able to confirm that.

https://www.w3.org/TR/trusted-types/#trustedhtml-stringification-behavior mentions the associated date value is returned.

Note that the associated data value is a string, so it can't contain null. I understand that the Gecko implementation currently returns an empty string, but that's just a weird by-product of having an internal "nullable" string type, which will return an empty string if it's assumed to be non-null. I think the implementation should be changed if the above spec change is done.

@mbrodesser-Igalia
Copy link
Collaborator Author

Yes, stringification of null isn't specified.

https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm probably needs to handle null explicitly, by setting dataString to an empty string. That's assuming #414 (comment) is what we want. @koto should be able to confirm that.

Given https://searchfox.org/mozilla-central/rev/a26e972e97fbec860400d80df625193f4c88d64e/testing/web-platform/tests/trusted-types/TrustedTypePolicy-createXXX.html#66, it's very likely what's desired for the spec too.

@mbrodesser-Igalia
Copy link
Collaborator Author

What's the use-case for letting createHTML return null instead of throwing?

CC @koto, @lukewarlow

@mbrodesser-Igalia
Copy link
Collaborator Author

What's the use-case for letting createHTML return null instead of throwing?

CC @koto, @lukewarlow

The report-only mode with a default policy (https://w3c.github.io/trusted-types/dist/spec/#default-policy-hdr).

@mbrodesser-Igalia
Copy link
Collaborator Author

It'd be clearer if createPolicy only accepted callbacks returning DOMString (not DOMString?) and a - to be added - createDefaultPolicy would allow callbacks returning DOMString?.

@lukewarlow
Copy link
Member

It would be clearer but unfortunately that ship sailed about 4 years ago...

@mbrodesser-Igalia
Copy link
Collaborator Author

I wonder if that ship really sailed. Currently, this is only shipped in one engine.

CC @annevk

@lukewarlow
Copy link
Member

That one engine gives you something like 75% market share though and it's a fairly core part of the API. Now it's possible that doesn't translate to much usage (ultimately we'd need usage stats from chrome). But it's ultimately not too hard to understand, implement and spec in its current form. And imo also not a particularly massive win given the effort to change.

@annevk
Copy link
Member

annevk commented Jul 1, 2024

This is closed, but stringifying in step 3 of https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm is still not defined...

@lukewarlow lukewarlow reopened this Jul 1, 2024
@lukewarlow
Copy link
Member

I'll reopen it. The spec at least says something about the null value now but yeah we need to go through and deal with bits like this and others more thorughly. I've been focusing on the larger normative bits that have been missing, but this smaller editorial work is definitely still on our TODO list.

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

Successfully merging a pull request may close this issue.

4 participants