html
html copied to clipboard
whatwg
Editorial: Update HostEnsureCanCompileStrings definition
Update the HostEnsureCanCompileStrings definition to match dynamic code brand checks stage 3 proposal.
Also update the call to EnsureCSPDoesNotBlockStringCompilation to pass these new arguments through.
Also update the timer initialization steps to call EnsureCSPDoesNotBlockStringCompilation directly, and include the new parameters.
Also define HostGetCodeForEval implementation
See https://github.com/w3c/webappsec-csp/pull/650 for corresponding CSP PR
Also see #10202 for context
(See WHATWG Working Mode: Changes for more details.)
/index.html ( diff ) /infrastructure.html ( diff ) /references.html ( diff ) /timers-and-user-prompts.html ( diff ) /webappapis.html ( diff )
Is the CSP integration correct though? It doesn't make sense we'd invoke something there that just drops an argument on the floor.
I think the CSP integration is correct for current browsers but there are ambitions of adding more checks using more arguments in the future. As such, ECMAScript refactored itself.
I guess the alternative is to have HTML drop the arguments on the floor for now, and only upgrade to passing them along when CSP starts using it.
Okay, if that's the story I think this PR is fine and the CSP PR needs a note at the end of the algorithm describing that the argument is intentionally ignored and might be used in the future.
The CSP PR expects both bodyString and source but we only pass one of them here. I guess that's a bug?
Yeah that's tracked at https://github.com/tc39/ecma262/issues/938, it's a pre-existing issue. I guess CSP could either:
- reconstruct source from the other parameters; or
- wait and see what happens with https://github.com/tc39/ecma262/pull/3294, which also passes source as a parameter
It seems at that point we'd have to update HTML again as well? I guess I'd rather we figure out a coherent story first.
Yeah so a second change will be needed if https://github.com/tc39/ecma262/pull/3294 gets accepted. Though if it's not CSP would have to reconstruct the string itself (if it even has all the information it needs to do that). Happy to wait on the HTML end till we have the follow up change accepted if you'd prefer. Thought I'd do it now as it seemed worth while updating HTML to match current ecmascript either way.
Well the status quo is broken. But if merge this PR and the CSP as-is, it's still broken. So I'm not sure what that's buying us.
Idk if it's neccessary given it will be addressed soon (all going well) but this currently has an extra parameter than the actual proposal doc and PR associated with it, so should I add an issue note?
We were told to remove the codeString param and recreate it in web land, but the reasoning was flawed and so we're going to re-request passing that value through at the next TC39 plenary. It deosn't seem useful to hold off on updating the specs till then (June time) espeically as this would delay upstreaming core parts of the trusted types spec to HTML and CSP.
From what I understand of @annevk's position, he would prefer we not change this more than once, so I think holding off is going to be necessary.
@domenic Part of the consensus from the last TC39 meeting was that this change should be a proposal (already at stage 3) rather than just a PR to the spec, so this PR can already link to the final version of the host hook from the proposal rather than waiting for it to be merged in ECMA-262 (also because that will require two shipping implementations).
If those participating in TC39 think acceptance of https://github.com/tc39/ecma262/pull/3294 is a given I guess I'm willing to trust that, but it seems weird that it's still a draft. I'm also not entirely sure what the ~ syntax means.
I do think we should have a coherent story between that TC39 PR, the CSP PR, and this HTML PR.
If those participating in TC39 think acceptance of https://github.com/tc39/ecma262/pull/3294 is a given I guess I'm willing to trust that, but it seems weird that it's still a draft. I'm also not entirely sure what the ~ syntax means.
So the PR is more so in preperation for it moving to stage 4 (once we have 2 shipping implementations). The proposal document https://tc39.es/proposal-dynamic-code-brand-checks/ is the live proposal, so I wouldn't worry too much that it's in draft.
The ~ syntax is for an enum.
I do think we should have a coherent story between that TC39 PR, the CSP PR, and this HTML PR.
I'll have a look at whether we can update that TC39 PR before the next meeting, so that CSP and HTML match it. Or at the very least whether we can update that proposal document.
@domenic and @annevk I've updated the tc39 PR and have https://github.com/tc39/proposal-dynamic-code-brand-checks/pull/17 which just needs merging for the proposal document. Then we have alignment between JS PR, HTML PR, and CSP PR.
This LGTM. @annevk do you want to take a final look?
We'll want to ensure https://github.com/w3c/webappsec-csp/pull/650 is merged around the same time. I'm not sure who has permissions to do that. I guess we should wait for at least PR approval before merging this.
We'll want to ensure https://github.com/w3c/webappsec-csp/pull/650 is merged around the same time. I'm not sure who has permissions to do that. I guess we should wait for at least PR approval before merging this.
As for permissions, I can merge the CSP PR (the repo doesn't even seem to require an approval which seems like an oversight in the configuration?), just need an approval on it.
@annevk I'm assuming you have permission to do that?
Would be good to try and get this across the line.
I'll rebase this HTML PR now.