`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.

[SirLouen]

Combined Bug Reproduction and Patch Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patches tested:

• First Patch: ​https://github.com/WordPress/wordpress-develop/pull/1165.diff
• Second Patch (Unit Tests): ​https://github.com/WordPress/wordpress-develop/pull/4591.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.29.0
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty 2.9
• MU Plugins: None activated
• Plugins:
  • Postdata Test 1.0.0
  • Test Reports 1.2.0

Bug Reproduction

1. Create a post. Take note of the ID for such post
2. For testing this, we need to add the code in Supp Artifacts somewhere where it could be executed, like a plugin, functions.php, etc… This code takes the code provided in the OP and adds a shortcodes to display the results of using that function during the loop processing.
3. Modify the code here: $test_post = get_post(1); and instead of 1 use the ID you got in the first step
4. Create a second post
5. Add the shortcode [test_postdata]
6. Check the post results
7. 🐞 Post ID after API call shows the first one used instead of the current post, as expected. This proves that REST Request is modifying the global $post data.

Expected Results

• The Post ID after API is the same before and after

Actual Results

1. ✅ Issue resolved with first patch.
2. ⚠️ Unit Tests are inadequate

Additional Notes

• The solution is pretty straightforward, as commented by @TimothyBlynJacobs long time ago. I can't understand why i thas not been merged yet. Maybe the absence of valid Unit Tests has been the problem. I will work on something.

• I'm trying to understand the Unit Tests provided in ​PR 4591, but they don't seem to be correct. For the first Unit Test test_postdata_reset_by_prepare_item_for_response, it's using a variable $post_id for the posts endpoint that has not even been declared previously.

• For the second one test_postdata_reset_by_prepare_item_for_response_revisions, it returns almost immediately Attempt to read property "post_content" on null. The problem here is that ​it will not add a revision because the post has not changed

• For this, I'm restoring the needs-unit-tests workflow tag, as the ones provided are not valid.

Supplemental Artifacts

Test Code:

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;
}

function test_template() {
    global $post;
    echo 'Original Post ID: ' . $post->ID . '<br>';
    
    $test_post = get_post(1);
    $response = convert_post_object_to_rest_response($test_post);
    echo 'Returned API ID: ' . $response['id'] . '<br>';

    echo 'Post ID After API Call: ' . $post->ID . '<br>';
    echo 'Post Title: ' . get_the_title() . '<br>';
}

add_shortcode('test_postdata', 'test_template');

[SirLouen]

Blocker #63635

[sachinrajcp123]

Fixes issue where setup_postdata() is not followed by wp_reset_postdata() in prepare_item_for_response().

[sachinrajcp123]

I’ve submitted a patch (43502.5.diff) that adds wp_reset_postdata() after setup_postdata() in the prepare_item_for_response() method of the WP_REST_Posts_Controller class.

This ensures global post data is restored after rendering post data for the REST API. Kindly review the patch.

Props sachinrajcp123