`WP_REST_Posts_Controller::check_read_permission()` should check if `$parent` exists before calling itself

[GhostToast]

In WP_REST_Posts_Controller::check_read_permission() there ends up being a check for if the post_status of the post in question is 'inherit' and if the post_parent is greater than 0. It then checks the parent for permissions regarding the child. However, sometimes (as I have found with attachments), the child believes it has a parent, but the parent is missing. This results in a null post being sent to $this->check_is_post_type_allowed() which fails early and throws error about property of non-object.

I believe if the parent is missing, the next sequence should take precedence: which is that if post_status is 'inherit', check_read_permission() returns true. This would essentially be the same as if the post_parent value was set to 0.

A simple check if $parent has a value before passing it to a recursive call of check_read_permission() can alleviate this.

To reproduce this error, try to access a post with attachments, using _embed on the REST API, where one of the attachments has a post_parent that is invalid (integer that doesn't exist in corresponding wp_posts table).

[GhostToast]

Check if parent exists before using it

[jnylen0]

Punting, 4.7.3 is upon us.

[jnylen0]

I investigated this a bit more, thinking that it might be pretty simple to fix, but I'm going to stick with my earlier recommendation to punt. Here's why:

• It's not clear to me how a post would get into this situation in the first place. wp_delete_post handles this situation by resetting the post_parent of any attachments, so this is likely to be very uncommon.

• WP core itself is pretty broken when this situation does occur. I forced it manually for the following attachment - ​https://nylen.io/wp-dev/i-need-dis-otter/ - you'll note the page 404s, which is also the case when I'm logged in. However, wp-admin still points here for the attachment description page, and this is the value of the link field in the REST API response. (The API URL is ​https://nylen.io/wp-dev/wp-json/wp/v2/media/21, which I can only view when authenticated.)

Given the above, this needs more discussion, probably a broader fix than just the REST API, and of course unit tests. The existing behavior is kind of broken, but at least it's consistently broken in wp-admin and the REST API.

[GhostToast]

At the very least I would argue some error handling be in place. It is assumed that because a non-zeropost_parent property is present, a valid $post object will exist with that ID. What about if we check for get_post() having returned something useful? get_post_status() handles this a bit, can see similar example here: https://core.trac.wordpress.org/browser/tags/4.7.2/src/wp-includes/post.php?marks=#L655

[rachelbaker]

Added unit tests for returning media file with an invalid parent id

[rachelbaker]

In 39881.2.diff​ I added two unit tests. The first test returns the media file successfully with the post_parent is invalid but the post_status is inherit. The second returns an error if the post_parent is invalid but the post_status is auto-draft.

[rachelbaker]

In 40306:

REST API: Confirm the parent post object of an attachment exists in WP_REST_Posts_Controller::check_read_permission().

Avoid a PHP Error when attempting to embed the parent post of an attachment, when the parent post ID is invalid. Instead check if the parent post object exists before checking the read permission for the parent post.

Props GhostToast.
Fixes #39881.

[rachelbaker]

@jnylen0 This can occur when a post and related post_meta are deleted with a MySQL query, instead of using wp_delete_post(). An attachment can have only one post_parent in WordPress but be used in multiple posts (talk about functionality that is kinda broken). Is this situation common? NO. Should our REST API endpoints return invalid data due to PHP Errors when it does? I wouldn't consider attachment pages as an apple to apple comparison for our media endpoint.

[rachelbaker]

Re-opened for porting to the 4.7.4 branch.

[swissspidy]

In 40337:

REST API: Confirm the parent post object of an attachment exists in WP_REST_Posts_Controller::check_read_permission().

Avoid a PHP Error when attempting to embed the parent post of an attachment, when the parent post ID is invalid. Instead check if the parent post
object exists before checking the read permission for the parent post.

Props GhostToast.
Fixes #39881.

Merges [40306] to the 4.7 branch.