REST API: Restore the global post after preparing a revision - #12248
REST API: Restore the global post after preparing a revision#12248MicahelE wants to merge 17 commits into
Conversation
WP_REST_Revisions_Controller::prepare_item_for_response() set the global $post and called setup_postdata() without restoring them, so the change leaked for the rest of the request. The autosaves controller delegates here, so preloading that endpoint in the block editor could leave the global $post pointing at an autosave and initialize the editor with the wrong post. Capture the previous global post and restore it on every return path. Update the existing "sets up postdata" tests to confirm rendered fields still reflect the revision while the global post no longer leaks. Fixes #65495.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Hi there! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Test ReportPatch tested: this PR (#12248), applied on top of Environment
Steps to reproduce
Reproduction script (WP-CLI), taken from the ticket: wp_set_current_user( 1 );
$pid = wp_insert_post( array( 'post_title' => 'Repro 65495', 'post_content' => 'original body', 'post_status' => 'publish' ) );
wp_create_post_autosave( array( 'post_ID' => $pid, 'post_title' => 'Repro 65495', 'post_content' => 'autosaved body', 'post_type' => 'post' ) );
global $post; $post = get_post( $pid ); setup_postdata( $post );
$before = (int) $GLOBALS['post']->ID;
$req = new WP_REST_Request( 'GET', "/wp/v2/posts/$pid/autosaves" );
$req->set_param( 'context', 'edit' );
rest_get_server()->dispatch( $req );
$after = (int) $GLOBALS['post']->ID;
printf( "before=%d after=%d => %s\n", $before, $after, $after !== $before ? "LEAKED to $after" : "OK" );Expected resultAfter the request, the global Actual result —
|
|
@MicahelE Thank you for the PR. Please add the AI disclosure section from the PR template to the description: wordpress-develop/.github/pull_request_template.md Lines 22 to 33 in 8071b4a |
|
@westonruter I will do that, thanks |
|
@MicahelE, do you have the bandwidth to address the feedback? Thank you! |
There was a problem hiding this comment.
Pull request overview
Fixes a REST API global state leak where WP_REST_Revisions_Controller::prepare_item_for_response() overwrites global $post / postdata via setup_postdata() and fails to restore them, which can cause downstream request handling (notably block editor autosaves preloading) to behave as if the autosave/revision is the “current” post.
Changes:
- Capture and restore the previous global post when preparing revisions (including the HEAD early-return path).
- Add/adjust PHPUnit coverage to ensure the global post is restored for revisions and autosaves responses.
- Update existing “sets up postdata” tests to validate rendered fields while confirming no global
$postleakage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php |
Saves/restores the prior global post context during revision response preparation. |
tests/phpunit/tests/rest-api/rest-revisions-controller.php |
Adds tests for restoring global $post (GET, HEAD, and no-global-post scenarios) and updates postdata assertions. |
tests/phpunit/tests/rest-api/rest-autosaves-controller.php |
Updates autosave postdata test to verify rendered fields without leaking the global post. |
Comments suppressed due to low confidence (1)
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:733
rest_prepare_revisionis applied after callingreset_post_data(), so callbacks for that filter can no longer rely on the revision’ssetup_postdata()context (template tags would resolve against the restored post instead of the revision). To avoid a behavior change for existing hooks while still preventing leaks, reset the globals afterapply_filters()returns.
$this->reset_post_data( $previous_post );
/**
* Filters a revision returned from the REST API.
*
* Allows modification of the revision right before it is returned.
*
* @since 4.7.0
*
* @param WP_REST_Response $response The response object.
* @param WP_Post $post The original revision object.
* @param WP_REST_Request $request Request used to generate the response.
*/
return apply_filters( 'rest_prepare_revision', $response, $post, $request );
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Corrects the @SInCE tag to 7.1.0, adds a nullable WP_Post type hint and void return type to reset_post_data(), and guards the saved global post with an instanceof check. Follow-up to [62161].
|
@t-hamano Thanks for the ping, and for merging I've pushed 8a9d73e, which applies @westonruter's three suggestions:
The AI disclosure section was already added to the description.
The Copilot review that came in alongside your comment raises two behavioural points — resetting the globals before |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:627
- The global post is reset before running the
rest_prepare_revisionfilter. Any filter callbacks that rely onsetup_postdata()having prepared globals for the revision (e.g., callingget_post()/template tags without passing an ID) will now see the previous global post instead, which is a behavior change. Consider restoring globals after the filter runs (but still before returning) so callbacks observe the same revision context while still preventing leakage beyond this method.
if ( $request->is_method( 'HEAD' ) ) {
$this->reset_post_data( $previous_post );
/** This filter is documented in wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php */
return apply_filters( 'rest_prepare_revision', new WP_REST_Response( array() ), $post, $request );
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:724
reset_post_data()is called before the finalrest_prepare_revisionfilter. For consistency with existing behavior, filter callbacks should still run while the revision is the global post/postdata, with globals restored immediately afterwards (before returning) to avoid leaking outside this method.
$this->reset_post_data( $previous_post );
/**
* Filters a revision returned from the REST API.
*
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:754
- When there is no previous global post,
wp_reset_postdata()may be a no-op (e.g. when$wp_query->postis empty). In that case the other globals populated bysetup_postdata()($id,$authordata, etc.) will still contain the revision’s values and continue leaking into the rest of the request even though$GLOBALS['post']is unset.
} else {
unset( $GLOBALS['post'] );
wp_reset_postdata();
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:754
- When
$previous_postis null,wp_reset_postdata()can repopulate$GLOBALS['post']from the main query (WP_Query::reset_postdata()sets$GLOBALS['post']when$wp_query->postis non-empty). That contradicts the intent to leave the global post unset when it was unset before. Unset$GLOBALS['post']after callingwp_reset_postdata()to preserve the previous state.
} else {
unset( $GLOBALS['post'] );
wp_reset_postdata();
}
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:627
reset_post_data()runs before therest_prepare_revisionfilter and return. Other REST controllers (e.g.WP_REST_Posts_Controller::prepare_item_for_response()inclass-wp-rest-posts-controller.php:1889-1897) keep$GLOBALS['post']/postdata set to the item while runningrest_prepare_*filters, so restoring early changes the filter context and can break filter callbacks that rely on global post state. Apply the filter first, then restore globals, then return.
if ( $request->is_method( 'HEAD' ) ) {
$this->reset_post_data( $previous_post );
/** This filter is documented in wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php */
return apply_filters( 'rest_prepare_revision', new WP_REST_Response( array() ), $post, $request );
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php:720
reset_post_data()is called before the finalrest_prepare_revisionfilter, which changes the global-post context seen by filter callbacks compared to other REST controllers (which runrest_prepare_*with$GLOBALS['post']set to the item). Consider running the filter first, then restoring globals, then returning the filtered response.
$this->reset_post_data( $previous_post );
tests/phpunit/tests/rest-api/rest-autosaves-controller.php:750
- This test indexes into
$response->get_data()without asserting the response was successful first. Adding a status assertion improves error reporting if the request fails and avoids notices from unexpected response shapes.
$request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/autosaves/' . self::$autosave_post_id );
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();
tests/phpunit/tests/rest-api/rest-revisions-controller.php:730
- This test reads
$response->get_data()and indexes into it without first asserting a successful response. Adding a status assertion here makes failures easier to diagnose and avoids notices if an error response is returned.
$request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/revisions/' . $this->revision_id1 );
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();
Set up the global post with `query_posts()` and `the_post()` so the full set of postdata globals is populated, then assert against `get_post()` and the global `$id` rather than reading `$GLOBALS['post']` directly. The previous assertions passed even when the restore skipped `setup_postdata()`, leaving the rest of the postdata globals pointing at the revision. Point the HEAD request test at the single revision route. The collection endpoint short-circuits before preparing items for HEAD requests, so the test never reached the branch it was meant to cover. Rename it to match the route it now exercises. Assert the response status and shape before indexing into the data, and use core's `Type|null` syntax for the `@global` tags. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the global post. Importing the global post into prepare_item_for_response() aliased the local `$post` to `$GLOBALS['post']`. Restoring the previous global post therefore wrote through that alias and overwrote `$post` before it was passed to the `rest_prepare_revision` filter, so the filter received the previous global post instead of the revision it documents. This only happened when a global post was set beforehand. Without one, the restore takes the `unset( $GLOBALS['post'] )` branch, which detaches the reference rather than writing through it, leaving `$post` intact. Apply the filter before restoring the global post at both return points. This also keeps the globals that filter callbacks see pointing at the revision, as they did previously. Assert the filter arguments in the existing tests with MockAction. Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When there was no global post to restore, reset_post_data() unset the global post and then called wp_reset_postdata(). That order defeated the unset: WP_Query::reset_postdata() assigns `$GLOBALS['post'] = $this->post` whenever the main query has a post, so a request could introduce a global post where there had been none. Reset the post data first, then unset the global post. The reset is still needed to clear the revision from the remaining post data globals. Add a test covering a main query that has a post while the global post is unset, which is the state a request is in before the loop runs. Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assert the response status and shape before indexing into the data, so a failed request reports the status rather than an undefined index. Set the global post up with query_posts() and the_post() and assert against get_post() and the global `$id`, matching the revision tests. Reading `$GLOBALS['post']` directly passed even when the restore skipped setup_postdata(), leaving the remaining post data globals on the autosave. Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Declaring `global $post` in `WP_REST_Revisions_Controller::prepare_item_for_response()` aliased the method's `$post` to the global. A filter that reassigned the global post while the response was being prepared -- as a plugin running a secondary loop on `the_content` may do -- therefore retargeted every field read afterwards, including `excerpt`, `meta`, the `parent` link, and the post passed to `rest_prepare_revision`. Restore the method-local `$post` and assign the global separately, as before, so that capturing the previous global post for later restoration no longer changes which post the remaining fields are read from. Add a regression test that swaps the global post on `the_content` and asserts the prepared fields and the `rest_prepare_revision` argument still come from the revision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`WP_REST_Revisions_Controller::prepare_item_for_response()` now restores the global post before returning rather than leaving the revision in place. Record that behavior change in the docblock, per the documentation standards for changed behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The method restores the global post that was in place before the revision was prepared, so `reset` described the opposite of its main branch and read confusingly next to core's `wp_reset_postdata()`, which it calls for the unrelated case where there was no previous global post to restore. The method is private, so there is no external effect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two `test_get_item_sets_up_postdata_without_leaking_global_post()` tests were the only ones added for this ticket without a `@covers` tag. Also move `@ticket` above `@global` in the revisions test so the tag order matches the surrounding tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only the global post is guaranteed to be restored. When there was no previous global post and the main query has no post either, which is the usual state during a REST request, `wp_reset_postdata()` is a no-op, so the remaining globals set by `setup_postdata()` -- among them `$id`, `$authordata` and `$pages` -- are left describing the revision. The global post is what leaked into the block editor, and it is now cleared. Record the rest of the behavior in the docblock rather than leaving it noted only in a test comment, so the limitation is visible to anyone reading the method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`global $post` binds a caller to the global symbol table entry. Unsetting that entry removes it from the symbol table, which detaches every binding made beforehand: the holder keeps a value that is no longer the global, so a later write through it is invisible to `get_post()`. That path is reachable. A caller's own `global $post` creates the entry as null, which is not a `WP_Post`, so there is no previous global post to restore and the clearing branch runs. Assigning null instead leaves the entry in place and keeps the binding intact, while `get_post()` still returns null because `isset()` is false for null. Add a regression test covering a caller that binds before the request and sets the post afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Two follow-ups from reviewing this, neither blocking the fix itself. 1.
|
westonruter
left a comment
There was a problem hiding this comment.
This ended up being more involved than I thought!
`WP_REST_Revisions_Controller::prepare_item_for_response()` set the global `$post` to the revision being prepared and called `setup_postdata()`, but never restored the previous value, so the change persisted for the remainder of the request. `WP_REST_Autosaves_Controller` delegates to this method and the block editor preloads the autosaves endpoint on every load, so a post with a pending autosave could leave the global `$post` pointing at the autosave revision while `edit-form-blocks.php` built the editor bootstrap, initializing the editor with the wrong post and rewriting the URL to it. The previous global `$post` is now captured before `setup_postdata()` and restored on every return path, including the early return for HEAD requests. Developed in #12248. Follow-up to r40601, r59899. Props micahele, gusgomezpg, westonruter, wildworks, dhrupo. See #40626, #43502. Fixes #65495. git-svn-id: https://develop.svn.wordpress.org/trunk@62952 602fd350-edb4-49c9-b593-d223f7449a82
`WP_REST_Revisions_Controller::prepare_item_for_response()` set the global `$post` to the revision being prepared and called `setup_postdata()`, but never restored the previous value, so the change persisted for the remainder of the request. `WP_REST_Autosaves_Controller` delegates to this method and the block editor preloads the autosaves endpoint on every load, so a post with a pending autosave could leave the global `$post` pointing at the autosave revision while `edit-form-blocks.php` built the editor bootstrap, initializing the editor with the wrong post and rewriting the URL to it. The previous global `$post` is now captured before `setup_postdata()` and restored on every return path, including the early return for HEAD requests. Developed in WordPress/wordpress-develop#12248. Follow-up to r40601, r59899. Props micahele, gusgomezpg, westonruter, wildworks, dhrupo. See #40626, #43502. Fixes #65495. Built from https://develop.svn.wordpress.org/trunk@62952 git-svn-id: http://core.svn.wordpress.org/trunk@62193 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Trac: https://core.trac.wordpress.org/ticket/65495
WP_REST_Revisions_Controller::prepare_item_for_response()sets the global$postand callssetup_postdata(), but never restores them, so the change leaks for the rest of the request.The autosaves controller delegates here, and the block editor preloads
/autosaveson every load. When the post has a pending autosave, the global$postis left pointing at the autosave, so the editor can initialize with the wrong post id and redirect to it. That presents as the editor randomly opening a different post.This captures the previous global
$postand restores it on every return path (including the HEAD early return), which also covers the autosaves endpoint.Reproduction (no browser):
Before:
LEAKED. After:OK.Tests: Adds coverage for the restore (GET, HEAD, and the no-global-post case). The existing "sets up postdata" tests were asserting the leak, so they now confirm rendered fields still reflect the revision while the global post is restored.
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Initial implementation and test drafting; final code and tests were reviewed, edited, and verified by me.