diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index 075ca04700489..cb867a4523c35 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -6736,24 +6736,38 @@ function wp_add_crossorigin_attributes( string $html ): string { 'SOURCE' => array( 'src' ), ); - while ( $processor->next_tag() ) { + /* + * Name of the bookmark on the AUDIO or VIDEO element the cursor is inside of, + * for as long as that element still needs the attribute. Closing tags are + * visited so the bookmark cannot outlive its element and be mistaken for the + * parent of a later SOURCE. + */ + $media_bookmark = null; + + while ( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) ) { $tag = $processor->get_tag(); if ( ! isset( $cross_origin_tag_attributes[ $tag ] ) ) { continue; } - $crossorigin = $processor->get_attribute( 'crossorigin' ); - if ( null !== $crossorigin ) { - continue; - } if ( 'AUDIO' === $tag || 'VIDEO' === $tag ) { - $processor->set_bookmark( 'audio-video-parent' ); + $media_bookmark = null; + + if ( ! $processor->is_tag_closer() && null === $processor->get_attribute( 'crossorigin' ) ) { + $media_bookmark = 'audio-video-parent'; + $processor->set_bookmark( $media_bookmark ); + } } - $processor->set_bookmark( 'resume' ); + if ( $processor->is_tag_closer() ) { + continue; + } - $sought = false; + $crossorigin = $processor->get_attribute( 'crossorigin' ); + if ( null !== $crossorigin ) { + continue; + } $is_cross_origin = false; @@ -6770,18 +6784,32 @@ function wp_add_crossorigin_attributes( string $html ): string { if ( $is_cross_origin ) { if ( 'SOURCE' === $tag ) { - $sought = $processor->seek( 'audio-video-parent' ); + if ( null !== $media_bookmark ) { + $processor->set_bookmark( 'resume' ); + + /* + * A seek past MAX_SEEK_OPS leaves the cursor where it is, so mark the + * parent only once the cursor has reached it. Marking a SOURCE does + * nothing for CORS and would hide the media element that was missed. + */ + if ( $processor->seek( $media_bookmark ) ) { + $processor->set_attribute( 'crossorigin', 'anonymous' ); + $processor->seek( 'resume' ); + } - if ( $sought ) { - $processor->set_attribute( 'crossorigin', 'anonymous' ); + /* + * The element is either marked or past the seek budget, which does not + * come back, so its remaining SOURCE children need nothing. + */ + $media_bookmark = null; } } else { $processor->set_attribute( 'crossorigin', 'anonymous' ); - } - if ( $sought ) { - $processor->seek( 'resume' ); - $processor->release_bookmark( 'audio-video-parent' ); + // A marked AUDIO or VIDEO needs nothing from its SOURCE children. + if ( 'AUDIO' === $tag || 'VIDEO' === $tag ) { + $media_bookmark = null; + } } } } diff --git a/tests/phpunit/tests/media/wpCrossOriginIsolation.php b/tests/phpunit/tests/media/wpCrossOriginIsolation.php index b1c40e8596780..8701ed381fa39 100644 --- a/tests/phpunit/tests/media/wpCrossOriginIsolation.php +++ b/tests/phpunit/tests/media/wpCrossOriginIsolation.php @@ -365,6 +365,7 @@ public function test_set_flag_preserves_upload_media_module_dependencies() { * Verifies that cross-origin elements get crossorigin="anonymous" added. * * @ticket 64766 + * @ticket 65930 * * @runInSeparateProcess * @preserveGlobalState disabled @@ -409,6 +410,9 @@ public function data_elements_that_should_get_crossorigin() { 'cross-origin source inside video' => array( '', ), + 'multiple cross-origin sources' => array( + '', + ), ); } @@ -420,6 +424,7 @@ public function data_elements_that_should_get_crossorigin() { * in credentialless mode without needing explicit CORS headers. * * @ticket 64766 + * @ticket 65930 * * @runInSeparateProcess * @preserveGlobalState disabled @@ -442,6 +447,47 @@ public function test_output_buffer_does_not_add_crossorigin( $html ) { $this->assertStringNotContainsString( 'crossorigin="anonymous"', $output ); } + /** + * A failed seek() must not leave the crossorigin attribute on the SOURCE element. + * + * WP_HTML_Tag_Processor::seek() returns false without moving the cursor once + * MAX_SEEK_OPS is exceeded. The cursor is then still on the SOURCE, so marking + * the element without checking the return value adds the attribute where it does + * nothing for CORS and leaves the parent media element unmarked. + * + * @ticket 65930 + * + * @covers ::wp_add_crossorigin_attributes + */ + public function test_source_is_not_marked_when_seeking_the_parent_fails() { + $this->setExpectedIncorrectUsage( 'WP_HTML_Tag_Processor::seek' ); + + /* + * Marking a media element costs two seeks, one back to the parent and one + * forward to resume, so one more than half the budget outlasts it. + */ + $media_elements = intdiv( WP_HTML_Tag_Processor::MAX_SEEK_OPS, 2 ) + 1; + + $output = wp_add_crossorigin_attributes( + str_repeat( + '', + $media_elements + ) + ); + + $this->assertSame( + 0, + preg_match_all( '/]*\bcrossorigin\b/i', $output ), + 'A SOURCE element must never receive the crossorigin attribute.' + ); + + $this->assertSame( + $media_elements - 1, + substr_count( $output, 'crossorigin="anonymous"' ), + 'Every media element reachable within the seek budget should have been marked.' + ); + } + /** * Data provider for elements that should not receive crossorigin="anonymous". * @@ -461,6 +507,12 @@ public function data_elements_that_should_not_get_crossorigin() { 'relative URL script' => array( '', ), + 'source outside a media element' => array( + '', + ), + 'source in a video with crossorigin' => array( + '', + ), ); }