Opened 9 years ago
Last modified 2 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: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | REST API | Version: | 4.9.4 |
| Severity: | normal | Keywords: | good-first-bug has-patch has-unit-tests |
| Cc: | Focuses: | rest-api |
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.
Attachments (8)
Change History (43)
#2
@
8 years 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.
8 years ago
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
#10
@
8 years ago
- Keywords good-first-bug added
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
#12
@
8 years ago
Hello guys,
Sorry for the mess up. Doing it for the first time :)
Please bear with me...
This ticket was mentioned in Slack in #core-restapi by manzoorwanijk. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
7 years ago
#18
@
6 years ago
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.
This ticket was mentioned in PR #1165 on WordPress/wordpress-develop by engahmeds3ed.
5 years ago
#19
Call wp_reset_postdata at the end of prepare_item_for_response method
Trac ticket: https://core.trac.wordpress.org/ticket/43502
This ticket was mentioned in PR #4591 on WordPress/wordpress-develop by @elenachavdarova.
3 years ago
#20
- Keywords has-unit-tests added; needs-unit-tests removed
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.
#21
@
3 years ago
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.
#22
@
14 months ago
- Keywords needs-unit-tests added; has-unit-tests removed
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
- Create a post. Take note of the ID for such post
- 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.
- Modify the code here:
$test_post = get_post(1);and instead of1use the ID you got in the first step - Create a second post
- Add the shortcode
[test_postdata] - Check the post results
- 🐞 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
$postdata.
Expected Results
- The Post ID after API is the same before and after
Actual Results
- ✅ Issue resolved with first patch.
- ⚠️ 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_idfor thepostsendpoint that has not even been declared previously.
- For the second one
test_postdata_reset_by_prepare_item_for_response_revisions, it returns almost immediatelyAttempt 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-testsworkflow 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');
@
13 months ago
Fixes issue where setup_postdata() is not followed by wp_reset_postdata() in prepare_item_for_response().
#25
@
13 months ago
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.
@
5 months ago
Refreshed patch for current trunk. Saves and restores $GLOBALSpost in prepare_item_for_response(), including the HEAD early return path. Includes 2 unit tests. All 283 posts controller tests pass.
This ticket was mentioned in PR #11455 on WordPress/wordpress-develop by @liaison.
5 months ago
#27
This PR addresses a long-standing issue in WP_REST_Posts_Controller::prepare_item_for_response() where the global $post object was not restored after calling setup_postdata().
When the REST API processes post data, it temporarily modifies the global $post state to ensure core functions (like get_the_content()) work correctly. However, failing to reset this state leads to "global context pollution," especially when the REST API is called internally during a custom WP_Query loop or other custom development contexts.
Changes included in this PR:
Manual State Backup: Saves the original $GLOBALSpost before modification.
Robust Restoration: Ensures the global $post is restored to its original state (including null) in both the main execution path and the early-return path for HEAD requests.
Comprehensive Unit Tests: 1. Verified that the global $post is restored to its previous object after the API call.
- Verified that the global $post is restored to null if it wasn't set before the call.
Trac ticket: https://core.trac.wordpress.org/ticket/43502
Use of AI Tools
AI assistance: Yes
Tool(s): Gemini
Model(s): Gemini 3 Flash
Used for: Identifying the core architectural issue (global state pollution vs. wp_reset_postdata limitations) and drafting the technical explanation for the PR description. The final implementation was verified against the latest Trac patch (#43502.6.diff) and local PHPUnit tests.
#28
@
5 months ago
I've refreshed the patch and opened PR #11455.
https://github.com/WordPress/wordpress-develop/pull/11455
Tests conducted:
- Verified all 238 CI checks passed (PHP 7.2 - 8.4).
- Included 2 new unit tests specifically for global $post restoration (one for existing post, one for null).
- Confirmed fix for 'pollution' during internal REST API calls.
@liaison commented on PR #11455:
5 months ago
#29
@apermo, I've fixed the indentation. Thanks a lot.
@liaison commented on PR #11455:
5 months ago
#30
@apermo, I've fixed the indentation. Thanks a lot.
This ticket was mentioned in PR #11695 on WordPress/wordpress-develop by @motylanogha.
4 months ago
#31
prepare_item_for_response() overwrote $GLOBALS['post'] and called setup_postdata() without restoring the previous state. Anything running after a REST request that re-used the controller (block rendering, the_content filters, etc.) saw the wrong global post.
This PR captures the previous global before mutation and restores it on every return path, including the early HEAD-request return.
Tests cover:
- Restoration when a global post is already set
- Restoration to
nullwhen no global post was set - Restoration on the HEAD-method early-return branch
Complements GH-1165 and GH-11455.
@motylanogha commented on PR #11695:
5 weeks ago
#33
[62952] just fixed the identical setup_postdata() / unrestored global $post bug in WP_REST_Revisions_Controller (committed under #65495, with a "See #43502" reference).
This PR applies the same fix to the sibling case: WP_REST_Posts_Controller::prepare_item_for_response() still does $GLOBALS['post'] = $post; setup_postdata( $post ); (around line 1854) without restoring the previous $post on any return path, including the early HEAD return. Same fix shape as [62952], different controller — a natural follow-up while this area is being tidied up.
Rebased status: MERGEABLE, full CI green, and has unit tests covering the restore on every return path. CC @westonruter since you just handled the revisions controller.
@
2 weeks ago
Refreshed patch for #43502: adds wp_reset_postdata() before the final return in WP_REST_Posts_Controller::prepare_item_for_response(). Covers the standard response path only (not the HEAD-request early return).
#34
@
2 weeks ago
Tested this using a reproduction script that sets up postdata for one post via setup_postdata(), then makes an internal REST API request (rest_do_request()) for a *different* post mid-loop, then checks global $post afterward.
Without any fix:
BEFORE REST call: global $post ID = 9 AFTER REST call: global $post ID = 6 (leaked to the REST-requested post) Expected: global $post ID = 9
This confirms the bug: the REST response process leaves global $post pointing at the REST-requested post instead of restoring the original loop's post.
Note on the attached patch: its context no longer applies cleanly to current trunk. prepare_item_for_response() in class-wp-rest-posts-controller.php now calls setup_postdata( $post ) unconditionally near the top of the method (not conditionally inside a post_type_supports( ..., 'revisions' ) check as the original patch assumes), so it doesn't apply as-is and needs to be refreshed.
Attached 43502.7.diff, which adds wp_reset_postdata() before the final return apply_filters(...) statement in prepare_item_for_response(). With this in place, the bug is fixed:
BEFORE REST call: global $post ID = 9 AFTER REST call: global $post ID = 9 Expected: global $post ID = 9
Note: this patch covers the standard response path. There's also an early return for HEAD requests (if ( $request->is_method( 'HEAD' ) )) earlier in the method that isn't addressed here and may need the same treatment for full coverage.
Environment: WordPress trunk (wordpress-develop local env), PHP 8.3.33, Windows.
#35
@
2 weeks ago
Thanks for the runnable reproduction, that is the part these old tickets usually lack.
One correction on the direction of 43502.7.diff, and it is also the reason your script cannot see the problem: wp_reset_postdata() on its own does not restore the caller's state, it restores the *main* query's state.
function wp_reset_postdata() { global $wp_query; if ( isset( $wp_query ) ) { $wp_query->reset_postdata(); } } public function reset_postdata() { if ( ! empty( $this->post ) ) { $GLOBALS['post'] = $this->post; $this->setup_postdata( $this->post ); } }
Two consequences follow.
1. A secondary loop gets a different wrong post back, not the right one.
The shape in the original report is a template looping something other than the main query:
$q = new WP_Query( array( 'post_type' => 'book' ) ); while ( $q->have_posts() ) { $q->the_post(); // sets $GLOBALS['post'] from $q rest_do_request( ... ); // the leak happens here // wp_reset_postdata() inside the controller restores $wp_query->post, // which is the MAIN query's post, not $q's. }
WP_Query::the_post() sets the global from $this, while wp_reset_postdata() reads $GLOBALS['wp_query']. In a secondary loop those are different objects, so the patch swaps one wrong post for another. Your script restores correctly only because the loop it sets up is the main query, so the two coincide there.
2. It cannot restore "there was no post".
reset_postdata() is a no-op when $wp_query->post is empty, and it has no way to express unsetting the global. If $GLOBALS['post'] was not set before the call, the patch either leaves the leaked value in place or invents a main-query post.
What holds in both cases is capturing the previous value and putting it back:
$previous_post = isset( $GLOBALS['post'] ) ? $GLOBALS['post'] : null; $GLOBALS['post'] = $post; setup_postdata( $post ); // ... $GLOBALS['post'] = $previous_post; wp_reset_postdata(); return apply_filters( "rest_prepare_{$this->post_type}", $response, $post, $request );
You are right that the HEAD early return needs the same treatment, and it is easy to miss because it returns before the main exit point.
Both points are already covered in https://github.com/WordPress/wordpress-develop/pull/11695, which takes the save-and-restore approach above, handles the HEAD path, and adds three regression tests: the normal path, the HEAD path, and the case where the global was unset beforehand. It applies to current trunk and CI is green on it.
There is committed precedent for the shape as well. [62952] fixed the identical unrestored $post global in WP_REST_Revisions_Controller under #65495, carrying a "See #43502" reference. This ticket is the sibling case in WP_REST_Posts_Controller, so landing it the same way keeps the two controllers consistent.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)

Hi @gerbenvandijk, welcome to WordPress Trac! Thanks for the report.
WP_REST_Revisions_Controller::prepare_item_for_response()has the same issue.