[wp-trac] [WordPress Trac] #42069: Saving metadata fails (randomly) if equal value already exists

[WordPress Trac]

#42069: Saving metadata fails (randomly) if equal value already exists
-------------------------------------+-----------------------
 Reporter:  JVel                     |       Owner:  (none)
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  5.0
Component:  REST API                 |     Version:  4.8.2
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:  rest-api
-------------------------------------+-----------------------

Comment (by dcavins):

 Hi @MattGeri-

 Thanks for your interest in this problem. I took a minute to test your
 patch, and meta of the type `boolean` or `integer` is still failing.
 Example meta definitions:

 {{{#!php
 // Boolean
 register_meta( 'post', 'featured_item', array(
         'sanitize_callback' => 'absint',
         'type' => 'boolean',
         'description' => 'test',
         'single' => true,
         'show_in_rest' => true,
 ) );

 // Integer
 register_meta( 'post', 'post_order', array(
         'sanitize_callback' => 'absint',
         'type' => 'integer',
         'description' => 'test',
         'single' => true,
         'show_in_rest' => true,
 ) );
 }}}

 The way-down-deep issue for this case is that if you pass in an integer,
 like `1`, and the strictly equal check doesn't catch the equivalency, it
 will also pass the check in `update_metadata()` (so at least it won't
 return `false` at that point) and be passed to `$wpdb->update()`.
 `$wpdb->update()` relies on `$wpdb->query()` for the actual insert, and
 the returned value is the number of rows affected, which is `0`. This
 causes `update_metadata()` to return `false` at this point:
 https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-
 includes/meta.php#L251

 My patch catches this case because both the old value and new value are
 cast to be the same type by the sanitization functions. Now that I know
 where the integer/boolean case is failing, I see that there are more ways
 to achieve this result, like relaxing the strictness of the check in
 `WP_REST_Meta_Fields::update_meta_value()`:
 {{{#!php
 if ( sanitize_meta( $meta_key, wp_unslash( $meta_value ), $meta_type ) ==
 $old_value[0] ) {
         return true;
 }
 }}}

 Alternatively, we could cast the incoming value as a string, because we
 know all meta values retrieved by `get_metadata()` are strings:
 {{{#!php
 if ( (string) sanitize_meta( $meta_key, wp_unslash( $meta_value ),
 $meta_type ) === $old_value[0] ) {
         return true;
 }
 }}}

 Thanks again for your thoughts on this problem.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/42069#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform