From cafba115ba3c8d3fb87565bf1c7618ad638ada8f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Apr 2026 13:07:20 +0000 Subject: [PATCH 1/5] Fix attachment page redirect for pretty permalink URLs redirect_canonical() fails to redirect attachment pages accessed via pretty permalinks (e.g. /my-image-jpg/) when wp_attachment_pages_enabled is 0. get_query_var( 'attachment_id' ) is only populated for ?attachment_id=123 URLs, not slug-based URLs. Falls back to get_queried_object_id() when the query var is empty. --- src/wp-includes/canonical.php | 3 + .../tests/canonical/attachmentRedirect.php | 65 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 tests/phpunit/tests/canonical/attachmentRedirect.php diff --git a/src/wp-includes/canonical.php b/src/wp-includes/canonical.php index 6b8c17c07d55a..c9a8977aafaa9 100644 --- a/src/wp-includes/canonical.php +++ b/src/wp-includes/canonical.php @@ -551,6 +551,9 @@ function redirect_canonical( $requested_url = null, $do_redirect = true ) { if ( is_attachment() && ! get_option( 'wp_attachment_pages_enabled' ) ) { $attachment_id = get_query_var( 'attachment_id' ); + if ( ! $attachment_id ) { + $attachment_id = get_queried_object_id(); + } $attachment_post = get_post( $attachment_id ); $attachment_parent_id = $attachment_post ? $attachment_post->post_parent : 0; $attachment_url = wp_get_attachment_url( $attachment_id ); diff --git a/tests/phpunit/tests/canonical/attachmentRedirect.php b/tests/phpunit/tests/canonical/attachmentRedirect.php new file mode 100644 index 0000000000000..cf7ee5eb69f8f --- /dev/null +++ b/tests/phpunit/tests/canonical/attachmentRedirect.php @@ -0,0 +1,65 @@ +post->create( + array( + 'post_type' => 'attachment', + 'post_title' => 'Test Image', + 'post_name' => 'test-image-jpg', + 'post_status' => 'inherit', + 'post_parent' => 0, + ) + ); + + // Set a fake attachment URL via metadata. + update_post_meta( $attachment_id, '_wp_attached_file', '2025/01/test-image.jpg' ); + + self::$attachment = get_post( $attachment_id ); + } + + /** + * Pretty permalink slug-based attachment URLs should redirect to the file URL + * when wp_attachment_pages_enabled is 0. + * + * This is a regression test: get_query_var( 'attachment_id' ) is only populated + * for ?attachment_id=123 URLs, not slug-based URLs. The fix falls back to + * get_queried_object_id(). + */ + public function test_pretty_permalink_attachment_redirects_when_pages_disabled() { + update_option( 'wp_attachment_pages_enabled', 0 ); + $this->set_permalink_structure( '/%postname%/' ); + + $expected_url = wp_get_attachment_url( self::$attachment->ID ); + + $this->assertCanonical( '/test-image-jpg/', $expected_url ); + } + + /** + * Query string ?attachment_id=ID should also redirect when pages are disabled. + */ + public function test_query_var_attachment_redirects_when_pages_disabled() { + update_option( 'wp_attachment_pages_enabled', 0 ); + $this->set_permalink_structure( '/%postname%/' ); + + $expected_url = wp_get_attachment_url( self::$attachment->ID ); + + $this->assertCanonical( '/?attachment_id=' . self::$attachment->ID, $expected_url ); + } +} From 3c98e41eb6edee9c6e74a3ad71d63c85c12e1ae0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Apr 2026 00:11:23 +0000 Subject: [PATCH 2/5] test: fix assertCanonical expected value to use path only assertCanonical() compares against the path component of the redirect URL, not the full URL. Use parse_url() to extract the path from wp_get_attachment_url(). --- tests/phpunit/tests/canonical/attachmentRedirect.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/canonical/attachmentRedirect.php b/tests/phpunit/tests/canonical/attachmentRedirect.php index cf7ee5eb69f8f..0d4ae8185ad65 100644 --- a/tests/phpunit/tests/canonical/attachmentRedirect.php +++ b/tests/phpunit/tests/canonical/attachmentRedirect.php @@ -46,9 +46,10 @@ public function test_pretty_permalink_attachment_redirects_when_pages_disabled() update_option( 'wp_attachment_pages_enabled', 0 ); $this->set_permalink_structure( '/%postname%/' ); - $expected_url = wp_get_attachment_url( self::$attachment->ID ); + // assertCanonical compares against the path component only. + $expected_path = parse_url( wp_get_attachment_url( self::$attachment->ID ), PHP_URL_PATH ); - $this->assertCanonical( '/test-image-jpg/', $expected_url ); + $this->assertCanonical( '/test-image-jpg/', $expected_path ); } /** @@ -58,8 +59,9 @@ public function test_query_var_attachment_redirects_when_pages_disabled() { update_option( 'wp_attachment_pages_enabled', 0 ); $this->set_permalink_structure( '/%postname%/' ); - $expected_url = wp_get_attachment_url( self::$attachment->ID ); + // assertCanonical compares against the path component only. + $expected_path = parse_url( wp_get_attachment_url( self::$attachment->ID ), PHP_URL_PATH ); - $this->assertCanonical( '/?attachment_id=' . self::$attachment->ID, $expected_url ); + $this->assertCanonical( '/?attachment_id=' . self::$attachment->ID, $expected_path ); } } From 37bdcdd355abaf98364632ac580311af33cbccdd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Apr 2026 00:29:58 +0000 Subject: [PATCH 3/5] test: expand attachment redirect tests for parent post visibility Cover the privacy guard at canonical.php line ~800: attachments on private posts should not redirect for anonymous users (would leak the file URL), but should redirect for authorized users. Also cover attachments on draft posts and the pages-enabled no-redirect case. --- .../tests/canonical/attachmentRedirect.php | 194 +++++++++++++++--- 1 file changed, 171 insertions(+), 23 deletions(-) diff --git a/tests/phpunit/tests/canonical/attachmentRedirect.php b/tests/phpunit/tests/canonical/attachmentRedirect.php index 0d4ae8185ad65..f75056f5b6c59 100644 --- a/tests/phpunit/tests/canonical/attachmentRedirect.php +++ b/tests/phpunit/tests/canonical/attachmentRedirect.php @@ -10,58 +10,206 @@ class Tests_Canonical_AttachmentRedirect extends WP_Canonical_UnitTestCase { /** - * Attachment post object. - * - * @var WP_Post + * Attachment post objects and related fixtures. */ - public static $attachment; + public static $unattached; + public static $public_parent; + public static $attached_to_public; + public static $private_parent; + public static $attached_to_private; + public static $draft_parent; + public static $attached_to_draft; + public static $editor_user; public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { - // Create a fake attachment (no real file upload needed). - $attachment_id = $factory->post->create( + self::$editor_user = $factory->user->create( array( 'role' => 'editor' ) ); + + // Unattached attachment. + $unattached_id = $factory->post->create( array( 'post_type' => 'attachment', - 'post_title' => 'Test Image', - 'post_name' => 'test-image-jpg', + 'post_title' => 'Unattached Image', + 'post_name' => 'unattached-image', 'post_status' => 'inherit', 'post_parent' => 0, ) ); + update_post_meta( $unattached_id, '_wp_attached_file', '2025/01/unattached-image.jpg' ); + self::$unattached = get_post( $unattached_id ); + + // Attachment on a public post. + self::$public_parent = $factory->post->create_and_get( + array( + 'post_type' => 'post', + 'post_title' => 'Public Post', + 'post_name' => 'public-post', + 'post_status' => 'publish', + ) + ); + $attached_public_id = $factory->post->create( + array( + 'post_type' => 'attachment', + 'post_title' => 'Public Attached Image', + 'post_name' => 'public-attached-image', + 'post_status' => 'inherit', + 'post_parent' => self::$public_parent->ID, + ) + ); + update_post_meta( $attached_public_id, '_wp_attached_file', '2025/01/public-attached-image.jpg' ); + self::$attached_to_public = get_post( $attached_public_id ); + + // Attachment on a private post. + self::$private_parent = $factory->post->create_and_get( + array( + 'post_type' => 'post', + 'post_title' => 'Private Post', + 'post_name' => 'private-post', + 'post_status' => 'private', + 'post_author' => self::$editor_user, + ) + ); + $attached_private_id = $factory->post->create( + array( + 'post_type' => 'attachment', + 'post_title' => 'Private Attached Image', + 'post_name' => 'private-attached-image', + 'post_status' => 'inherit', + 'post_parent' => self::$private_parent->ID, + ) + ); + update_post_meta( $attached_private_id, '_wp_attached_file', '2025/01/private-attached-image.jpg' ); + self::$attached_to_private = get_post( $attached_private_id ); - // Set a fake attachment URL via metadata. - update_post_meta( $attachment_id, '_wp_attached_file', '2025/01/test-image.jpg' ); + // Attachment on a draft post. + self::$draft_parent = $factory->post->create_and_get( + array( + 'post_type' => 'post', + 'post_title' => 'Draft Post', + 'post_name' => 'draft-post', + 'post_status' => 'draft', + 'post_author' => self::$editor_user, + ) + ); + $attached_draft_id = $factory->post->create( + array( + 'post_type' => 'attachment', + 'post_title' => 'Draft Attached Image', + 'post_name' => 'draft-attached-image', + 'post_status' => 'inherit', + 'post_parent' => self::$draft_parent->ID, + ) + ); + update_post_meta( $attached_draft_id, '_wp_attached_file', '2025/01/draft-attached-image.jpg' ); + self::$attached_to_draft = get_post( $attached_draft_id ); + } - self::$attachment = get_post( $attachment_id ); + /** + * Helper to get the expected redirect path for an attachment. + */ + private function get_expected_path( $attachment_id ) { + return parse_url( wp_get_attachment_url( $attachment_id ), PHP_URL_PATH ); } + // ------------------------------------------------------------------------- + // Unattached attachment tests. + // ------------------------------------------------------------------------- + /** * Pretty permalink slug-based attachment URLs should redirect to the file URL * when wp_attachment_pages_enabled is 0. * - * This is a regression test: get_query_var( 'attachment_id' ) is only populated - * for ?attachment_id=123 URLs, not slug-based URLs. The fix falls back to - * get_queried_object_id(). + * This is the primary regression test: get_query_var( 'attachment_id' ) is only + * populated for ?attachment_id=123 URLs, not slug-based URLs. The fix falls back + * to get_queried_object_id(). */ - public function test_pretty_permalink_attachment_redirects_when_pages_disabled() { + public function test_unattached_slug_url_redirects_when_pages_disabled() { update_option( 'wp_attachment_pages_enabled', 0 ); $this->set_permalink_structure( '/%postname%/' ); - // assertCanonical compares against the path component only. - $expected_path = parse_url( wp_get_attachment_url( self::$attachment->ID ), PHP_URL_PATH ); - - $this->assertCanonical( '/test-image-jpg/', $expected_path ); + $this->assertCanonical( '/unattached-image/', $this->get_expected_path( self::$unattached->ID ) ); } /** * Query string ?attachment_id=ID should also redirect when pages are disabled. */ - public function test_query_var_attachment_redirects_when_pages_disabled() { + public function test_unattached_query_var_url_redirects_when_pages_disabled() { update_option( 'wp_attachment_pages_enabled', 0 ); $this->set_permalink_structure( '/%postname%/' ); - // assertCanonical compares against the path component only. - $expected_path = parse_url( wp_get_attachment_url( self::$attachment->ID ), PHP_URL_PATH ); + $this->assertCanonical( '/?attachment_id=' . self::$unattached->ID, $this->get_expected_path( self::$unattached->ID ) ); + } + + // ------------------------------------------------------------------------- + // Attached to a public post. + // ------------------------------------------------------------------------- + + /** + * Attachment on a public post should redirect via slug URL. + */ + public function test_attached_to_public_post_slug_url_redirects() { + update_option( 'wp_attachment_pages_enabled', 0 ); + $this->set_permalink_structure( '/%postname%/' ); + + $this->assertCanonical( '/public-attached-image/', $this->get_expected_path( self::$attached_to_public->ID ) ); + } + + // ------------------------------------------------------------------------- + // Attached to a private post — logged out (should NOT redirect). + // ------------------------------------------------------------------------- + + /** + * Attachment on a private post should not redirect for anonymous users, + * to avoid leaking the file URL. + */ + public function test_attached_to_private_post_no_redirect_for_anonymous() { + update_option( 'wp_attachment_pages_enabled', 0 ); + $this->set_permalink_structure( '/%postname%/' ); + wp_set_current_user( 0 ); + + $this->assertCanonical( '/private-attached-image/', '/private-attached-image/' ); + } + + // ------------------------------------------------------------------------- + // Attached to a private post — authorized user (should redirect). + // ------------------------------------------------------------------------- + + /** + * Attachment on a private post should redirect for a user who can read it. + */ + public function test_attached_to_private_post_redirects_for_authorized_user() { + update_option( 'wp_attachment_pages_enabled', 0 ); + $this->set_permalink_structure( '/%postname%/' ); + wp_set_current_user( self::$editor_user ); + + $this->assertCanonical( '/private-attached-image/', $this->get_expected_path( self::$attached_to_private->ID ) ); + } + + // ------------------------------------------------------------------------- + // Attached to a draft post (should NOT redirect). + // ------------------------------------------------------------------------- + + /** + * Attachment on a draft post should not redirect for anonymous users. + */ + public function test_attached_to_draft_post_no_redirect_for_anonymous() { + update_option( 'wp_attachment_pages_enabled', 0 ); + $this->set_permalink_structure( '/%postname%/' ); + wp_set_current_user( 0 ); + + $this->assertCanonical( '/draft-attached-image/', '/draft-attached-image/' ); + } + + // ------------------------------------------------------------------------- + // Pages enabled — should NOT redirect. + // ------------------------------------------------------------------------- + + /** + * When attachment pages are enabled, slug URLs should not redirect to the file. + */ + public function test_no_redirect_when_attachment_pages_enabled() { + update_option( 'wp_attachment_pages_enabled', 1 ); + $this->set_permalink_structure( '/%postname%/' ); - $this->assertCanonical( '/?attachment_id=' . self::$attachment->ID, $expected_path ); + $this->assertCanonical( '/unattached-image/', '/unattached-image/' ); } } From 4f677531629f41045829c7f8979adca401a56ef8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Apr 2026 00:32:39 +0000 Subject: [PATCH 4/5] test: fix PHPCS equals sign alignment warnings Use create_and_get() directly into self:: properties to avoid intermediate variables that trigger MultipleStatementAlignment warnings. --- .../tests/canonical/attachmentRedirect.php | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/phpunit/tests/canonical/attachmentRedirect.php b/tests/phpunit/tests/canonical/attachmentRedirect.php index f75056f5b6c59..2ea701edd6bfb 100644 --- a/tests/phpunit/tests/canonical/attachmentRedirect.php +++ b/tests/phpunit/tests/canonical/attachmentRedirect.php @@ -25,7 +25,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { self::$editor_user = $factory->user->create( array( 'role' => 'editor' ) ); // Unattached attachment. - $unattached_id = $factory->post->create( + self::$unattached = $factory->post->create_and_get( array( 'post_type' => 'attachment', 'post_title' => 'Unattached Image', @@ -34,8 +34,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_parent' => 0, ) ); - update_post_meta( $unattached_id, '_wp_attached_file', '2025/01/unattached-image.jpg' ); - self::$unattached = get_post( $unattached_id ); + update_post_meta( self::$unattached->ID, '_wp_attached_file', '2025/01/unattached-image.jpg' ); // Attachment on a public post. self::$public_parent = $factory->post->create_and_get( @@ -46,7 +45,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_status' => 'publish', ) ); - $attached_public_id = $factory->post->create( + self::$attached_to_public = $factory->post->create_and_get( array( 'post_type' => 'attachment', 'post_title' => 'Public Attached Image', @@ -55,8 +54,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_parent' => self::$public_parent->ID, ) ); - update_post_meta( $attached_public_id, '_wp_attached_file', '2025/01/public-attached-image.jpg' ); - self::$attached_to_public = get_post( $attached_public_id ); + update_post_meta( self::$attached_to_public->ID, '_wp_attached_file', '2025/01/public-attached-image.jpg' ); // Attachment on a private post. self::$private_parent = $factory->post->create_and_get( @@ -68,7 +66,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_author' => self::$editor_user, ) ); - $attached_private_id = $factory->post->create( + self::$attached_to_private = $factory->post->create_and_get( array( 'post_type' => 'attachment', 'post_title' => 'Private Attached Image', @@ -77,8 +75,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_parent' => self::$private_parent->ID, ) ); - update_post_meta( $attached_private_id, '_wp_attached_file', '2025/01/private-attached-image.jpg' ); - self::$attached_to_private = get_post( $attached_private_id ); + update_post_meta( self::$attached_to_private->ID, '_wp_attached_file', '2025/01/private-attached-image.jpg' ); // Attachment on a draft post. self::$draft_parent = $factory->post->create_and_get( @@ -90,7 +87,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_author' => self::$editor_user, ) ); - $attached_draft_id = $factory->post->create( + self::$attached_to_draft = $factory->post->create_and_get( array( 'post_type' => 'attachment', 'post_title' => 'Draft Attached Image', @@ -99,8 +96,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_parent' => self::$draft_parent->ID, ) ); - update_post_meta( $attached_draft_id, '_wp_attached_file', '2025/01/draft-attached-image.jpg' ); - self::$attached_to_draft = get_post( $attached_draft_id ); + update_post_meta( self::$attached_to_draft->ID, '_wp_attached_file', '2025/01/draft-attached-image.jpg' ); } /** From f80ca498ab4aa247f837cdf7183fb304846866bb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Apr 2026 02:06:11 +0000 Subject: [PATCH 5/5] test: fix child attachment URLs and PHPCS alignment Child attachment URLs with pretty permalinks take the form /parent-slug/attachment-slug/, not just /attachment-slug/. Fix test URLs to include the parent post slug. Break consecutive self:: assignments with blank lines to avoid PHPCS MultipleStatementAlignment warnings between differently-named properties. --- .../tests/canonical/attachmentRedirect.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/tests/canonical/attachmentRedirect.php b/tests/phpunit/tests/canonical/attachmentRedirect.php index 2ea701edd6bfb..64bddc61ae36a 100644 --- a/tests/phpunit/tests/canonical/attachmentRedirect.php +++ b/tests/phpunit/tests/canonical/attachmentRedirect.php @@ -45,6 +45,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_status' => 'publish', ) ); + self::$attached_to_public = $factory->post->create_and_get( array( 'post_type' => 'attachment', @@ -66,6 +67,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_author' => self::$editor_user, ) ); + self::$attached_to_private = $factory->post->create_and_get( array( 'post_type' => 'attachment', @@ -87,6 +89,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'post_author' => self::$editor_user, ) ); + self::$attached_to_draft = $factory->post->create_and_get( array( 'post_type' => 'attachment', @@ -140,13 +143,16 @@ public function test_unattached_query_var_url_redirects_when_pages_disabled() { // ------------------------------------------------------------------------- /** - * Attachment on a public post should redirect via slug URL. + * Attachment on a public post should redirect via its child slug URL. + * + * With pretty permalinks, child attachment URLs take the form + * /parent-slug/attachment-slug/. */ public function test_attached_to_public_post_slug_url_redirects() { update_option( 'wp_attachment_pages_enabled', 0 ); $this->set_permalink_structure( '/%postname%/' ); - $this->assertCanonical( '/public-attached-image/', $this->get_expected_path( self::$attached_to_public->ID ) ); + $this->assertCanonical( '/public-post/public-attached-image/', $this->get_expected_path( self::$attached_to_public->ID ) ); } // ------------------------------------------------------------------------- @@ -162,7 +168,7 @@ public function test_attached_to_private_post_no_redirect_for_anonymous() { $this->set_permalink_structure( '/%postname%/' ); wp_set_current_user( 0 ); - $this->assertCanonical( '/private-attached-image/', '/private-attached-image/' ); + $this->assertCanonical( '/private-post/private-attached-image/', '/private-post/private-attached-image/' ); } // ------------------------------------------------------------------------- @@ -177,7 +183,7 @@ public function test_attached_to_private_post_redirects_for_authorized_user() { $this->set_permalink_structure( '/%postname%/' ); wp_set_current_user( self::$editor_user ); - $this->assertCanonical( '/private-attached-image/', $this->get_expected_path( self::$attached_to_private->ID ) ); + $this->assertCanonical( '/private-post/private-attached-image/', $this->get_expected_path( self::$attached_to_private->ID ) ); } // ------------------------------------------------------------------------- @@ -192,7 +198,7 @@ public function test_attached_to_draft_post_no_redirect_for_anonymous() { $this->set_permalink_structure( '/%postname%/' ); wp_set_current_user( 0 ); - $this->assertCanonical( '/draft-attached-image/', '/draft-attached-image/' ); + $this->assertCanonical( '/draft-post/draft-attached-image/', '/draft-post/draft-attached-image/' ); } // -------------------------------------------------------------------------