Make WordPress Core

Opened 7 months ago

Last modified 5 weeks ago

#60618 new defect (bug)

REST API: Meta update fails if unrelated unchanged field is a multi-item array

Reported by: kadamwhite's profile kadamwhite Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch has-unit-tests has-testing-info
Focuses: rest-api Cc:

Description

In some plugins there may be code which repeatedly calls add_post_meta without registration, e.g. calling

<?php
add_post_meta( 1, 'example_meta_key', 'A' );
add_post_meta( 1, 'example_meta_key', 'A' );
add_post_meta( 1, 'example_meta_key', 'A' );

In the case where this bug was observed, the meta value was always retrieved as single, but the add_post_meta() call never utilized the unique flag. This results in multiple rows in the database for meta_key example_meta_key and meta_value A, for the post 1.

If the plugin later adds meta registration for this field, and both

  • registers the post meta field as 'single' => true
  • exposes the post meta field as 'show_in_rest' => true

then subsequent saves to this post through the REST API will fail.

The reason is that the REST API iterates through post meta that is passed back in the REST post, and checks whether the incoming value is the same as the stored value using this check in WP_REST_Meta_Fields::update_meta_value:

<?php
if ( is_array( $old_value ) && 1 === count( $old_value )
        && $this->is_meta_value_same_as_stored_value( $meta_key, $subtype, $old_value[0], $value )
) {
        return true;
}

In our case the $old_value will be an array, but the count will be 3, so we never opt in to our similar-data check.

This means that the incoming value will be passed through and will call update_meta, which will return false because update_meta returns false "on failure or if the value passed to the function is the same as the one that is already in the database." (emphasis added)

If a new value is passed for the existing meta, all rows will be updated as expected. It is only unchanged values which fail the comparison.

Summary: Our logic in our update meta function is problematic, and doesn't handle existing arrays of metadata properly when checking for an identical existing value.

Change History (5)

#1 @swissspidy
7 months ago

  • Version changed from trunk to 4.7

#2 @antonvlasenko
7 weeks ago

Reproduction Report

This report validates that the issue can be reproduced.

Environment

  • WordPress: 6.6
  • PHP: 7.4.33
  • Server: Apache/2.4.58 (Unix) PHP/7.4.33
  • Database: mysqli (Server: 5.7.42 / Client: mysqlnd 7.4.33)
  • Browser: Safari 17.5 (macOS)
  • Theme: Twenty Twenty-Four 1.0
  • MU-Plugins: None activated

Actual Results

  • ✅ When attempting to update a post meta value via the REST API for a meta key with multiple existing database entries and the REST meta field defined as single, the API returns a 500 HTTP error if the value already exists in the database.

Additional Notes

The issue is consistent with the reporter's description.

Supplemental Artifacts

https://cldup.com/ERoxepyXr2.png

#3 @antonvlasenko
5 weeks ago

  • Keywords needs-patch added

I'm working on a patch for this issue.

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


5 weeks ago
#4

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

#5 @antonvlasenko
5 weeks ago

  • Keywords has-testing-info added

The PR I've submitted addresses the issue of updating meta values with duplicate entries.

Current behavior

The current implementation of the update_metadata() function can lead to three possible outcomes:

# Invalid ID or data:

  • If the ID, type, or other data related to the metadata entity being updated is not valid, the function returns false.
  • This results in a rest_meta_database_error REST response code and a 500 HTTP status returned by WP_REST_Meta_Fields::update_meta_value() method.

# No rows affected:

  • If the ID and data are valid, but no rows in the database are updated or inserted (i.e., the number of affected rows is 0), the function still returns false.
  • This outcome also results in a rest_meta_database_error REST response code and a 500 HTTP status, even though no actual database error occurs.

# Successful update:

  • If the ID and data are valid and the number of affected rows in the database is greater than 0, the function results in a success response.

Issue with current implementation

The second outcome is problematic because it incorrectly signals a database error when no rows are affected, despite no actual error occurring. This leads to misleading error responses, which can be confusing and unhelpful for users and developers.

Proposed solution

To address this issue, my PR introduces a new parameter, $is_failure, to the update_metadata() function. This parameter allows the function to differentiate between a true database error and a situation where no rows are affected. With this change:

  • The function can accurately determine when a database error occurs.
  • The REST API can return more precise response codes and HTTP statuses.
  • The misleading signaling of errors in the second outcome is resolved.

Steps to test this PR

  1. Create a new post in WordPress and note the post ID ($post_id) for reference.
  2. Add three identical meta values to the newly created post:
    add_post_meta( $post_id, 'foo_meta_key', 'bar' );
    add_post_meta( $post_id, 'foo_meta_key', 'bar' );
    add_post_meta( $post_id, 'foo_meta_key', 'bar' );
    
  1. Register your custom post meta:
    register_post_meta(
        'YOUR_POST_TYPE',
        'foo_meta_key',
        array(
            'show_in_rest'  => true,
            'single'        => true,
            'type'          => 'string',
            'auth_callback' => function () {
                return current_user_can( 'edit_posts' );
            },
        )
    );
    
  1. Send a POST request to the /wp/v2/<your_post_type>/ . $post_id endpoint to update the meta value. Ensure the meta value used matches the value added initially (e.g., 'bar').

(very general) example that uses curl:

curl -X POST https://your-wordpress-site.com/wp-json/wp/v2/<your_post_type>/<post_id> \
    -H "Content-Type: application/json" \
    -H "Authorization: Bearer YOUR_ACCESS_TOKEN" \
    -d '{
          "meta": {
              "foo_meta_key": "bar"
          }
      }'
  1. Ensure that the request is successful by checking for a 200 response code.

*Note:* This test case is covered by the WP_Test_REST_Post_Meta_Fields::test_update_meta_should_not_fail_with_duplicate_values_for_single_registered_post_meta test, ensuring that updating meta values with duplicate entries does not result in errors.

Note: See TracTickets for help on using tickets.