Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#52373 new defect (bug)

URL returned by get_attachment_link() can 404.

Reported by: peterwilsoncc's profile peterwilsoncc Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4
Component: Media Keywords: has-patch has-unit-tests
Focuses: Cc:


The permalink returned by get_attachment_link() and get_permalink( /* attachment */ ) can result in a file not found page in the following circumstances:

  • The attachment's post parent has been deleted but post_parent is set (ie, post parent is pointing to an invalid ID).
  • The attachment's post parent is an unregistered post type.

In both cases get_permalink() will return a URL in the format that will 404 when visited on the front end of the website.

If pretty URLs are enabled, the URL should return as the permalink.

Adding the following unit test will demonstrate the bug:

public function test_attachment_attached_to_non_existent_post_type_does_not_404() {
        global $wp_post_types;

        $this->set_permalink_structure( '/%year%/%monthnum%/%day%/%postname%/' );

        // Create temporay post type.
        register_post_type( 'not_a_post_type', array( 'public' => true ) );
        $post_id = self::factory()->post->create( array( 'post_type' => 'not_a_post_type' ) );

        // Attach media to post of post type.
        $attachment_id = self::factory()->attachment->create_object(
                        'post_mime_type' => 'image/jpeg',
                        'post_type'      => 'attachment',
                        'post_title'     => 'An Attachment!',
                        'post_status'    => 'inherit',

        // Unregister post type.
        foreach ( $wp_post_types as $id => $pt ) {
                if ( 'not_a_post_type' === $pt->name ) {
                        unset( $wp_post_types[ $id ] );

        // Visit permalink.
        $this->go_to( get_permalink( $attachment_id ) );
        $this->assertQueryTrue( 'is_attachment' );

This looks to have been introduced in #1914

Change History (4)

This ticket was mentioned in PR #926 on WordPress/wordpress-develop by peterwilsoncc.

3 years ago

  • Keywords has-patch has-unit-tests added

Failing unit tests to start with.

#2 @peterwilsoncc
3 years ago

If pretty URLs are enabled, the URL should return as the permalink.

This is only true for the %postname% permalink structure.

If pretty URLs are enabled, the URL should return a URL in the same format as the post post type, but with attachment/attachment-name as the %postname% replacement.

#3 @peterwilsoncc
3 years ago

In 50132:

Canonical: Prevent ID enumeration of private post slugs.

Add check to redirect_canonical() to ensure private posts only redirect for logged in users.

Modifies the read_post mata capability to user get_post_status() rather than the post's post_status property to allow attachments to redirect based on the inherited post status.

Introduces wp_force_ugly_post_permalink() to unify the check to determine if an ugly link should be displayed in each of the functions used for determining permalinks: get_permalink(), get_post_permalink(), _get_page_link() and get_attachment_link().

Improves logic of get_attachment_link() to validate parent post and resolution of inherited post status. This is an incomplete fix of #52373 to prevent the function returning links resulting in a file not found error. Required to unblock this ticket.

Props peterwilsoncc, TimothyBlynJacobs.
See #52373.
Fixes #5272.

This ticket was mentioned in Slack in #core-media by peterwilsoncc. View the logs.

3 years ago

Note: See TracTickets for help on using tickets.