Add an |includeReportOnly| boolean argument to Does sink type require trusted types? #518
Conversation
| Given a [=realm/global object=] (|global|), a string (|sinkGroup|) this algorithm | ||
| returns `true` if the [=injection sink=] requires a [=Trusted Type=], and | ||
| `false` otherwise. | ||
| Given a [=realm/global object=] (|global|), a string (|sinkGroup|) and a boolean (|includeReportOnly|) this algorithm |
There was a problem hiding this comment.
nit: while you are here, maybe add a comma before the "this algorithm"
| 1. For each |policy| in |global|'s <a>CSP list</a>: | ||
| 1. If |policy|'s <a>directive set</a> does not contain a <a>directive</a> | ||
| whose [=directive/name=] is `"require-trusted-types-for"`, skip to the next |policy|. | ||
| 1. Let |directive| be the |policy|'s |directive set|'s [=directive=] whose name | ||
| is `"require-trusted-types-for"` | ||
| 1. If |directive|'s [=directive/value=] does not contain a <a>trusted-types-sink-group</a> which is a match | ||
| for |sinkGroup|, skip to the next |policy|. | ||
| 1. Set |result| to `true`. | ||
| 1. Let |enforced| be true if |policy|'s [=policy/disposition=] is `"enforce"`, and false otherwise. | ||
| 1. Set |result| to true if |enforced| is true, and set to |includeReportOnly| otherwise. |
There was a problem hiding this comment.
If I read this correctly, the name includeReportOnly seems wrong, since the "enforce" policies are always forcing the result to true ; while the "report" policies only forces the result to true if includeReportOnly is false.
Also the "set to |includeReportOnly|" wording sounds a bit ambiguous to me. At least as a non-native speaker I initally misunderstood this meant setting the result variable to the value of includeReportOnly, which does not make sense.
So I guess what you want is rename the parameter includeEnforceOnly and rewrite the line to something like:
- If
includeEnforceOnlyis false or |policy|'s [=policy/disposition=] is"enforce"then set |result| totrue.
The boolean values of the caller would still need to be changed to includeEnforceOnly=false if we want to keep including all the policies.
Finally, even before your change it seems the result can only be set to true and never goes back to false ; so probably you can either just exit the "for each" loop immediately when it's set to true. Alternatively, remove the "result" variable and use "return true" or "return false" statements.
| @@ -1018,7 +1018,7 @@ Given a {{TrustedType}} type (|expectedType|), a [=realm/global object=] (|globa | |||
| 1. If |input| has type |expectedType|, return stringified | |||
| |input| and abort these steps. | |||
| 1. Let |requireTrustedTypes| be the result of executing [$Does sink type require trusted types?$] algorithm, | |||
| passing |global|, and |sinkGroup|. | |||
| passing |global|, |sinkGroup|, and true. | |||
There was a problem hiding this comment.
So with includeReportOnly=true means we change current behavior and only include policies with "report" disposition. That does not seem to be what you want?
This is needed by w3c/webappsec-csp#665
Preview | Diff