Opened 6 weeks ago
Closed 5 weeks ago
#64803 closed defect (bug) (fixed)
Media: Skip cross-origin isolation for third-party page builders
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 7.0 | Priority: | normal |
| Severity: | normal | Version: | trunk |
| Component: | Media | Keywords: | has-patch commit has-unit-tests |
| Focuses: | Cc: |
Description
In 7.0, WordPress is adding headers to enable cross-origin isolation for the block editor, unlocking SharedArrayBuffer for client-side media processing.
DIP or COEP/COOP headers isolate the document into its own agent cluster, which blocks same-origin iframe access. Third-party page builders (e.g. Elementor, Beaver Builder, Divi) that override the block editor typically rely on same-origin iframes and would break under DIP.
These page builders register custom action query parameters (e.g. action=elementor) to load their editor instead of the block editor. This ticket adds a check to client side media when a non-edit action parameter is detected on the post editing screen.
Open question: Some editors like Web Stories use action=edit but replace the block editor entirely. The current check would still apply DIP in that case. We may need a filter or an alternative detection mechanism to handle this edge case.
PR: PR #11170
See also: discussion on PR #11098
Change History (28)
This ticket was mentioned in PR #11170 on WordPress/wordpress-develop by @adamsilverstein.
6 weeks ago
#1
- Keywords has-patch added
#3
@
6 weeks ago
Tested with WordPress trunk and Elementor.
Before applying the patch:
Cross-Origin-Embedder-Policy and Cross-Origin-Opener-Policy headers were present when opening the editor with action=elementor.
https://postimg.cc/HctzBfPB
After applying the patch:
These headers are no longer sent for post.php?post=68&action=elementor.
https://postimg.cc/Jt45RYF0
Patch works as expected.
#4
follow-up:
↓ 5
@
6 weeks ago
I tried Beaver Builder and it seems editing a post takes me to the frontend and I'm not in the admin. For example, I'm taken to /?page_id=1939&fl_builder&fl_builder_ui.
#5
in reply to:
↑ 4
@
5 weeks ago
Replying to westonruter:
I tried Beaver Builder and it seems editing a post takes me to the frontend and I'm not in the admin. For example, I'm taken to
/?page_id=1939&fl_builder&fl_builder_ui.
Did you notice any issues using it though? Can you test uploading an image?
This ticket was mentioned in Slack in #meta by adamsilverstein. View the logs.
5 weeks ago
@adamsilverstein commented on PR #11170:
5 weeks ago
#7
In my testing with Elementor _without_ this PR things seemed to be fixed, perhaps because we switched to using Document Isolation Policy. I will confirm this with their team, then I think we can close this PR as "working as expected".
@louiswol94 commented on PR #11170:
5 weeks ago
#8
In my testing with Elementor _without_ this PR things seemed to be fixed, perhaps because we switched to using Document Isolation Policy. I will confirm this with their team, then I think we can close this PR as "working as expected".
@adamsilverstein Can you test by removing this filter? https://github.com/elementor/elementor/pull/34900/changes
This fix was added on our side in 3.35 (latest version). But all our users that have < 3.35 would be affected.
@adamsilverstein commented on PR #11170:
5 weeks ago
#9
In my testing with Elementor _without_ this PR things seemed to be fixed, perhaps because we switched to using Document Isolation Policy. I will confirm this with their team, then I think we can close this PR as "working as expected".
@adamsilverstein[[Image(chrome-extension://hgomfjikakokcbkjlfgodhklifiplmpg/images/wp-logo.png)]] Can you test by removing this filter? https://github.com/elementor/elementor/pull/34900/changes
This fix was added on our side in 3.35 (latest version). But all our users that have < 3.35 would be affected.
I was actually testing on an older version before upgrading and it seemed to work fine, but I will also try disabling the filter now that I have upgraded Elementor.
@westonruter commented on PR #11170:
5 weeks ago
#10
@louiswol94 I don't get any error with that line commented out.
However, I'm seeing something else unexpected here:
I don't see that wp_set_up_cross_origin_isolation() is getting called at all when accessing the Elementor editor. However, if I change it to:
add_action( 'load-post.php', fn() => wp_set_up_cross_origin_isolation() );
Then it _does_ run and I get an error in the console:
Uncaught SecurityError: Failed to read a named property 'elementorFrontend' from 'Window': Blocked a frame with origin "http://localhost:8000" from accessing a cross-origin frame.
And I see the Elementor loading spinner indefinitely:
Why wouldn't add_action( 'load-post.php', 'wp_set_up_cross_origin_isolation' ) work in Elementor?
@adamsilverstein commented on PR #11170:
5 weeks ago
#11
@louiswol94 - I did verify that Elementor loads without error even with that filter removed, so I'm not sure this PR is needed. It would still be good to address Weston's question above.
@westonruter commented on PR #11170:
5 weeks ago
#12
I can confirm that the patch fixes the issue in Elementor. However, I can only reproduce the issue if I do this change as well:
-
src/wp-includes/default-filters.php
a b add_filter( 'plupload_default_settings', 'wp_show_heic_upload_error' ); 679 679 // Client-side media processing. 680 680 add_action( 'admin_init', 'wp_set_client_side_media_processing_flag' ); 681 681 // Cross-origin isolation for client-side media processing. 682 add_action( 'load-post.php', 'wp_set_up_cross_origin_isolation');682 add_action( 'load-post.php', fn() => wp_set_up_cross_origin_isolation() ); 683 683 add_action( 'load-post-new.php', 'wp_set_up_cross_origin_isolation' ); 684 684 add_action( 'load-site-editor.php', 'wp_set_up_cross_origin_isolation' ); 685 685 add_action( 'load-widgets.php', 'wp_set_up_cross_origin_isolation' );
(Aside from also commenting out the change introduced in https://github.com/elementor/elementor/pull/34900.)
@louiswol94 commented on PR #11170:
5 weeks ago
#13
Hey guys...
Quick question:
Does DIP break same-origin contentWindow access?
According to this https://developer.chrome.com/blog/document-isolation-policy
Iframes isolated with Document Isolation Policy don't have synchronous DOM access to same-origin iframes that are not isolated.
I get this error on my console (on http://elementorlocal.local/):
The Document-Isolation-Policy header has been ignored because the URL's origin was untrustworthy. Please deliver the response using the HTTPS protocol. You can also use the 'localhost' origin instead. See https://www.w3.org/TR/powerful-features/#potentially-trustworthy-origin.
Which makes me think that if I was on a trusted origin, DIP would be enforced and I might get a security error.
Then also, is it possible you cannot reproduce due to the Gutenburg plugin doing:
remove_action( 'load-post.php', 'wp_set_up_cross_origin_isolation' ); remove_action( 'load-post-new.php', 'wp_set_up_cross_origin_isolation' );
Thanks for bearing with me here guys :D I am just trying my best to understand the full picture and make sure we reach the right conclusion.
@louiswol94 commented on PR #11170:
5 weeks ago
#14
Something else that might be important evidence...
I ran our test suite against this PR: https://github.com/elementor/elementor/pull/35068/changes
In this PR I removed the filter and made sure the WP nightly version is used.
From the logs I can see PlayWright used Chromium 139.0.7258.5 which is above 137.
And all the tests passed.
@louiswol94 commented on PR #11170:
5 weeks ago
#15
I will monitor this thread, but after my latest findings if you guys still do not think this PR is necessary, then I am in agreement not to merge it.
I really want to thank you guys for the work you are doing!
Kind regards
Louis
@adamsilverstein commented on PR #11170:
5 weeks ago
#16
I will monitor this thread, but after my latest findings if you guys still do not think this PR is necessary, then I am in agreement not to merge it.
@louiswol94 thanks for the continued testing here. One way to test Beta 3 on an actual site is to install the Beta Tester Plugin - https://wordpress.org/plugins/wordpress-beta-tester/. Install it and set it to download nightlies, then update. Installing the Gutenberg plugin is a good proxy, but doesn't represent the exact code WordPress will release with. If you have some sites running older Elementor versions you could test on that would provide empirical validation.
Hi, sorry to jump in like this - but if this PR doesn't get merged, millions of Elementor sites will become unusable.
@miriamelementor - don't worry, we still have time to get this _very small change_ into the release, but I want to make sure its really needed. We made an architectural change recently (switching the header approach) that may have already fixed the issue. The fix in this PR turns off the feature for Elementor, meaning you won't be able to use it or any of the new capabilities it provides, so i want to make sure we are turning it off for a reason!
@louiswol94 commented on PR #11170:
5 weeks ago
#17
Thanks @adamsilverstein - I forgot about the Beta Tester Plugin. I am glad you prompted me to test further.
I installed:
- WordPress
7.0-beta3-61869. - Elementor 3.34.4
On a Live website. And get the following:
editor.min.js?ver=3.34.4:2 Uncaught SecurityError: Failed to read a named property 'elementorFrontend' from 'Window': Blocked a frame with origin "https://naharwuz.elementor.cloud" from accessing a cross-origin frame.
at editor.min.js?ver=3.34.4:2:1259679
at Editor.onPreviewLoaded (editor.min.js?ver=3.34.4:2:1260247)
at HTMLIFrameElement.dispatch (load-scripts.php?c=1&load%5Bchunk_0%5D=jquery-core,jquery-migrate,jquery-ui-core,jquery-ui-mouse,underscore,backbone,wp-api-request,wp-hooks&ver=7.0-beta3-61869:2:40035)
at v.handle (load-scripts.php?c=1&load%5Bchunk_0%5D=jquery-core,jquery-migrate,jquery-ui-core,jquery-ui-mouse,underscore,backbone,wp-api-request,wp-hooks&ver=7.0-beta3-61869:2:38006)
@louiswol94 commented on PR #11170:
5 weeks ago
#18
@westonruter commented on PR #11170:
5 weeks ago
#19
@louiswol94:
Then also, is it possible you cannot reproduce due to the Gutenburg plugin doing:
remove_action( 'load-post.php', 'wp_set_up_cross_origin_isolation' ); remove_action( 'load-post-new.php', 'wp_set_up_cross_origin_isolation' );
Right you are! I deactivated Gutenberg and then I was able to get the error, and this PR fixes the issue.
@westonruter commented on PR #11170:
5 weeks ago
#20
I get this error on my console (on http://elementorlocal.local/):
The Document-Isolation-Policy header has been ignored because the URL's origin was untrustworthy. Please deliver the response using the HTTPS protocol. You can also use the 'localhost' origin instead. See https://www.w3.org/TR/powerful-features/#potentially-trustworthy-origin.
Which makes me think that if I was on a trusted origin, DIP would be enforced and I might get a security error. (This also aligns with the code comment in this PR).
@adamsilverstein This relates to what we were discussing with whether client-side media handling should be enabled on localhost. In addition to the error @louiswol94 got above, I also see in the console:
Client-side media processing unavailable: SharedArrayBuffer is not available. This may be due to missing cross-origin isolation headers.. Using server-side processing.
This is because SharedArrayBuffer is never available in non-secure or localhost context.
I think wp_set_up_cross_origin_isolation() should be updated to short-circuit if the ( ! is_ssl() && $_SERVER['HTTP_HOST'] !== 'localhost' ). Or I guess, rather, that this should be the default value of wp_is_client_side_media_processing_enabled():
-
src/wp-includes/media.php
diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index 8e4f67a349..79e2dc5937 100644
a b function wp_get_image_editor_output_format( $filename, $mime_type ) { 6371 6371 * @return bool Whether client-side media processing is enabled. 6372 6372 */ 6373 6373 function wp_is_client_side_media_processing_enabled(): bool { 6374 $enabled = ( is_ssl() || 'localhost' === $_SERVER['HTTP_HOST'] ); 6375 6374 6376 /** 6375 6377 * Filters whether client-side media processing is enabled. 6376 6378 * 6377 6379 * @since 7.0.0 6378 6380 * 6379 * @param bool $enabled Whether client-side media processing is enabled. Default true .6381 * @param bool $enabled Whether client-side media processing is enabled. Default true if SSL or localhost. 6380 6382 */ 6381 return (bool) apply_filters( 'wp_client_side_media_processing_enabled', true);6383 return (bool) apply_filters( 'wp_client_side_media_processing_enabled', $enabled ); 6382 6384 } 6383 6385 6384 6386 /**
This should fix the issue that Louis observed.
@louiswol94 I presume you were testing on a non-HTTPS site first?
@louiswol94 commented on PR #11170:
5 weeks ago
#21
@louiswol94 I presume you were testing on a non-HTTPS site first?
@westonruter Correct - http://elementorlocal.local/
@adamsilverstein commented on PR #11170:
5 weeks ago
#22
@westonruter added the localhost change in https://github.com/WordPress/wordpress-develop/pull/11170/commits/a65f438a5ae68bbd3402a21f97f35c08adbc546b
@adamsilverstein commented on PR #11170:
5 weeks ago
#23
I added a couple of tests for this in https://github.com/WordPress/wordpress-develop/pull/11170/commits/85d6e62e24b40b10a6da49f98be932e0ba563913 and it is ready for another review @westonruter @mukeshpanchal27.
I'd like to get this committed asap.
@adamsilverstein commented on PR #11170:
5 weeks ago
#24
@adamsilverstein commented on PR #11170:
5 weeks ago
#25
Checking test failures...
@adamsilverstein commented on PR #11170:
5 weeks ago
#26
Checking test failures...
Ah, since the CI runs on a non-SSL, non-localhost environment, wp_is_client_side_media_processing_enabled() returns false which is breaking some tests. Fixing.
## Summary
actionquery parameteraction=editbut replaces the block editor entirely)See discussion: https://github.com/WordPress/wordpress-develop/pull/11098#discussion_r2064009989
## Test plan
actionparameter (e.g.action=elementor) does NOT get DIP headersaction=editis used but the block editor is replaced (e.g. Web Stories)