WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 5 months ago

#49695 new defect (bug)

REST API check_template function can return false error

Reported by: Kipperlenny Owned by:
Milestone: Awaiting Review 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 (5)

#1 @TimothyBlynJacobs
7 months 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
7 months 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
7 months ago

  • Keywords reporter-feedback removed

#4 @TimothyBlynJacobs
5 months 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
5 months 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?

Note: See TracTickets for help on using tickets.