Add reporting and report-only mode. #529
Conversation
This is a fairly large change, which adds proper support for reporting and a report-only mode to Permissions Policy. Reporting can be configured for individual features, with a new "report-to" parameter on the header declarations, or can be configured for all features, using a new "*" pseudo-feature. The Permissions-Policy-Report-Only header allows a parallel policy to be constructed, which will gernerate warning reports (with a "Report" disposition) but which will not cause use of the feature to be blocked.
|
FYI, after talking with @arichiv, I've removed the special handling for the |
| 9. Otherwise, for each <a>permissions-source-expression</a> |item| in |allowlist|'s <a>expressions</a>: | ||
| append the <a lt="serialization of an origin">serialization</a> of it to | ||
| |result|. | ||
| 9. Otherwise, for each <a>permissions-source-expression</a> |item| in |
There was a problem hiding this comment.
nit: could be good to link to [=list/iterate|for each=] (I think that should work)
There was a problem hiding this comment.
I'll update all of those, and do a general cleanup pass, in another CL -- there are a lot of for-eaches in here.
|
|
||
| 1. Let |header name| be "<code>Permissions-Policy-Report-Only</code>" if | ||
| |report-only| is True, or "<code>Permissions-Policy</code>" otherwise. |
There was a problem hiding this comment.
Lowercase T for "true". Same goes for "False" in other instances below. See https://infra.spec.whatwg.org/#booleans. If the spec does this elsewhere, we can maybe take care of this in a separate PR (or not at all, if you prefer, it isn't a big deal really, just a nit).
There was a problem hiding this comment.
Will address in a separate cleanup as well. Thanks!
index.bs
Outdated
| <section> | ||
| ## <dfn export abstract-op id="report-permissions-policy-violation">Generate report for violation of permissions policy on settings</dfn> ## {#algo-report-permissions-policy-violation} | ||
|
|
||
| <div class="algorithm" data-algorithm="report-permissions-policy-violation"> | ||
| Given a [=feature=] (|feature|), an <a>environment settings object</a> | ||
| (|settings|), and an optional string (|group|), this algorithm generates a | ||
| <a>report</a> about the <a>violation</a> of the policy for |feature|. | ||
| (|settings|), a String (|disposition|), and an optional string (|group|), |
There was a problem hiding this comment.
One problem here is that we always pass in group, its just sometimes it is the actual value null. In that case, I think we should either:
- Adjust the callers to not pass it in at all if the string is null (thus taking advantage of the optionality here)
- Adjust the algorithm to accept a "string-or-null" (commonly seen in HTML Standard) and react to the null case appropriately.
There was a problem hiding this comment.
I'll make it mandatory, and react to null; in earlier iterations of reporting, there was no way to configure the group, and so it was effectively always "default" (we actually never called it from this spec, the intention at the time was to call this from other specs instead, and maybe they would choose a different endpoint group).
SHA: 3e1f10d Reason: push, by clelland Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This is a fairly large change, which adds proper support for reporting and a report-only mode to Permissions Policy. Reporting can be configured for individual features, with a new "report-to" parameter on the header declarations.
The Permissions-Policy-Report-Only header allows a parallel policy to be constructed, which will generate warning reports (with a "Report" disposition) but which will not cause use of the feature to be blocked.
Preview | Diff