Make WordPress Core

Opened 11 months ago

Last modified 10 days ago

#41445 assigned defect (bug)

post_parent can prevent media from embedding correctly

Reported by: loboyle Owned by: adamsilverstein
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.4
Component: REST API Keywords: has-patch needs-testing dev-feedback needs-unit-tests
Focuses: rest-api Cc:


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

        "message":"You don't have permission to do this.",

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

Attachments (1)

41445.diff (2.2 KB) - added by stuartfeldt 4 months ago.

Download all attachments as: .zip

Change History (22)

This ticket was mentioned in Slack in #core by kadamwhite. View the logs.

10 months ago

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

10 months ago

#3 @ritter.aaron
10 months ago

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

This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.

9 months ago

#5 @benjamincreative
9 months ago

  • Version 4.7.5 deleted

Same here on 4.8.2

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

This ticket was mentioned in Slack in #core by virusakos. View the logs.

8 months ago

#7 @riobahtiar
7 months ago

  • Focuses rest-api added
  • Keywords needs-patch added
  • Version set to 4.8.3

Hi All, Any updates for this ticket?

Because I just experienced the same issue,

WordPress Version 4.8.3

#8 @mythovik
7 months ago

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:


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

#9 @Krstarica
5 months ago

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

#10 @stuartfeldt
5 months ago

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.

#11 @neoian2
4 months ago

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.

#12 @riobahtiar
4 months ago

  • Version 4.8.3 deleted

#13 @riobahtiar
4 months ago

  • Version set to 4.9.4

bugs still occurs in v4.9.4

This ticket was mentioned in Slack in #core-restapi by riobahtiar. View the logs.

4 months ago

#15 @adamsilverstein
4 months ago

@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?

4 months ago

#16 @stuartfeldt
4 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#17 @stuartfeldt
4 months ago

  • Keywords dev-feedback added

#18 @adamsilverstein
4 months ago

@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

#19 @Krstarica
13 days ago

Still present in 4.9.6.

#20 @adamsilverstein
10 days ago

  • Keywords needs-unit-tests added

This could also use unit tests.

#21 @adamsilverstein
10 days ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to adamsilverstein
  • Status changed from new to assigned
Note: See TracTickets for help on using tickets.