Make WordPress Core

Opened 7 weeks ago

Closed 7 days ago

#65495 closed defect (bug) (fixed)

WP_REST_Revisions_Controller::prepare_item_for_response() leaks the global $post (block editor can initialize/redirect to the wrong post)

Reported by: gusgomezpg Owned by: westonruter
Priority: normal Milestone: 7.1
Component: REST API Version: 4.8
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses: rest-api

Description

This is the WP_REST_Revisions_Controller instance of the same defect tracked in #43502 (WP_REST_Posts_Controller::prepare_item_for_response() doesn't reset postdata after calling setup_postdata()). In comment:1 of that ticket, @SergeyBiryukov noted that the revisions controller has the same issue. This ticket documents the revisions/autosaves path specifically, with a deterministic reproduction, a user-visible block-editor symptom, and a patch.

Summary

WP_REST_Revisions_Controller::prepare_item_for_response() sets the global $post to the revision being prepared and calls setup_postdata(), but never restores the previous global $post. The change persists for the remainder of the request.

WP_REST_Autosaves_Controller extends this controller and delegates to it, so the leak occurs on the /wp/v2/<type>/<id>/autosaves route as well.

Why it is user-visible (block editor)

On every block-editor page load, block_editor_rest_api_preload() preloads the autosaves endpoint. When the edited post has a pending autosave, the global $post is left pointing at the autosave revision for the rest of edit-form-blocks.php. That file builds the inline wp.editPost.initializeEditor( 'editor', <type>, <id>, ... ) call from the global $post, so the editor can be initialized with the wrong post id and its history sync rewrites the URL -- i.e. the editor "redirects" to a different post. Because it only happens when an autosave exists, it presents as an intermittent "the editor randomly opens/redirects to another post" bug.

Affected code

wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php, prepare_item_for_response(): $GLOBALS['post'] is set and setup_postdata() called, with no restore on either the HEAD early-return path or the final return.

Call path

wp-admin/post.php
  -> wp-admin/edit-form-blocks.php  -> block_editor_rest_api_preload()
      -> rest_do_request() ... WP_REST_Server::dispatch()
          -> WP_REST_Autosaves_Controller::get_items()
              -> WP_REST_Autosaves_Controller::prepare_item_for_response()
                  -> WP_REST_Revisions_Controller::prepare_item_for_response()
                      -> setup_postdata()   // global $post = autosave revision, never reset

Reproduction (deterministic, no browser)

Clean install, default theme, no plugins. With WP-CLI:

wp eval '
wp_set_current_user( 1 );
$pid = wp_insert_post( array( "post_title"=>"Repro", "post_content"=>"original body", "post_status"=>"publish" ) );
wp_create_post_autosave( array( "post_ID"=>$pid, "post_title"=>"Repro", "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" );
'

Observed on stock WP 7.0: before=31 after=32 => LEAKED to 32 (after is the autosave revision). With the attached patch: before=N after=N => OK, consistently across repeated runs (verified by reverting/re-applying the patch on the same install).

Proposed fix

Capture the previous global $post before setup_postdata() and restore it on every return path (including the HEAD early-return). Fixing it in the revisions controller also covers the autosaves endpoint, which delegates here. Patch attached.

Note: wp_reset_postdata() alone is not a sufficient restore here -- in the REST/admin context the main query's post is not the post being edited, so the previous global must be captured and restored explicitly. (This matches the finding in #43502 comment:20/comment:21 that wp_reset_postdata() did not revert the data.) The helper restores the captured global, falling back to wp_reset_postdata() only when there was no previous global.

See #43502 (parent/posts controller). This patch is the revisions-controller counterpart and can be committed alongside it.

Attachments (1)

43502-revisions-controller.diff (2.1 KB ) - added by gusgomezpg 7 weeks ago.

Download all attachments as: .zip

Change History (11)

This ticket was mentioned in PR #12248 on WordPress/wordpress-develop by @micahele.


7 weeks ago
#1

  • Keywords has-unit-tests added; needs-unit-tests removed

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

WP_REST_Revisions_Controller::prepare_item_for_response() sets the global $post and calls setup_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 /autosaves on every load. When the post has a pending autosave, the global $post is 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 $post and restores it on every return path (including the HEAD early return), which also covers the autosaves endpoint.

Reproduction (no browser):

wp eval '
$pid = wp_insert_post( array( "post_status" => "publish" ) );
wp_create_post_autosave( array( "post_ID" => $pid, "post_content" => "x", "post_type" => "post" ) );
global $post; $post = get_post( $pid ); setup_postdata( $post );
rest_get_server()->dispatch( new WP_REST_Request( "GET", "/wp/v2/posts/$pid/autosaves" ) );
echo $GLOBALS["post"]->ID === $pid ? "OK" : "LEAKED";
'

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.

@dhrupo commented on PR #12248:


6 weeks ago
#2

## Test Report

Patch tested: this PR (#12248), applied on top of trunk.

Environment

  • WordPress: 7.1-alpha (latest trunk)
  • PHP: 8.3.30
  • Server: nginx (the wordpress-develop Docker environment)
  • Database: MySQL

Reproduction on trunk (before the patch)

Using the deterministic WP-CLI script from the ticket:

before=14 after=15 => LEAKED to 15

Confirmed: after a GET /wp/v2/posts/<id>/autosaves request, the global $post is left pointing at the autosave revision (15) instead of the post being edited (14).

After applying this PR

before=16 after=16 => OK

The global $post is restored to its previous value. Verified across repeated runs by reverting and re-applying.

Automated tests

phpunit --filter 'WP_Test_REST_Autosaves_Controller|WP_Test_REST_Revisions_Controller'
OK (105 tests, 549 assertions)

All green, including the new coverage for the leak, the HEAD-request early-return path, and the case where there was no previous global post (it's left unset rather than being clobbered).

The fix captures the previous global $post and restores it on every return path, which matches the root cause described in the ticket. Looks good to me. 👍

#3 @westonruter
6 weeks ago

  • Milestone Awaiting Review7.1
  • Version 7.04.8

This seems to have been introduced way back in WordPress 4.8 via r40601 to fix #40626.

Since it wasn't a bug introduced in 7.0, it doesn't make sense to include necessarily in 7.0.1.

@westonruter commented on PR #12248:


6 weeks ago
#4

@MicahelE Thank you for the PR. Please add the AI disclosure section from the PR template to the description:

https://github.com/WordPress/wordpress-develop/blob/8071b4a9e668e423e6bd3989efb7e73d4902cb3c/.github/pull_request_template.md?plain=1#L22-L33

@micahele commented on PR #12248:


6 weeks ago
#5

@westonruter I will do that, thanks

#6 @westonruter
6 weeks ago

  • Owner set to westonruter
  • Status newreviewing

@wildworks commented on PR #12248:


10 days ago
#7

@MicahelE, do you have the bandwidth to address the feedback? Thank you!

@micahele commented on PR #12248:


10 days ago
#8

@t-hamano Thanks for the ping, and for merging trunk in.

I've pushed 8a9d73e7cf, which applies @westonruter's three suggestions:

  • @since corrected to 7.1.0
  • reset_post_data() now takes ?WP_Post $previous_post and returns void
  • The saved global post is guarded with an instanceof WP_Post check

The AI disclosure section was already added to the description.

WP_Test_REST_Revisions_Controller and WP_Test_REST_Autosaves_Controller are green locally (105 tests, 549 assertions).

The Copilot review that came in alongside your comment raises two behavioural points — resetting the globals before rest_prepare_revision fires, and wp_reset_postdata() potentially repopulating $GLOBALS['post'] from $wp_query. I'll look at those separately.

@westonruter commented on PR #12248:


7 days ago
#9

Two follow-ups from reviewing this, neither blocking the fix itself.

### 1. rest_prepare_autosave now sees the restored global post

WP_REST_Autosaves_Controller::prepare_item_for_response() delegates to the revisions controller and *then* does the rest of its work:

Since the restore happens inside the delegated call, the last three now run with the global post restored, where previously they saw the autosave. So any rest_prepare_autosave callback — or any autosave register_rest_field() get_callback — relying on get_the_ID() or template tags will behave differently after this change.

That looks intended rather than accidental, and it follows naturally from fixing the leak. Flagging it mainly so it can be captured on the ticket and considered for a dev note, since it is a behavior change visible to extenders that the tests here would not catch.

Worth noting the revisions controller itself is unaffected on this point: its add_additional_fields_to_object() call happens *before* the restore, so revision additional-field callbacks still see the revision.

### 2. The same unrestored setup_postdata() pattern exists elsewhere

Out of scope for this ticket, but worth a follow-up:

The posts-controller one is less harmful in the reported scenario, since the post it leaves behind is usually the post being edited rather than an autosave, but it is the same underlying issue.

Links are pinned to 4eaba0e so the line references stay accurate.

---

*This comment was written by Claude Code (model: Claude Opus 5, 1M context) during a review of this PR, and posted after I checked it over. Line references were verified against the pinned commit.*

#10 @westonruter
7 days ago

  • Resolutionfixed
  • Status reviewingclosed

In 62952:

REST API: Restore the global post after preparing a revision.

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 https://github.com/WordPress/wordpress-develop/pull/12248.
Follow-up to r40601, r59899.

Props micahele, gusgomezpg, westonruter, wildworks, dhrupo.
See #40626, #43502.
Fixes #65495.

Note: See TracTickets for help on using tickets.