Media: Redirect inactive attachment pages for logged-out users.

Ensure logged out users are redirected to the media file when attachment pages are inactive. This removes the read_post capability check from the canonical redirects as anonymous users lack the permission.

This was previously committed in [57310] before being reverted in [57318]. This update includes a fix to cover instances where revealing a URL could be considered a data leak and greatly expands the unit tests to ensure that this is covered along with many other instances.

Follow-up to [56657], [56658], [56711], [57310], [57318].

Reviewed by joemcgill.
Merges [57357] to 6.4 branch.

Props peterwilsoncc, jorbin, afercia, aristath, chesio, joppuyo, jorbin, lakshmananphp, poena, sergeybiryukov, swissspidy, johnbillion, mukesh27.
Fixes #59866.
See #57913.



git-svn-id: https://develop.svn.wordpress.org/branches/6.4@57358 602fd350-edb4-49c9-b593-d223f7449a82

src/wp-includes/canonical.php

@@ -548,13 +548,23 @@ function redirect_canonical( $requested_url = null, $do_redirect = true ) {
 	$is_attachment_redirect = false;
 
 	if ( is_attachment() && ! get_option( 'wp_attachment_pages_enabled' ) ) {
-		$attachment_id = get_query_var( 'attachment_id' );
+		$attachment_id        = get_query_var( 'attachment_id' );
+		$attachment_post      = get_post( $attachment_id );
+		$attachment_parent_id = $attachment_post ? $attachment_post->post_parent : 0;
 
-		if ( current_user_can( 'read_post', $attachment_id ) ) {
-			$redirect_url = wp_get_attachment_url( $attachment_id );
-
-			$is_attachment_redirect = true;
+		$attachment_url = wp_get_attachment_url( $attachment_id );
+		if ( $attachment_url !== $redirect_url ) {
+			/*
+			* If an attachment is attached to a post, it inherits the parent post's status. Fetch the
+			* parent post to check its status later.
+			*/
+			if ( $attachment_parent_id ) {
+				$redirect_obj = get_post( $attachment_parent_id );
+			}
+			$redirect_url = $attachment_url;
 		}
+
+		$is_attachment_redirect = true;
 	}
 
 	$redirect['query'] = preg_replace( '#^\??&*?#', '', $redirect['query'] );

tests/phpunit/tests/canonical.php

@@ -14,7 +14,7 @@ class Tests_Canonical extends WP_Canonical_UnitTestCase {
 		parent::set_up();
 		wp_set_current_user( self::$author_id );
 
-		add_filter( 'pre_option_wp_attachment_pages_enabled', '__return_true' );
+		update_option( 'wp_attachment_pages_enabled', 1 );
 	}
 
 	/**
@@ -406,23 +406,83 @@ class Tests_Canonical extends WP_Canonical_UnitTestCase {
 	}
 
 	/**
+	 * Test canonical redirects for attachment pages when the option is disabled.
+	 *
 	 * @ticket 57913
+	 * @ticket 59866
+	 *
+	 * @dataProvider data_canonical_attachment_page_redirect_with_option_disabled
 	 */
-	public function test_canonical_attachment_page_redirect_with_option_disabled() {
-		add_filter( 'pre_option_wp_attachment_pages_enabled', '__return_false' );
+	public function test_canonical_attachment_page_redirect_with_option_disabled( $expected, $user = null, $parent_post_status = '' ) {
+		update_option( 'wp_attachment_pages_enabled', 0 );
+
+		if ( '' !== $parent_post_status ) {
+			$parent_post_id = self::factory()->post->create(
+				array(
+					'post_status' => $parent_post_status,
+				)
+			);
+		} else {
+			$parent_post_id = 0;
+		}
 
 		$filename = DIR_TESTDATA . '/images/test-image.jpg';
 		$contents = file_get_contents( $filename );
 		$upload   = wp_upload_bits( wp_basename( $filename ), null, $contents );
 
-		$attachment_id   = $this->_make_attachment( $upload );
+		$attachment_id   = $this->_make_attachment( $upload, $parent_post_id );
+		$attachment_url  = wp_get_attachment_url( $attachment_id );
 		$attachment_page = get_permalink( $attachment_id );
 
+		// Set as anonymous/logged out user.
+		if ( null !== $user ) {
+			wp_set_current_user( $user );
+		}
+
 		$this->go_to( $attachment_page );
 
-		$url      = redirect_canonical( $attachment_page, false );
-		$expected = wp_get_attachment_url( $attachment_id );
+		$url = redirect_canonical( $attachment_page, false );
+		if ( is_string( $expected ) ) {
+			$expected = str_replace( '%%attachment_url%%', $attachment_url, $expected );
+		}
 
 		$this->assertSame( $expected, $url );
 	}
+
+	/**
+	 * Data provider for test_canonical_attachment_page_redirect_with_option_disabled().
+	 *
+	 * @return array[]
+	 */
+	public function data_canonical_attachment_page_redirect_with_option_disabled() {
+		return array(
+			'logged out user, no parent'      => array(
+				'%%attachment_url%%',
+				0,
+			),
+			'logged in user, no parent'       => array(
+				'%%attachment_url%%',
+			),
+			'logged out user, private parent' => array(
+				null,
+				0,
+				'private',
+			),
+			'logged in user, private parent'  => array(
+				'%%attachment_url%%',
+				null,
+				'private',
+			),
+			'logged out user, public parent'  => array(
+				'%%attachment_url%%',
+				0,
+				'publish',
+			),
+			'logged in user, public parent'   => array(
+				'%%attachment_url%%',
+				null,
+				'publish',
+			),
+		);
+	}
 }