Opened 6 weeks ago
Last modified 4 weeks ago
#65754 new defect (bug)
Revisionable, Multivalue Post Meta are not saved properly via REST autosave
| Reported by: | wongjn | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | REST API | Version: | 6.4 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
The Autosave REST API controller does not take into account meta fields that may be multivalued, it seems to assume all meta fields passed to it are single (trac source):
// Attached any passed meta values that have revisions enabled.
if ( ! empty( $meta ) ) {
foreach ( $revisioned_meta_keys as $meta_key ) {
if ( isset( $meta[ $meta_key ] ) ) {
update_metadata( 'post', $revision_id, $meta_key, wp_slash( $meta[ $meta_key ] ) );
}
}
}
Set up
- Register a revisionable, multivalue post meta field. Minimal plugin example:
<?php /* * Plugin Name: Test Autosave */ function _test_autosave_register_meta() { $args = array( 'sanitize_callback' => 'sanitize_text_field', 'show_in_rest' => array( 'schema' => array( 'type' => 'string', ), ), 'revisions_enabled' => true, ); register_post_meta( 'post', 'foo', $args ); } add_action( 'init', '_test_autosave_register_meta' );
- Set the
AUTOSAVE_INTERVALto something low for easier testing, like5:wp config set AUTOSAVE_INTERVAL 5 --raw
Ordefine( 'AUTOSAVE_INTERVAL', 5 );
- Add some post meta values:
wp post meta add 1 foo bar wp post meta add 1 foo baz
- Verify the data in REST,
GET /wp-json/wp/v2/posts/1?_fields=meta{ "meta": { "foo": [ "bar", "baz" ], "footnotes: "" } }
- Open the post in the block editor.
- Open devtools to be ready.
- Edit the content.
- Wait for autosave.
- View the
POST /wp-json/wp/v2/posts/1/autosavesresponse from the server in devtools network tab. You'll see something like:{ … "meta": { "foo": [ null, null ], … } … }
If you query the database directly, you'll see in the postmeta table, there will be 2 rows for the autosave, with identical serialized array values. Each member being null, and there being 2 (one for each value):
+---------+---------------------+--------------------+
| post_id | meta_key | meta_value |
+---------+---------------------+--------------------+
| 1 | foo | a:2:{i:0;N;i:1;N;} |
| 1 | foo | a:2:{i:0;N;i:1;N;} |
+---------+---------------------+--------------------+
This is further supports my assertion that this is incorrect behavior.
WordPress Playground Reproduction
Steps 1, 2 & 3 already set up for testing.
Blueprint
{
"$schema": "https://playground.wordpress.net/blueprint-schema.json",
"login": true,
"landingPage": "/wp-admin/post.php?post=1&action=edit",
"steps": [
{
"step": "writeFile",
"path": "/wordpress/wp-content/plugins/test-autosave.php",
"data": "<?php\n/*\n * Plugin Name: Test Autosave\n */\n\nfunction _test_autosave_register_meta() {\n\t$args = array(\n\t\t'sanitize_callback' => 'sanitize_text_field',\n\t\t'show_in_rest' => array(\n\t\t\t'schema' => array(\n\t\t\t\t'type' => 'string',\n\t\t\t),\n\t\t),\n\t\t'revisions_enabled' => true,\n\t);\n\tregister_post_meta( 'post', 'foo', $args );\n}\nadd_action( 'init', '_test_autosave_register_meta' );"
},
{
"step": "activatePlugin",
"pluginName": "Test Autosave",
"pluginPath": "/wordpress/wp-content/plugins/test-autosave.php"
},
{
"step": "wp-cli",
"command": "wp post meta add 1 foo bar"
},
{
"step": "wp-cli",
"command": "wp post meta add 1 foo baz"
},
{
"step": "wp-cli",
"command": "wp config set AUTOSAVE_INTERVAL 5 --raw"
}
],
"preferredVersions": {
"wp": "latest",
"php": "8.3"
},
"features": {}
}
Change History (5)
This ticket was mentioned in PR #12788 on WordPress/wordpress-develop by jigneshbhavani.
6 weeks ago
#1
- Keywords has-patch has-unit-tests added
#2
@
6 weeks ago
Reproduced on trunk, and the stored data is a bit worse than the report suggests. PR: https://github.com/WordPress/wordpress-develop/pull/12788
The relevant part is in WP_REST_Autosaves_Controller::create_post_autosave(). When the autosave revision is created, _wp_copy_post_meta() copies each value of a multiple value key into its own row, which is right. The controller then writes the submitted values with:
update_metadata( 'post', $revision_id, $meta_key, wp_slash( $meta[ $meta_key ] ) );
update_metadata() without a $prev_value updates every row sharing that meta key, so each of those rows gets overwritten with the whole array rather than one value each:
meta_key | meta_value
foo | a:2:{i:0;s:3:"bar";i:1;s:3:"qux";}
foo | a:2:{i:0;s:3:"bar";i:1;s:3:"qux";}
That matches the two identical serialized rows you saw in postmeta.
There is a second, smaller issue just above it, in the change detection:
$old_meta = get_metadata_raw( 'post', $post_id, $meta_key, true ); $new_meta = $meta[ $meta_key ] ?? '';
$single is hardcoded to true, so for a multiple value key the stored data is read back as a single string and compared against an array. That comparison never matches, so the autosave is always treated as different from the post and a revision gets written on every request even when nothing changed. Not part of the original report, but it has the same cause, so it is fixed in the same patch. Happy to split it out if that is preferred.
The patch keys off the single argument the meta key was registered with, rather than checking whether the submitted value happens to be an array. That distinction matters, because a key registered with single => true can legitimately hold an array, and the existing test_update_item_with_json_meta covers that case. For multiple value keys it replaces the stored rows the same way _wp_copy_post_meta() writes them, delete_metadata() followed by one add_metadata() per value. Single value meta still goes through update_metadata() and is unaffected.
Added a regression test. On trunk it fails with:
Failed asserting that two arrays are identical. - 0 => 'bar' - 1 => 'qux' + 0 => Array ( 0 => 'bar', 1 => 'qux' ) + 1 => Array ( 0 => 'bar', 1 => 'qux' )
and passes with the change. The restapi, revision and meta groups all pass.
#3
@
4 weeks ago
- Milestone Awaiting Review → 6.4
Thanks for the report. My investigation indicates that this issue first appeared in r56714, which corresponds to WordPress 6.4.
#4
@
4 weeks ago
Confirmed, r56714 is the one. It added both of the lines this fixes to create_post_autosave(): get_metadata_raw( 'post', $post_id, $meta_key, true ) with $single hardcoded, and the update_metadata() call with no $prev_value. The first release containing it is 6.4.0.
Should the 6.4 go in Version rather than Milestone? The ticket is currently the only open one in the 6.4 milestone, which completed in 2023, so I am not sure it will surface in triage from there.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
## Problem
WP_REST_Autosaves_Controller::create_post_autosave()stores every revisioned meta key with a single call:update_metadata( 'post', $revision_id, $meta_key, wp_slash( $meta[ $meta_key ] ) );update_metadata()called without a$prev_valueupdates *every* row that shares the meta key. For a meta key registered withsingle => falsethe autosave revision already has one row per value, copied there by_wp_copy_post_meta(). Each of those rows is then overwritten with the complete array, so a key with two values ends up as:meta_key | meta_value foo | a:2:{i:0;s:3:"bar";i:1;s:3:"qux";} foo | a:2:{i:0;s:3:"bar";i:1;s:3:"qux";}instead of one row holding
barand another holdingqux.There is a second, smaller problem in the change detection a few lines above:
$singleis hardcoded totrue, so for a multiple value key the stored data is read back as a single string and compared against an array. That comparison can never match, so the autosave is always considered different from the post even when nothing actually changed, and a revision is written on every request.## Fix
Whether a key is single is now taken from the
singleargument it was registered with, rather than from whether the submitted value happens to be an array. That distinction matters: a key registered withsingle => truecan legitimately hold an array, and the existingtest_update_item_with_json_metacovers exactly that case. Keys with no registration keep the previous single behaviour.For multiple value keys the stored rows are replaced the same way
_wp_copy_post_meta()writes them in the first place, withdelete_metadata()followed by oneadd_metadata()per value. The change detection reads back with the matching$singleargument so the comparison is like for like.Single value meta is unaffected and still goes through
update_metadata().## Testing instructions
Register a revisionable, multiple value post meta field:
Add two values to a post, then open it in the block editor, change the content and wait for an autosave:
Inspect the
POST /wp-json/wp/v2/posts/1/autosavesresponse. Before this change the returnedmeta.foodoes not reflect the submitted values and the revision rows each contain the whole serialized array. After it, each value is stored in its own row and comes back correctly.Automated coverage is included:
test_update_item_with_multiple_value_metafails on trunk with:and passes with this change. The full
restapi,revisionandmetagroups pass.