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

[gerbenvandijk]

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.

[SergeyBiryukov]

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

WP_REST_Revisions_Controller::prepare_item_for_response() has the same issue.

[soulseekah]

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?

[audrasjb]

Bumping to 4.9.6 due to 4.9.5 beta release.

[desrosj]

No progress here. Going to punt.

[desrosj]

Moving all tickets in 4.9.7 to 4.9.8.

[rachelbaker]

We did used to set the $post global back to the previous post in v1, but I missed this when adding the setup_postdata() function back in v2. Setting the global was needed for filters like the_content however I should have been kind enough to restore the global when done. Be kind, rewind!

This is worth a patch and a unit test.

Some links for historical reference:

WP REST API v1 where the $post global was restored: ​https://github.com/WP-API/WP-API/blob/master/lib/class-wp-json-posts.php#L687

Bug Introduced in this PR: ​https://github.com/WP-API/WP-API/pull/753

Responding to this issue: ​https://github.com/WP-API/WP-API/issues/738

[kakomap]

Thanks @rachelbaker. Looking into this

[manzoorwani.jk]

Hello guys,
Sorry for the mess up. Doing it for the first time :)
Please bear with me...

[manzoorwani.jk]

I hope the last one is OK :)

[manzoorwani.jk]

Any feedback on this? :)

[TimothyBlynJacobs]

Hi @manzoorwanijk, sorry for such the long delay in getting back to you.

This should be able to be simplified by calling wp_reset_postdata at the end of prepare_item_for_response since the main query will take care of backing up the post global.

This will also need unit tests.

Call wp_reset_postdata at the end of prepare_item_for_response method

Trac ticket: https://core.trac.wordpress.org/ticket/43502

Hi, I am including some tests for the changes from this PR:

​https://github.com/WordPress/wordpress-develop/pull/1165/

However I want to mention that the post data is still not being reverted even with the wp_reset_postdata() action added to the methods.

[elenachavdarova]

Hi,

In ​https://github.com/WordPress/wordpress-develop/pull/4591, I have included tests for the changes from this PR: ​https://github.com/WordPress/wordpress-develop/pull/1165

However I want to mention that the post data is still not being reverted even with the wp_reset_postdata() action added to the methods.