Bug 254065

Summary: Cross-Origin-Embedder-Policy incorrectly blocks iframe on cache hit
Product: WebKit Reporter: Sam Verschueren <sam.verschueren>
Component: Page LoadingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, cdumez, roberto.vidal, sam.verschueren, twisniewski, webkit-bug-importer
Priority: P2 Keywords: BrowserCompat, InRadar
Version: Safari 16   
Hardware: All   
OS: All   
URL: https://github.com/SamVerschueren/webkit-coep-disk-cache
Attachments:
Description Flags
Screenshot showing the COEP error for disk cached responses none

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?