Skip to content
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

embed/object #5

Closed
annevk opened this issue Jan 5, 2021 · 7 comments
Closed

embed/object #5

annevk opened this issue Jan 5, 2021 · 7 comments
Labels

Comments

@annevk
Copy link
Owner

annevk commented Jan 5, 2021

It seems to me that the current way https://fetch.spec.whatwg.org/#corb is described it does not work well with the embed and object elements, as something gets blocked before those elements get to decide to instantiate a browsing context and both elements can render HTML.

I suspect this is an oversight in the formal model as bypassing the CORB check for these elements should be safe given that nested browsing contexts ought to get their own process when appropriate.

cc @anforowicz

@anforowicz
Copy link
Collaborator

Hmmm... indeed:

  • In Chromium, CORB does not apply to navigate requests (although sadly this is not checked directly against the properties of network::ResourceRequest, but in a very indirect way, by controlling the is_corb_enabled property of network::mojom::URLLoaderFactoryParams and having a different, separate URLLoaderFactory for navigation requests
  • In Chromium, CORB applies today to cors requests, but this is not necessary now, when CORS is enforced outside of the renderer process.
  • In the long-term, CORB should only apply to no-cors requests (just like CORP).

Errr... I guess I owe you a pull request to add a step to CORB that checks the request mode. I'll try to learn (relearn?) how to work with git remotes and github... :-/

@annevk
Copy link
Owner Author

annevk commented Jan 5, 2021

Most of that is already covered. We only apply the CORB check for no-cors. The problem is that the embed and object elements also make a no-cors request (before they can be navigated) and there it should probably not apply.

@anforowicz
Copy link
Collaborator

anforowicz commented Jan 5, 2021

I see. I think you are hitting a problem similar to https://crbug.com/929300. I tried to re-read the discussion in that bug, but I am not sure if I understood much from it.

  1. NetworkService in Chromium enforces that only the supervisor/browser process may trigger navigate requests. Renderer processes may ask the browser process to start a navigation, but the request (and response data) is controlled by the browser process.
  2. CORB in Chromium applies to all requests from a renderer hosting http content (including <object src="https://other.origin.com/x.pdf">) in a https://example.com/page.html).
  3. CORB in Chromium does not apply to requests from extensions. PDFs in Chromium are renderered from an extension:
    example.com/page.html --embeds--> artificially conjured and generally invisible to web content chrome-extension:// iframe --embeds--> PPAPI-based PDF plugin
  4. Somehow (magic to me... :-( ) Chromium supports cross-origin PDFs in <object>, despite the restrictions above. I think the answer lies within https://crbug.com/929300, but I am not sure how exactly it all works together...

@anforowicz
Copy link
Collaborator

anforowicz commented Jan 5, 2021

I suspect this is an oversight in the formal model as bypassing the CORB check for these elements should be safe given that nested browsing contexts ought to get their own process when appropriate.

It is indeed weird that some <object> elements (e.g. ones hosting Flash) can use no-cors fetches, but some (e.g. ones hosting PDF) use navigate. Maybe with the deprecation of PPAPI, we could eventually say that all '' elements trigger a navigate request?

PS. I guess in #5 (comment) I got confused and started answering a wrong question (about how the implementation in Chromium works, rather than how the spec should model CORB vs no-cors vs navigate vs <object> - I assume that this issue is more about the spec world, than the implementation world)

@annevk
Copy link
Owner Author

annevk commented Jan 6, 2021

Yeah, this is about the theoretical model. I do want to investigate if we can move to using navigate exclusively for embed and object elements, but I suspect that is not possible due to images.

The way embed/object roughly (there are a ton of issues with the current specification) work theoretically is that initially they do a no-cors request and then inspect the response and then either render the response (if it's an image) or create a nested browsing context and navigate to the response (e.g., if it's text/html). In implementations that logic will be split across process boundaries.

What Chrome seems to do for PDFs matches what it would do for text/html and I think matches what Firefox does as well. Create a nested browsing context and navigate it to the PDF. I think that means that only images can end up in the problematic process at which point it's fine to bypass CORB/ORB as long as the set of images that can up in the problematic process is not a larger set than CORB/ORB allows.

@annevk
Copy link
Owner Author

annevk commented May 17, 2022

I think the fix here would be skipping request's whose destination is "embed" or "object" and trusting that they are handled appropriately.

I'm not a 100% sure if all implementations still classify these initial embed and object element requests as "no-cors" however. E.g., when it comes to Sec-Fetch-Site and such. Part of the problem is that those elements have not been maintained.

cc @farre

@annevk
Copy link
Owner Author

annevk commented May 31, 2022

I just checked the HTML Standard and it already uses mode "navigate" for these requests (despite them not really being navigations). That will cause them to bypass this filter so we don't have to deal with them then.

@annevk annevk closed this as completed May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants