Make WordPress Core

Opened 2 months ago

Closed 6 weeks ago

Last modified 6 weeks ago

#64314 closed defect (bug) (fixed)

Fatal error when enabling revisions for post meta of type array/object

Reported by: lapsrj's profile LAPSrj Owned by: westonruter's profile westonruter
Milestone: 7.0 Priority: normal
Severity: normal Version: 6.4
Component: Revisions Keywords: has-patch php8
Focuses: php-compatibility Cc:

Description (last modified by westonruter)

Since WordPress 6.4 it is now possible to enable revisions for post meta. However when verifying if there are changes in the meta fields the function expects only strings, breaking if the meta format is array or object.

How to reproduce the bug:

  • Register a post meta of type array/object
  • Set a value for it in a post
  • Try to save the post

The API request to save the post will fail, returning a 500 error. The PHP log will list say:

PHP Fatal error: Uncaught TypeError: trim(): Argument #1 ($string) must be of type string, array given

The cause of the error is the function wp_save_post_revision, at wp-includes/revision.php. At line 190 it uses normalize_whitespace( $post->field ), which only works with strings, however $post->field will be an array if the meta is an array and that will cause the fatal error.

Since this is a supported feature since WordPress 6.4 and is causing fatal errors, I'm setting the severity to critical. Even though the issue is with meta fields, I'm setting the component as "Posts, Post Types" because the issue is at the part that handles saving the revisions and the same code is used both to fetch post attributes and post meta.

Attachments (1)

trac64314.zip (179.6 KB) - added by LAPSrj 2 months ago.
Plugin that reproduces the issue

Download all attachments as: .zip

Change History (14)

This ticket was mentioned in PR #10560 on WordPress/wordpress-develop by LAPSrj.


2 months ago
#1

  • Keywords has-patch added

This is my proposed fix for ticket 64314, where a fatal error happens when trying to save a post if you have enabled revision for a post meta that is an array.

In this proposed fix I'm using serialize to make any type of variable a string. I've chosen that because:

  • The smallest number of changes to the code
  • The same line of code will work for any type of variable
  • Is the way how WP saves arrays in the database, so seamed like a good choice.

Trac ticket: 64314

#2 @LAPSrj
2 months ago

  • Component changed from Posts, Post Types to Revisions

#3 @westonruter
2 months ago

  • Milestone changed from Awaiting Review to 7.0

@LAPSrj Would you share some example plugin code that registers array/object postmeta and enables revisions for testing purposes?

@LAPSrj
2 months ago

Plugin that reproduces the issue

#4 @westonruter
8 weeks ago

  • Owner set to westonruter
  • Status changed from new to reviewing

#5 @manhphucofficial
8 weeks ago

I was able to reproduce the fatal error on my local environment using the trac64314.zip reproducer plugin.

Environment:

  • WordPress 6.8.3
  • PHP 8.2.24

When saving a post that contains array/object postmeta, the editor displays “Updating failed.” and the REST API request returns an HTTP 500 error. The PHP error log shows the following fatal error:

[29-Nov-2025 14:44:42 UTC] PHP Fatal error:  Uncaught TypeError: trim(): Argument #1 ($string) must be of type string, array given in /var/www/html/wordpress-test/wp-includes/formatting.php:5499
Stack trace:
#0 /var/www/html/wordpress-test/wp-includes/formatting.php(5499): trim()
#1 /var/www/html/wordpress-test/wp-includes/revision.php(190): normalize_whitespace()
#2 /var/www/html/wordpress-test/wp-includes/revision.php(116): wp_save_post_revision()
#3 /var/www/html/wordpress-test/wp-includes/class-wp-hook.php(326): wp_save_post_revision_on_insert()
#4 /var/www/html/wordpress-test/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters()
#5 /var/www/html/wordpress-test/wp-includes/plugin.php(517): WP_Hook->do_action()
#6 /var/www/html/wordpress-test/wp-includes/post.php(5831): do_action()
#7 /var/www/html/wordpress-test/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php(1042): wp_after_insert_post()
#8 /var/www/html/wordpress-test/wp-includes/rest-api/class-wp-rest-server.php(1292): WP_REST_Posts_Controller->update_item()
#9 /var/www/html/wordpress-test/wp-includes/rest-api/class-wp-rest-server.php(1125): WP_REST_Server->respond_to_request()
#10 /var/www/html/wordpress-test/wp-includes/rest-api/class-wp-rest-server.php(439): WP_REST_Server->dispatch()
#11 /var/www/html/wordpress-test/wp-includes/rest-api.php(459): WP_REST_Server->serve_request()
#12 /var/www/html/wordpress-test/wp-includes/class-wp-hook.php(324): rest_api_loaded()
#13 /var/www/html/wordpress-test/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters()
#14 /var/www/html/wordpress-test/wp-includes/plugin.php(565): WP_Hook->do_action()
#15 /var/www/html/wordpress-test/wp-includes/class-wp.php(418): do_action_ref_array()
#16 /var/www/html/wordpress-test/wp-includes/class-wp.php(818): WP->parse_request()
#17 /var/www/html/wordpress-test/wp-includes/functions.php(1342): WP->main()
#18 /var/www/html/wordpress-test/wp-blog-header.php(16): wp()
#19 /var/www/html/wordpress-test/index.php(17): require('...')
#20 {main}
  thrown in /var/www/html/wordpress-test/wp-includes/formatting.php on line 5499

This confirms the issue described in this ticket: normalize_whitespace() calls trim(), which only accepts strings, but receives an array from $post->$field in wp_save_post_revision().

After applying the proposed fix from PR 10560 and updating the comparison to serialize the values:

<?php
foreach ( array_keys( _wp_post_revision_fields( $post ) ) as $field ) {
    if ( normalize_whitespace( serialize( $post->$field ) ) !== normalize_whitespace( serialize( $latest_revision->$field ) ) ) {
        $post_has_changed = true;
        break;
    }
}

the fatal error no longer occurs.

Results after applying the patch:

  • The post saves successfully with no “Updating failed.” notice
  • No more PHP fatal errors
  • Revisions are created correctly when array/object meta values change
  • I tested creating new posts and repeatedly modifying the meta, and everything continues to work as expected
  • No regressions observed in my testing

Based on my results, the patch in PR 10560 fully resolves the issue and appears safe to include.

Last edited 6 weeks ago by westonruter (previous) (diff)

#6 @westonruter
6 weeks ago

  • Severity changed from critical to normal
  • Version changed from 6.8.3 to 3.6

It looks like this was introduced in [23849] as part of WP 3.6.

#7 @westonruter
6 weeks ago

  • Version changed from 3.6 to 6.4

Well, as noted in the description, it was introduced specifically in 6.4 with the introduction of postmeta revisions in [56714] to fix #20299.

#8 @westonruter
6 weeks ago

  • Description modified (diff)

@westonruter commented on PR #10560:


6 weeks ago
#9

Gemini's review of all the changes, including the new test:

Here's my review of the changes:

Changes in src/wp-includes/revision.php:

The core change is the introduction of maybe_serialize() before normalize_whitespace():

  • src/wp-includes/revision.php

    a b function wp_save_post_revision( $post_id ) { 
    187187                       $post_has_changed = false;
    188188
    189189                       foreach ( array_keys( _wp_post_revision_fields( $post ) ) as $field ) {
    190                                if ( normalize_whitespace( $post->$field ) !== normalize_whitespace( $latest_revision->$field ) ) {
     190                               if ( normalize_whitespace( maybe_serialize( $post->$field ) ) !== normalize_whitespace( maybe_serialize( $latest_revision->$field ) ) ) {
    191191                                       $post_has_changed = true;
    192192                                       break;
    193193                               }
  • Reasoning: This is an excellent change to address a potential TypeError in PHP 8.0+ when normalize_whitespace() (which internally uses preg_replace()) receives a non-string value (like an array or object). By using maybe_serialize(), any non-scalar values will be converted to a string representation before whitespace normalization, preventing a crash and allowing for consistent comparison.
  • Coding Standards: The change adheres to WordPress coding standards regarding formatting and function usage. maybe_serialize() is an idiomatic WordPress function for handling data that might need serialization for storage or comparison.
  • PHP 7.2 Compatibility: This change is fully compatible with PHP 7.2 and proactively prevents issues that arise in PHP 8.0+.

Changes in tests/phpunit/tests/post/revisions.php:

A new test method test_wp_save_post_revision_with_array_post_meta() has been added.

  • @ticket and @covers Tags: Correctly uses @ticket 64314 and @covers ::wp_save_post_revision.
  • Comprehensive Setup: The test cleverly sets up the scenario by:
    • Explicitly setting wp_save_post_revision_check_for_changes to true to ensure change detection is active.
    • Using the wp_post_revision_meta_keys filter to ensure that the custom favorite_things meta key is saved with each revision. This is crucial for the latest_revision to have the correct data for comparison.
    • Using the _wp_post_revision_fields filter to inform wp_save_post_revision that favorite_things should be considered a revisionable field, allowing WP_Post::__get to retrieve the meta value.
  • Clear Assertions:
    • The test sets an initial array value, saves a revision, and asserts that the revision was created and that the array was correctly saved to the revision's meta.
    • It then updates the array value, saves a second revision, and asserts that this second revision was successfully created (proving that the change was detected) and that the updated array was saved to its meta.
  • Relevance to Fix: This test directly validates the fix by introducing an array into a revisioned field and confirming that wp_save_post_revision works as expected without errors and with proper change detection.
  • Nitpicks: The use of anonymous functions for filters is acceptable in unit tests, as WP_UnitTestCase typically cleans up filters between tests. No explicit remove_filter or remove_all_filters is strictly necessary at the end of the test method in this context for robustness.

Conclusion:

Both the code change in src/wp-includes/revision.php and the accompanying unit test in tests/phpunit/tests/post/revisions.php are well-executed. The change is a robust fix for a compatibility issue, and the test provides excellent coverage for the scenario, demonstrating adherence to best practices for extending WordPress revision functionality with custom array-based meta.

The overall quality of the proposed changes is high.

#10 @westonruter
6 weeks ago

Note:

If anyone is experiencing this issue, a workaround is simply to add this code to prevent WP from checking for changes when a new revision is being saved:

<?php
if ( version_compare( get_bloginfo( 'version' ), '7.0', '<' ) ) {
        add_filter( 'wp_save_post_revision_check_for_changes', '__return_false' );
}

#11 @westonruter
6 weeks ago

  • Focuses php-compatibility added
  • Keywords php8 added

#12 @westonruter
6 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 61372:

Revisions: Prevent fatal error in PHP 8+ when saving a post revision with revisioned non-scalar post meta.

Developed in https://github.com/WordPress/wordpress-develop/pull/10560

Follow-up to [56714].

Props LAPSrj, manhphucofficial, westonruter.
See #20299.
Fixes #64314.

@westonruter commented on PR #10560:


6 weeks ago
#13

Committed in r61372 (48b2b65)

Note: See TracTickets for help on using tickets.