Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#49695 closed defect (bug) (fixed)

REST API check_template function can return false error

Reported by: kipperlenny's profile Kipperlenny Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.6 Priority: normal
Severity: normal Version: 4.9
Component: REST API Keywords: 2nd-opinion dev-feedback needs-patch needs-unit-tests
Focuses: rest-api Cc:

Description

wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php Line 1209

function check_template


This function does not check get_post for an empty post:

<?php
$allowed_templates = wp_get_theme()->get_page_templates( get_post( $request['id'] ), $this->post_type );

On my Setup, this returned the WP_Error rest_invalid_param because get_post() returned an empty Post (with $post->ID == 0).

I changed this to:

<?php
$post = get_post( $request['id'] );
if($post->ID == 0) {
        $post = null;
}

// If this is a create request, get_post() will return null and wp theme will fallback to the passed post type.
$allowed_templates = wp_get_theme()->get_page_templates( $post, $this->post_type );

This checks, if post is empty. This will save some API Requests from running in errors and does no harm.

PS: No idea, why get_post() can return an empty WP_Post object. My API did not send an ID and was a POST Request.

Change History (7)

#1 @TimothyBlynJacobs
4 years ago

  • Keywords reporter-feedback added
  • Version changed from 5.3.2 to 4.9

Hi @Kipperlenny,

Thanks for the ticket. Have you tested this with all plugins disabled and a default theme? The global post should be not set at that point unless a plugin is interfering and setting invalid data.

#2 @Kipperlenny
4 years ago

YES - and I know the problem is inside a different plugin (it is using $post = "something different, not a wp post" inside the main file).

But - check_template() from WP Rest API should be more reliable about this - it cannot be sure, that $post is used somewhere else in a different way.

Thats why I suggest:

<?php
$post = get_post( $request['id'] );
if($post->ID == 0) {
        $post = null;
}

// If this is a create request, get_post() will return null and wp theme will fallback to the passed post type.
$allowed_templates = wp_get_theme()->get_page_templates( $post, $this->post_type );

or even better:

<?php
$requested_post = get_post( $request['id'] );
if($post->ID == 0) {
        $requested_post = null;
}

// If this is a create request, get_post() will return null and wp theme will fallback to the passed post type.
$allowed_templates = wp_get_theme()->get_page_templates( $requested_post, $this->post_type );

if you don't want to use $post here.

With this easy change, check_template() could work more reliable.

#3 @Kipperlenny
4 years ago

  • Keywords reporter-feedback removed

#4 @TimothyBlynJacobs
4 years ago

  • Keywords 2nd-opinion dev-feedback needs-patch needs-unit-tests added; has-patch removed

From my perspective, there are a lot of different ways that plugins could interfere with the REST API so it can return invalid results and I don't think this situation is particularly different. Maybe others feel differently.

Cc: @kadamwhite, @spacedmonkey.

#5 @spacedmonkey
4 years ago

I am not against adding the following lines at the start of check_template method.

$post = $this->get_post( $request['id'] );
                if ( is_wp_error( $post ) ) {
                        return $post;
                }

I don't understand why the issue is happening?

#6 @TimothyBlynJacobs
3 years ago

  • Milestone changed from Awaiting Review to 5.6

#7 @TimothyBlynJacobs
3 years ago

  • Owner set to TimothyBlynJacobs
  • Resolution set to fixed
  • Status changed from new to closed

In 49301:

REST API: Make template handling resilient against plugins setting the global post.

Plugins shouldn't be setting the global post object during a REST API request, but if they did this could cause unexpected errors when creating a post with a template.

Props Kipperlenny, TimothyBlynJacobs.
Fixes #49695.

Note: See TracTickets for help on using tickets.