Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#40029 closed defect (bug) (wontfix)

Media endpoint can return wrong number of results

Reported by: wonderboymusic's profile wonderboymusic Owned by: joehoyle's profile joehoyle
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: needs-patch
Focuses: Cc:

Description

WP_REST_Posts_Controller::check_read_permission() will filter out items from a collection based on readability of the item's parent.

Attachments have a weird association with a post via the "Uploaded To" paradigm, but can exist outside of a parent after that parent mysteriously disappears - example: bug in WordPress, post gets trashed and deleted but image does not rescind its association, etc.

What can happen - I request 10 items on page 1 and get back 7? The fulfillment of the request should be opaque to the client. I have 100s of images - if I ask for 10, I should get 10. The SQL probably needs to be more aggressive by including a clause like:

post_parent = 0 OR post_parent IN (.... {subquery that looks only for IDs that exist} ....)

Change History (4)

#1 @jnylen0
7 years ago

I think there are also other situations where the pagination/count values returned by the API are not reliable. We should investigate this more in general and either resolve these situations, or document this caveat and indicate that client code should be prepared to deal with this.

This situation in particular is pretty edge-case since WordPress should handle this correctly under normal circumstances. It would be good to see some repro steps if this is more common than I'm thinking. See also #39881.

#2 @rmccue
7 years ago

This is basically only an issue if plugins filter an individual post out using permissions. We should document this behaviour, but I don't see that there's a great way to "fix" it (i.e. always return the number you asked for). It's similar to requesting 10 items, but only 7 exist (i.e. small site, or at the end of pagination). The per_page value is an upper-bound only.

#3 @joehoyle
7 years ago

  • Owner set to joehoyle
  • Status changed from new to assigned

We should either close this or take the parent into account, however to I don't think we should go beyond what WP_Query is capable of. If WP_Query can't intelligently return the correct number of results then I don't think we should patch. The problem is we'll need to recursively fill to 10, and keep doing more WP_Queries.

Next step: work out if we can provide additional args to WP_Query when getting the list of media so it's returning the correct amount without us having to strip them manually PHP-side. I'm pretty sure that's _not_ going to be possible though.

#4 @wonderboymusic
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

I am pretty sure I reported this based on an inconsistent state in my DB due to cache, I think we can close this

Note: See TracTickets for help on using tickets.