-
Notifications
You must be signed in to change notification settings - Fork 76
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
Update handling of timer functions #481
Conversation
* |sink| set to `Window setInterval` if <var ignore>repeat</var> is true, `Window setTimeout` otherwise. | ||
* |sinkGroup| set to `'script'`. | ||
|
||
Note: This matches the logic that the extended attribute would apply. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the string context here, so no need to do any of this block.
@otherdaniel you implemented this functionality in Chromium iirc would you be able to take a quick look over this PR to sanity check it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Generally, this looks exactly right to me!
There's one editorial nitpick with the note.
The other issue is about the definition of |compilationType|
, where I might well be overlooking something. Would be good to get that resolved.
Based on @otherdaniel review I'm going to go ahead and merge this |
SHA: 39cc82e Reason: push, by lukewarlow Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #480
This effectively follows the Chromium implementation, we use the StringContext attribute to do the enforcement for setTimeout and setInterval and then update the
HostEnsureCanCompileStrings
call (which now actually calls the CSP function directly) accordingly.We then update
EnsureCSPDoesNotBlockStringCompilation
to only do TT validation for eval and Function, not timers.Preview | Diff