WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 3 weeks ago

#43502 new defect (bug)

`WP_REST_Posts_Controller::prepare_item_for_response()` doesn't reset postdata after calling setup_postdata()

Reported by: gerbenvandijk Owned by:
Milestone: 4.9.7 Priority: normal
Severity: normal Version: 4.9.4
Component: REST API Keywords: needs-patch needs-unit-tests
Focuses: rest-api Cc:

Description

Hi all,

I seem to have stumbled upon a small oversight in WP_REST_Posts_Controller::prepare_item_for_response() , where it doesn't reset postdata after calling setup_postdata(), rendering this part of the API useless for developers who want to use this API in their custom code.

Ran into it while writing a custom function, that uses this part of the API to convert a regular post object into an object that matches the json scheme of the API.

I’ve fixed it temporarily by doing setting $GLOBALS[ 'post' ] = $original_post_id in my function (and by passing original_post_id as parameter to my function) - but it seems to me that this should be fixed in core.

function convert_post_object_to_rest_response($post)
{

    global $wp_rest_server;
    $post_type = get_post_type_object($post->post_type);
    $request = WP_REST_Request::from_url(rest_url(sprintf('wp/v2/%s/%d', $post_type->rest_base, $post->ID)));
    $request = rest_do_request($request);
    $data = $wp_rest_server->response_to_data($request, isset($_GET[ '_embed' ]));
    return $data;

}

It returns the right data, which is great - but when i in turn use this function to supply a field in one of my endpoints with this dat, all fields below it seem to inherit the ID i’ve passed.

i’ve traced this back to WP_REST_Posts_Controller::prepare_item_for_response, where it’s doing a setup_postdata call without resetting it later.

For now i’ve fixed it by setting $GLOBALS[ 'post' ] = $original_post_id in my function (and by passing original_post_id as parameter to my function), but I figured i'd report it here so it can be fixed from the core.

Change History (7)

#1 @SergeyBiryukov
2 months ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.9.5

Hi @gerbenvandijk, welcome to WordPress Trac! Thanks for the report.

WP_REST_Revisions_Controller::prepare_item_for_response() has the same issue.

#2 @soulseekah
2 months ago

The prepare_item_for_response method and the controller itself is not supposed to be called from within a loop, which is what you seem to be doing. You appear to be using a whole REST server for mere data transformation, overkill and unintended use ahead :)

However, it indeed does set the $post global and sets the global post data up. Should it? Probably not. Does it - yes. Why? get_the_content(), which doesn't accept an explicit post_ID argument, meaning that it will only work with a global context setup.

Should it play nice and reset the $post global to what it was? Maybe.

Should it also call wp_reset_postdata? Certainly not. For one it has no effect in the REST server context. For two, if it's called from your scenario it would actually mess up your loop, resetting too early (wp_reset_postdata should only be called after the loop has ended, right?)

In my opinion, this should be left alone. The context the REST server is being called from is highly atypical. The server is expected to live a very short life serving one response per request.

Thoughts?

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


2 months ago

#4 @audrasjb
2 months ago

  • Milestone changed from 4.9.5 to 4.9.6

Bumping to 4.9.6 due to 4.9.5 beta release.

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


5 weeks ago

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


4 weeks ago

#7 @desrosj
3 weeks ago

  • Milestone changed from 4.9.6 to 4.9.7

No progress here. Going to punt.

Note: See TracTickets for help on using tickets.