Bug 254065 - Cross-Origin-Embedder-Policy incorrectly blocks iframe on cache hit
Summary: Cross-Origin-Embedder-Policy incorrectly blocks iframe on cache hit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: Safari 16
Hardware: All All
: P2 Normal
Assignee: Chris Dumez
URL: https://github.com/SamVerschueren/web...
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2023-03-17 02:47 PDT by Sam Verschueren
Modified: 2023-06-08 17:20 PDT (History)
6 users (show)

See Also:


Attachments
Screenshot showing the COEP error for disk cached responses (223.71 KB, image/png)
2023-03-17 08:16 PDT, Sam Verschueren
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Verschueren 2023-03-17 02:47:55 PDT
The issue might be related to https://bugs.webkit.org/show_bug.cgi?id=245346. The issue is present on Safari 16, Safari TP, and Epiphany.

At StackBlitz, we are now looking into bringing WebContainers to Safari (https://blog.stackblitz.com/posts/introducing-webcontainers/). We're testing on the Safari Technology Preview as it should have all blocks in place to make it work, which is super exciting!

While testing, I ran into a bug where the `COEP` header on iframes coming from disk cache is not taken into account correctly. I made a git repository to demo what's going wrong https://github.com/SamVerschueren/webkit-coep-disk-cache.

Let me write it down here as well.


# Scenario:
We have a top-level document which defines `COEP: require-corp`, and wants to embed an iframe. The iframe specifies `CORP: cross-origin`.

This works because the iframe indicates the `CORP: cross-origin` header. For the demo in my repository, it could've been `same-origin` as well because they are hosted on the same domain, but in my real-world use case these are different domains.

The iframe is also served with `Cache-Control: public, max-age=3600` to make sure the browser caches the resource.


# Issue:
When starting the demo and opening the page, the iframe is correctly loaded and visible. Refreshing the page also works. However, when quitting the browser entirely with ⌘ Q, re-opening it and opening the web page again, breaks. The iframe will not be loaded. The error in DevTools looks like this

> Refused to display '' in a frame because of Cross-Origin-Embedder-Policy.
Refreshing the page with DevTools open sometimes fixes the issue. But it seems to only fix it if the `iframe.html` resources is loaded from `Memory Cache`. In the scenario where it's loaded from `Disk Cache`, it seems that the `CORP` header from the resource is not taken into account. This is probably the reason why the behaviour is different when quitting Safari entirely, because then the resource doesn't live in memory anymore.


If there's any additional information that you might need to look into this bug, please let me know!
Comment 1 Sam Verschueren 2023-03-17 08:15:44 PDT
Currently we worked around this issue by serving the iframe resource with `Cache-Control: no-store`. This is not ideal but it works and the resource itself is also quite small in size.
Comment 2 Sam Verschueren 2023-03-17 08:16:41 PDT
Created attachment 465479 [details]
Screenshot showing the COEP error for disk cached responses
Comment 3 roberto.vidal 2023-03-18 15:14:13 PDT
This is well above my paygrade, but from what I can gather, the issue lies here: https://github.com/WebKit/WebKit/blob/729daab8b1fcb955d6e487a7b6266894695972f5/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp#L666

When `shouldInterruptNavigationForCrossOriginEmbedderPolicy` is called, the `m_response` is _not_ yet updated in the case of a cache hit, but it is instead set to a dummy value (presumably the empty URL set in https://github.com/WebKit/WebKit/blob/729daab8b1fcb955d6e487a7b6266894695972f5/Source/WebCore/loader/FrameLoader.cpp#L382 ?).
Comment 4 Chris Dumez 2023-03-18 17:48:55 PDT
Thanks for the report and initial investigation. I will try and get to this bug soon.
Comment 5 Chris Dumez 2023-03-20 07:54:09 PDT
I am able to reproduce with the provided test case. Thank you.
Comment 6 Chris Dumez 2023-03-20 08:15:51 PDT
(In reply to roberto.vidal from comment #3)
> This is well above my paygrade, but from what I can gather, the issue lies
> here:
> https://github.com/WebKit/WebKit/blob/
> 729daab8b1fcb955d6e487a7b6266894695972f5/Source/WebKit/NetworkProcess/
> NetworkResourceLoader.cpp#L666
> 
> When `shouldInterruptNavigationForCrossOriginEmbedderPolicy` is called, the
> `m_response` is _not_ yet updated in the case of a cache hit, but it is
> instead set to a dummy value (presumably the empty URL set in
> https://github.com/WebKit/WebKit/blob/
> 729daab8b1fcb955d6e487a7b6266894695972f5/Source/WebCore/loader/FrameLoader.
> cpp#L382 ?).

That's exactly what was going on :)
Comment 7 Chris Dumez 2023-03-20 09:17:01 PDT
Pull request: https://github.com/WebKit/WebKit/pull/11712
Comment 8 EWS 2023-03-21 10:22:32 PDT
Committed 261924@main (38e9c1ce273d): <https://commits.webkit.org/261924@main>

Reviewed commits have been landed. Closing PR #11712 and removing active labels.
Comment 9 Radar WebKit Bug Importer 2023-03-21 10:23:17 PDT
<rdar://problem/107002434>
Comment 10 Thomas Wisniewski [:twisniewski] 2023-06-08 17:20:19 PDT
I don't see the WPTs in this commit on wpt.fyi. Were they synced?