Skip to content
This repository was archived by the owner on Mar 14, 2024. It is now read-only.

Add worker script explanation to COOP-COEP article#8206

Merged
alexandrascript merged 4 commits intomainfrom
coop-coep-update10
Jun 23, 2022
Merged

Add worker script explanation to COOP-COEP article#8206
alexandrascript merged 4 commits intomainfrom
coop-coep-update10

Conversation

@agektmr
Copy link
Member

@agektmr agektmr commented Jun 20, 2022

This pull request updates https://web.dev/cross-origin-isolation-guide/ and https://web.dev/coop-coep/ to better explain how to opt-in to cross-origin isolation when worker scripts are used.
cc: @ArthurSonzogni

@agektmr agektmr requested a review from alexandrascript June 20, 2022 12:07
@chrome-devrel-review-bot
Copy link
Collaborator

Hello! This is an automated review by our custom reviewbot. It updates automatically when code or GitHub comments in this pull request are created or updated.

Requested changes

If there are any common problems with the content files you created or modified, they will be listed here.

src/site/content/en/blog/coop-coep/index.md

  • This file passed all of our automated Markdown audits.

src/site/content/en/secure/cross-origin-isolation-guide/index.md

  • This file passed all of our automated Markdown audits.

@netlify
Copy link

netlify bot commented Jun 20, 2022

Deploy Preview for web-dev-staging ready!

Name Link
🔨 Latest commit b0c3d8d
🔍 Latest deploy log https://app.netlify.com/sites/web-dev-staging/deploys/62b46ecfc57bc00008c80ccc
😎 Deploy Preview https://deploy-preview-8206--web-dev-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@ArthurSonzogni ArthurSonzogni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
Here are some comments below:

@agektmr
Copy link
Member Author

agektmr commented Jun 23, 2022

@ArthurSonzogni Please approve if the current changes look good

Copy link
Contributor

@alexandrascript alexandrascript left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % small updates

@alexandrascript alexandrascript added the content update for issues that do not require new content (only for updates to existing content) label Jun 23, 2022
@alexandrascript alexandrascript merged commit 658f5d3 into main Jun 23, 2022
@alexandrascript alexandrascript deleted the coop-coep-update10 branch June 23, 2022 13:50
Copy link

@ArthurSonzogni ArthurSonzogni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM % removing the sentence about CORP and workers.

* For iframes, use CORP and COEP headers as follows:
`Cross-Origin-Resource-Policy: same-origin` (or `same-site`, `cross-origin`
depending on the context) and `Cross-Origin-Embedder-Policy: require-corp`.
* For iframes and worker scripts, set the `Cross-Origin-Resource-Policy:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting CORP is needed only for cross-origin iframe. Not really useful for workers. They are always same-origin.
So, I would remove the "and worker scripts".

COEP on the other site is needed or useful for every environments created by the document. You moved the section later. This is discussed separately, which works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can only spawn a same-origin worker at the first level, but can load a cross-origin script via that worker if you use importScripts. In that case, you have to use CORP: cross-origin header. You can see it working at https://cross-origin-isolation.glitch.me/?coep=require-corp&

Copy link
Member Author

@agektmr agektmr Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this contradicts 🤔
https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Using_web_workers#spawning_subworkers

Workers may spawn more workers if they wish. So-called sub-workers must be hosted within the same origin as the parent page.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood "worker script" as the "worker's main response".

This is right worker can then use fetch/importScript to load any resources and those resources are subject to CORP checks. Yes, you can nested DedicatedWorker.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, how about phrasing the line like this:

For iframes and cross-origin worker scripts loaded via `importScripts`, set the `Cross-Origin-Resource-Policy:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels weird, because it would mean this applies exclusively to those two resources, which is wrong; it applies to every resources.
Also, I feel weird putting on the same level the main resources (the iframe's response) and a subresource (an external script loaded from the worker).

Maybe you can discuss about all subresources, indepedently of the context (window or worker), and give some examples of subresources?

  • From a COEP:require-corp document or a COEP:require-corp worker, cross-origin subresources loaded without CORS must set Cross-Origin-Resource-Policy: cross-origin header to opt-in being embedded. For instance, this applies to: <script>, importScript, <link>, <video>, <iframe>, etc...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea. I'll create a pull request so we can continue the discussion there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

content update for issues that do not require new content (only for updates to existing content)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants