WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 10 days ago

#42069 new defect (bug)

Saving metadata fails (randomly) if equal value already exists

Reported by: JVel Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8.2
Component: REST API Keywords: has-patch
Focuses: rest-api Cc:

Description

When using js rest api to save page.

Duplicate value test in wp-includes/rest-api/fields/class-wp-rest-meta-fields.php line 303 fails and causes error "Could not update meta value in database."

if ( $old_value[0] === $meta_value ) {

Problem seems to be that $old_value is not going through wp_slash but new $meta_value is.

I get it working by changing code to test against $value instead.

if ( $old_value[0] === $value ) {

Attachments (1)

42069.1.diff (3.9 KB) - added by dcavins 10 days ago.
Improve reliability of updating meta via REST API.

Download all attachments as: .zip

Change History (4)

#1 @pilou69
5 months ago

Having an issue with this too in 4.8.2.

It's not random, it always happens when the meta is not modified and has any characters that must be slashed.

We found 3 ways to work around this:

  1. Apply modification above, but it overrides the core :(
    if ( $old_value[0] === $value ) {
    
  1. Register meta fields as single => false. But that seems very ugly.
  1. Create custom endpoint:

create a class extending WP_REST_Meta_Fields, and override update_meta_value function with patch applied above

extend WP_REST_Posts_Controller (WP_REST_{$type}_Controller),)

override in you custom endpoint controller
$this->meta with your Class with the patch

@dcavins
10 days ago

Improve reliability of updating meta via REST API.

#2 @dcavins
10 days ago

  • Keywords has-patch added

I too ran into this problem today and was able to identify two cases where WP_REST_Meta_Fields::update_meta_value() was failing to interpret the values as identical, but update_metadata() was returning false (because the values were considered equal by that function), so the REST request was getting a 500 error back. These two cases are problematic:

  • Text containing characters that are slashed or converted to entities, like " or <.
  • Booleans, like a checkbox, that, when unchecked, pass the integer 0.

In the first case, the comparison in WP_REST_Meta_Fields::update_meta_value() tries to strict compare &lt; to <, and, in the second case, the string '0' (all saved meta is strings) is compared to the incoming sanitized value of (int) 0.

I've created a diff with tests showing these failures and a proposed patch to improve the comparison in WP_REST_Meta_Fields::update_meta_value() so that it catches more cases that update_metadata() or $wpdb->query() will consider equal.

If there are other cases that I can write tests for, please describe the situation in detail in your comments. I'd like to find all failures and make sure that the fix works for all of them.

Thanks for your comments!

#3 @dcavins
10 days ago

Also note that #42810 is related.

Note: See TracTickets for help on using tickets.