post_parent can prevent media from embedding correctly

[loboyle]

If media is uploaded for a post, then used as a featured image on another post, and the original parent is not accessible via the REST API (e.g. because it's in the trash, not published etc), then it cannot be embedded on the post that is accessible.

To reproduce

• make a new post with a featured image
• trash the post
• make a new post, using the first image as the featured image
• request the second post over the rest API with media embedding enabled

The media will not be embedded, instead a forbidden result will be embedded error

{
  "wp:featuredmedia":[  
     {  
        "code":"rest_forbidden",
        "message":"You don't have permission to do this.",
        "data":{  
           "status":403
        }
     }
  ]
}

See ​https://github.com/WP-API/WP-API/issues/2596 for the original issue. Also related is https://core.trac.wordpress.org/ticket/30691.

[ritter.aaron]

we experience the same issue, with the difference that the image was first uploaded to the library via Medialibrary and then attached to a pods post in to a field without any meta data (basicaly using pods with additional fields in table) (which works) and then attached to a blog post which showed the same issue with permission denied 403 (when directly attaching to the post it worked)

one more thing i found is that when attaching to the post the file name gets truncated

when uploading via media library it does not get truncated

[benjamincreative]

Same here on 4.8.2

Uploaded to one custom post type as draft, added to another public custom post type...

[riobahtiar]

Hi All, Any updates for this ticket?

Because I just experienced the same issue,

WordPress Version 4.8.3

[mythovik]

In my case was a jpg error. Some images make this error:

After a while i discover that is about the name of the pic, in my case:

PeñaNieto.jpg

The "ñ" and other special chars, could cause this error.

[Krstarica]

Same on 4.9.1.

If post_parent of image is having status other than 'publish', rest_forbidden occurs.

For example, for image having ID 125676, the result of this query has to be 'publish':

SELECT b.post_status FROM wp_posts a, wp_posts b WHERE a.ID = 125676 AND a.post_parent=b.ID

[stuartfeldt]

So can we override get_item_permissions_check in WP_REST_Attachments_Controller, and then check if the requested attachment id is also set as _thumbnail_id postmeta -- and then if the request attachment id is a thumbnail on a published post allow it in the wp:featuredmedia response?

Or is that too much overhead to check all of wp_postmeta for each of these calls?

I'm happy to submit a patch if this is an acceptable approach.

[neoian2]

This is driving me nuts. The only solution I've found is to upload a new image for each post until this is fixed.

I feel like @stuartfeldt's solution would work, but it seems like a temporary fix to a bigger problem.

[riobahtiar]

bugs still occurs in v4.9.4

[adamsilverstein]

@stuartfeldt Your suggestion to override get_item_permissions_check in WP_REST_Attachments_Controller, and then check if the requested attachment id is also set as _thumbnail_id postmeta sounds reasonable - could you work on a patch?

[adamsilverstein]

@stuartfeldt thanks for the patch!

I have a couple of questions/comments about the code:

• should we check that $requestid? available before using?
• add a query limit to avoid runaway queries
• you can remove $has_permission entirely: just return true if $this->check_read_permission( $post ); is true
• similarly, I don't think need to track $wp_error - instead, since you could be looking thru multiple posts with the same attached image, you are really looking for matches you have access to and can ignore the rest.

Reviewing the approach I also have two basic architectural concerns:

• performance impact - meta queries are expensive
• possible unexpected side effects, eg opening permissions where they should not be

This ticket could use feedback/review from a REST API component maintainer about the overall approach. cc: @rmccue @kadamwhite @joehoyle @rachelbaker @jnylen0

[Krstarica]

Still present in 4.9.6.

[adamsilverstein]

This could also use unit tests.

[danielbachhuber]

Here's another GitHub issue with prior conversation on the topic: ​https://github.com/WP-API/WP-API/issues/1987#issuecomment-247355568

Essentially, this is a flaw in WordPress' permissions model for attachments. The only way around it is with a meta query (querying to see if the attachment is used as _thumbnail_id on any posts), but meta queries have negative performance implications.

[sboisvert]

Regarding the query performance concerns. VIP has added an index on meta_key, meta_value(50) to address these types of queries (the index replaces the meta_key index). This makes these reverse lookups go very quickly. Perhaps it would be worth including it in core for this. It seems that reverse lookups on meta_value are getting more and more common from the code we see.

[nickrigby]

This issue also occurs if you attach the featured image at draft stage (the post has never been published). The post parent of the attachment retains the post parent id of a revision, which means the same permissions problem occurs.

[jwktje]

Any update on this issue?

[adamsilverstein]

@jwktje Thanks for the ping. Probably too late for this to land in 5.3, I'm milestoning as 5.4 so we make sure to address it then.

[kadamwhite]

#48314 was marked as a duplicate.

[danielbachhuber]

I've just run into this myself :P

At the very least, we should remove the WP_Error object from the embedded data. I don't think it makes sense to include in any cases.

[jmacon]

hey @adamsilverstein wondering if this got folded in to the 5.4 release? Thanks!

[galbaras]

This seems to indicate a problem of WordPress not updating media-to-post attachment information when a post changes status to/from "publish".

Basically, if a post is trashed, it's incorrect to show its embedded images as attached to it. If WordPress updated media records with the deletion, there would not be an API issues.

Conversely, if a deleted post is reinstated, its embedded images should be re-attached to it.

Trac ticket:

[TimothyBlynJacobs]

Instead of having to do an expensive meta query every time we check read permissions, could we try updating the post parent instead?

So whenever a post is moved to a non-public status, check if it is the parent of any media items. If so, looks for a new parent candidate by doing the _thumbnail_id meta query?

[peterwilsoncc]

The get_post_status() function accounts for attachments with an inherit status on trashed and deleted parents:

• for attachments on trashed posts, it returns the parent's status immediately prior to it been trashed: get_post_meta( $post->post_parent, '_wp_trash_meta_status', true )
• for attachments on deleted posts, it assumes the attachment has a published status

Could the REST API use get_post_status() in the attachment controller when determining the permissions?

Trac ticket:

[adamsilverstein]

41445.3.diff​ uses get_post_status as suggested by @peterwilsoncc. I added this in src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php (instead of over-riding in the media controller) since it should be safe to use across post types.

Does this approach resolve the issue you reported @loboyle?

[loboyle]

Looks like an elegant fix, yes - thanks @peterwilsoncc and @adamsilverstein.

[orhanc]

@adamsilverstein We're facing the same issue with the latest release of wordpress version 5.8.2.

[adamsilverstein]

Hey @orhanc thanks for the ping, apologies for letting this ticket linger... I had overlooked this 'future release' ticket and it is probably too late to land this change for 5.9, so I will tag it for 6.0.

Although the code in 41445.3.diff​ works in testing, it is not quite ready to land...

First, @TimothyBlynJacobs raised an important point in this comment - calling get_post_status can add an additional meta query (specifically when the parent is in the trash) so it might be worth considering what Timothy suggested:

whenever a post is moved to a non-public status, check if it is the parent of any media items. If so, looks for a new parent candidate by doing the _thumbnail_id meta query?

So that would be another post that had used the image as featured?

Also from @galbaras -

Conversely, if a deleted post is reinstated, its embedded images should be re-attached to it.

That would be good to support if possible, especially since it is the current behavior. The exception would be the case where the image has already been attached to a different post (which isn't possible now).

In addition, this change needs some backing unit tests to validate the expected behavior.

[galbaras]

Here's a bit of a radical idea: the relationship between media and posts is currently many-to-one due the use of post_parent to handle it. However, in real life, it's many-to-many, because there's nothing stopping users from embedding the same item in multiple posts.

WordPress handles many-to-many relationships via wp_postmeta, not wp_posts fields. Perhaps it's time to review this relationship?

Having said that, I've seen cases where images are linked to posts (by plugins, e.g. within a gallery or custom field) without updating their respective post_parent fields. So there may be a need for a way (e.g. scheduled task) to clean that up.

Subsequently, if a parent post is deleted, the association with the published post can still be used to check whether the media should be accessible.

[szabolcsgleszer]

Hi @galbaras , @adamsilverstein,

thanks for your comments.

If I understood it well, the whole concept for the relationship between media and posts has to be revalidated and only after that we can expect to have a fix for this?
Either a new one or the one from Adam.

I am asking it again, because we are dependent from it and would like to know if we can calculate with this feature or not.

[costdev]

Per the discussion in the bug scrub, I'm adding has-testing-info. See the ticket's description for testing instructions.

Does anyone have any further thoughts on how to proceed without the additional, expensive meta query? Is the radical approach in this comment desirable, or?

Additionally, the current PR was submitted against master. This should be submitted against trunk instead.

I've retargeted this to trunk and merged in an up-to-date copy to trigger the tests.

[Krstarica]

@galbaras has a point. There are several ways media can be handled:
1) Upload media during post editing
2) Add existing media in the new post
3) Upload media using Media Library, then use it in the post
4) Reference existing media using custom field/plugin

WordPress creates relationship between media and post for (1) scenario only. This should be definitely reviewed (in the new ticket?)

In the meantime we should probably completely remove post_parent check, because we don't have a clue whether media is added to the post using (2), (3) and (4) scenario.

[galbaras]

There are also scenarios where the same image is included in more than one post, of which only one is inaccessible. Currently, if post_parent is inaccessible, that's the end of the road (false negative), and if it's accessible, this might be a problem (false positive).

[jnz31]

i'm bumping into this problem for the third time now. and each time i come to the conclusion, that i don't need the permissions check. can we get a filter to disable the check once and for all?

[lukasbesch]

#47782 was marked as a duplicate.

I am also voting for a many-to-many relationship (shared some thoughts on the topic in the other ticket: https://core.trac.wordpress.org/ticket/47782#comment:4).

This could also be very handy to programmatically clean up the media library and identify unused (or most used) images.

[antpb]

It was agreed in the recent Media component meeting that this ticket may need to move to 6.1 to allow enough time to solve and test.

[antpb]

Moving this into 6.2 as there was not enough progress in the current release to bring this across the line. It looks really close, just needs unit tests and confirmation that the fix is still valid.

[adamsilverstein]

The patch I worked on no longer works as expected and I'm not sure the original ticket from @stuartfeldt applies still - maybe someone can test that?

Marking this as needs-patch, I'll leave this as targeting 6.2 for now since this is a bug.

[kkalarickal]

We are at the test table in WC Asia - 2023 Bangkok Contributor Day

We are trying to reproduce this original issue using these instructions against the latest nightly build (2/17/2023).

• make a new post with a featured image
• confirm that requesting the first post over the rest API with media embedding enabled works.

e.g.
Request : ​http://<your-site-url>/wp-json/wp/v2/posts/secondPostID?_embed
Response: look for "wp:featuredmedia" entry.

• now trash the post

• make a new post, using the first image as the featured image
• now do a request of the second post over the rest API with media embedding enabled

Expectation: with the latest nightly build the API request should fail with code:rest_forbidden

[ashokrd2013]

Tested this ticket from WC ASIA Bangkok 2023 Contributor Day

Reproduction Report

This report validates that the issue can be reproduced.

Environment

• OS: macOS 11.2.3
• Web Server: Nginx
• PHP: 7.4.30
• WordPress: 6.2-beta2-55340-src
• Browser: Safari 15.4
• Theme: Twenty Twenty-Two
• Active Plugins:
  • Hello Dolly 1.7.2
  • WP Reset 1.97

Actual Results

• Error condition occurs (reproduced).

Additional Notes

• I checked the following scenario:

1. Created post & added featured image
2. Created a second post with the same featured image used in the first post
3. Deleted first post and checked second post featured image metadata through URL - ​http://<your-site-url>/wp-json/wp/v2/posts/secondPostID?_embed
4. Issue reproduced.

Supplemental Artifacts

Before deleting the first Post - ​https://prnt.sc/5jFIaRBOnQZ1
After deleting the first post - ​https://prnt.sc/Oc1k6QR4I2eo

[sandsine]

Testing this issue from WordCamp Asia 2023 @ Bangkok, Thailand (#wcasia)

Reproduce this issue with the latest nightly build (2/17/2023).

Environment
OS: macOS 12.5
Web Server: Nginx
PHP: 8.1.9
WordPress: 6.2-beta2-55340-src
Browser: Chrome 09.0.5414.119
Theme: Twenty Twenty-Three
0 Active Plugins

---

Reproduce steps:

1. Create new post with new image and set as featured image, then publish the post.
2. Create a second post with the same image (from 1.) and set as featured image, then publish the post.
3. Get post data via WP JSON API:

http://<website_url>/wp-json/wp/v2/posts/<secondpost_id>?_embed

4. See result from JSON, the featured image was set, see this image and looking for wp:featuredmedia.
5. Then delete the first post (from 1.)
6. Refresh the WP JSON URL again (same URL as same as 3.) with and looking for wp:featuredmedia, the featured image meta data was deleted with the first post. You can see this image(https://imgur.com/a/MOWdHfd)?. (this's for after deleted the first post).

• The JSON API request failed with status code 401 and code "rest_forbidden" with the latest nightly build (2/17/2023).

7. ✅ Issue successfully reproduced.

[robinwpdeveloper]

From WordCamp Asia 2023 Test Table:
Verifying that Issue still exists in the latest trunk.
We have tried with multiple scenarios (manual testing).

[611shabnam]

We are testing from WordCamp Asia 2023 from Thailand.

This report validates that the issue can be reproduced.

Environment

---

OS: macOS Monetery 12.3.1 (21E258)
Web Server: Nginx
PHP: 7.4.1
WordPress: 6.2-beta2-55340-src
Browser: Safari 15.4
Theme: Twenty Twenty-Two
Active Plugins:
Hello Dolly 1.7.2
Akismet Antispam 5.0.2

Actual Results
Error condition occurs (reproduced).

---

Additional Notes
I checked the following scenario:
Created post & added featured image
Created a second post with the same featured image used in the first post
Deleted first post and checked second post featured image metadata through URL - ​http://<your-site-url>/wp-json/wp/v2/posts/secondPostID?_embed
Next, I created a third post and added the same featured image through the URL - ​http://<your-site-url>/wp-json/wp/v2/posts/secondPostID?_embed

Issue reproduced.

Supplemental Artifacts
Before deleting the Second Post - ​https://tinyurl.com/2ohw8x6b
After deleting the Second post - ​https://tinyurl.com/2e5529bk
After creating the third post - ​https://tinyurl.com/2k2k7yqm

[siddhantwadhwani]

Testing from WordCamp Asia 2023 - Contributor Day from Thailand, Bangkok. #WCAsia #Bluehost

This report validates that the issue can be reproduced, and also applying the latest patch seems to work straight-forward in the latest WordPress nightly build.

Applied and tested the patch: https://core.trac.wordpress.org/attachment/ticket/41445/41445.3.diff

Environment
OS: macOS Ventura 13.0.1
Web Server: Nginx
PHP: 7.4.33
WordPress: 6.2-beta2-55340-src
Browser: Chrome 109.0.5414.119
Theme: Twenty Twenty-Three (twentytwentythree)
Active Plugins:
SQL Buddy Version 1.0.0 by Delicious Brains
Actual Results
Error condition occurs (reproduced).
Patch-3 works successfully

Additional Notes
I tested the following scenarios:

Scenario 1: (Normal case)

1. Created a post & added a featured image. Published the post.
2. Created another post and added the same first (featured) image. Published the post.

Performed a Rest API call for the second post through the browser and Postman with media embedding enabled and was able to see appropriate data appear under the wp:featuredmedia json body.

Reference: ​https://imgur.com/cC0emJN

Scenario 2: (Issue reproduced)

1. Created a post & added a featured image. Published the post.
2. Created another post and added the same first (featured) image. Published the post.
3. Trashed the first post.

Noticed that on making the Rest API call for the second post, the wp:featuredmedia data returned forbidden error with a 401 response status code.
✅ Issue successfully reproduced.

Reference: ​https://imgur.com/Fxb89yi

Scenario 3 (Applying Patch-3) @adamsilverstein

1. Ran the command npm run grunt patch:41445 and selected the 3rd patch to apply (41445.3.diff)
2. Tested the end to end scenario again where I created the first and second post afresh.

I noticed that despite both using the same featured image and trashing the first one, the Rest api call for the second post wp:featuredmedia no longer throws the forbidden error we were seeing earlier!
✅ Patch applied and worked successfully.

Reference:
Patch applied Rest API call response - ​https://imgur.com/RcU0s8H

This concludes that this issue could still be reproduced and the latest patch works fine for this scenario straight-forward in the latest WordPress nightly build version

[costdev]

As we're approaching 6.2 Beta 4 and this ticket still has needs-patch and needs-unit-tests, I'll move this ticket to Future Release.

@adamsilverstein when you're available, can you run another test to see whether you're getting the same results as comment:62? I'm sure you're as curious as I am about the successful manual test result 🤔

I've merged in trunk which includes the fix for WP#52326, tests are now passing.

[robinwpdeveloper]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/839

Environment

• OS: macOS 11.2.3
• Web Server: Nginx
• PHP: 7.4.30
• WordPress: 6.3-alpha-55505-src
• Browser: Google Chrome 111.0.5563.110
• Theme: Twenty Twenty-Three
• Active Plugins:
  • No plugins activated.

Actual Results

• ✅ Issue resolved with patch.

Additional Notes

Steps Followed:

• Step 1: Create a post (Post 1) with a featured image
• Step 2: Create another post (Post 2) with the same image used in Post 1. Just select the image from media library. No need to upload.
• Step 3: Now trash Post 1.
• Step 4: Check <your-site-url>/wp-json/wp/v2/posts/secondPostID?_embed
• Step 5: Search wp:featuredmedia (2nd occurrence says: rest_forbidden)
• Step 6: Now apply the patch (nom run grunt patch:https://github.com/WordPress/wordpress-develop/pull/839) and recheck step 5. Now it shows media data (rest_forbidden issue resolved).

Supplemental Artifacts

Before: ​https://tinyurl.com/2mteyjcy
After: ​https://tinyurl.com/2mx722p8

[robinwpdeveloper]

We can update the keywords.
needs-patch and needs-testing can be removed
as the PR is working and issue mentioned in the PR discussion is resolved in another ticket.

Next steps:

• We need unit tests
• Review from core committer.

@costdev might help us (in free time) with the unit testing :)

So what is the problem here? I'm also waiting for a fix :)

[wagnerloch]

Still not fixed. I was trying to get featured media from a new post and it was returning 401. Checked the media and it was uploaded many years ago, probably it was on a deleted post.

[ekazda]

6+ years and still no fix for this. For users that need to overcome this, I added this to my functions file:

function modify_rest_response_featured_image($data, $post, $request) {
    // Get the featured media ID
    $featured_media_id = get_post_thumbnail_id($post->ID);

    if ($featured_media_id) {
        // Get the featured media post
        $featured_media = get_post($featured_media_id);

        // Prepare the attachment response manually
        $media_response = array(
            'id' => $featured_media->ID,
            'date' => $featured_media->post_date,
            'slug' => $featured_media->post_name,
            'type' => $featured_media->post_type,
            'link' => get_permalink($featured_media->ID),
            'title' => array(
                'rendered' => $featured_media->post_title
            ),
            'author' => $featured_media->post_author,
            'caption' => array(
                'rendered' => $featured_media->post_excerpt
            ),
            'alt_text' => get_post_meta($featured_media->ID, '_wp_attachment_image_alt', true),
            'media_type' => $featured_media->post_mime_type,
            'mime_type' => $featured_media->post_mime_type,
            'media_details' => array(
                'width' => get_post_meta($featured_media->ID, '_wp_attachment_metadata', true)['width'],
                'height' => get_post_meta($featured_media->ID, '_wp_attachment_metadata', true)['height'],
                'file' => $featured_media->guid,
                'filesize' => filesize(get_attached_file($featured_media->ID)),
                'sizes' => add_source_url_to_sizes(wp_get_attachment_metadata($featured_media->ID)['sizes'], $featured_media->ID),
                'image_meta' => get_post_meta($featured_media->ID, '_wp_attachment_metadata', true)['image_meta']
            ),
            'source_url' => wp_get_attachment_url($featured_media->ID)
        );

        // Add 'wp:featuredmedia' link to the response
        $data->add_link('wp:featuredmedia', rest_url('wp/v2/media/' . $featured_media_id), array('embeddable' => true));

        // Add 'wp:featuredmedia' array to the response, containing the media response
        $data->data['wp:featuredmedia'] = array($media_response);
    }

    return $data;
}

function add_source_url_to_sizes($sizes, $media_id) {
    foreach ($sizes as $key => &$size) {
        $attachment = wp_get_attachment_image_src($media_id, $key);
        $size['source_url'] = $attachment[0];
    }
    return $sizes;
}

function add_custom_rest_prepare_filters() {
    $post_types = get_post_types([], 'objects');

    foreach ($post_types as $post_type) {
        add_filter("rest_prepare_{$post_type->name}", 'modify_rest_response_featured_image', 10, 3);
    }
}

add_action('rest_api_init', 'add_custom_rest_prepare_filters');

I don't love this because I'm sure it adds a decent amount of overhead to my response time, but I need this to be reliable and this seemed to solve it for me. I just reference my custom data instead of the default data. Hope this helps somebody.

@adamsilverstein What are we waiting for here?