Handle null policyValue in Create a Trusted Type #527
Conversation
| @@ -981,6 +981,7 @@ a string |value| and a list |arguments|, execute the following steps: | |||
| 1. Let |policyValue| be the result of executing [$Get Trusted Type policy value$] with the same arguments as this algorithm and additionally true as |throwIfMissing|. | |||
| 1. If the algorithm threw an error, rethrow the error and abort the following steps. | |||
| 1. Let |dataString| be the result of stringifying |policyValue|. | |||
There was a problem hiding this comment.
Not sure if stringifying is even defined to mean a specific thing, this spec in general probably needs checking over for this sort of thing but leave that for #472
SHA: cf78f7e Reason: push, by mbrodesser-Igalia Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| @@ -981,6 +981,7 @@ a string |value| and a list |arguments|, execute the following steps: | |||
| 1. Let |policyValue| be the result of executing [$Get Trusted Type policy value$] with the same arguments as this algorithm and additionally true as |throwIfMissing|. | |||
| 1. If the algorithm threw an error, rethrow the error and abort the following steps. | |||
| 1. Let |dataString| be the result of stringifying |policyValue|. | |||
| 1. If |policyValue| is null or undefined, set |dataString| to the empty string. | |||
There was a problem hiding this comment.
I don't understand how this solves the issue? Stringifying policyValue would still happen with policyValue potentially being null?
Seems to me like this should be
1. If |policyValue| is null or undefined, set |dataString| to the empty string. Otherwise, let |dataString| be the result of stringifying |policyValue|.
There was a problem hiding this comment.
It stringifies it which would set dataString to "null" (presumably) and then dataString is set to an emptyString in the case the original value was null (or undefined). Just feels cleaner to read imo.
There was a problem hiding this comment.
Looking more into it, I find it quite confusing. I'm going to assume that Get Trusted Type policy value actually calls https://webidl.spec.whatwg.org/#invoke-a-callback-function, since it's technically invoking a WebIDL callback, not an ECMAScript function. After invoking the callback we have a nullable DOMString WebIDL value. Stringifying a value of that type is not specified anywhere afaik? Having undefined mentioned also doesn't make sense for a value of that type.
Isn't it enough to then just replace these 2 steps with:
1. If |policyValue| is null, set |policyValue| to the empty string.
and replace dataString with policyValue in the following step?
There was a problem hiding this comment.
Yeah we need to look more into the spec as a whole it's probably missing correct infra syntax in many places. This change was largely just to make it clear what to do in that specific case.
Cc @koto would you have any spare capacity to spend some time making the spec more correct from an editorial perspective?
There was a problem hiding this comment.
Cc @koto would you have any spare capacity to spend some time making the spec more correct from an editorial perspective?
Sorry, I don't have enough capacity to help with this for now :/
SHA: cf78f7e Reason: push, by lukewarlow Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #469
Preview | Diff