Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#48264 closed defect (bug) (fixed)

Item type of 'integer' fails to save unchanged values with 'array' type in register_meta()

Reported by: augustuswm's profile augustuswm Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.3
Component: Options, Meta APIs Keywords: has-patch has-unit-tests
Focuses: rest-api Cc:

Description

We have something like the following in our 5.3 based test environment:

<?php
register_meta('post', 'items',
  [
    'single' => true,
    'type' => 'array',
    'show_in_rest' => [
      'schema' => [
        'type' => 'array',
        'items' => [
          'type' => 'integer'
        ]
      ]
    ]
  ]
);

This is mirrored by a Gutenberg block with the attributes:

items: {
  type: 'array',
  source: 'meta',
  meta: 'items'
},

We are running into an issue where saving that meta data when no changes have occured results in a 500 (Updating failed. Error message: Could not update the meta value of items in database.)

It looks like it might be related to the early return check in class-wp-rest-meta-fields.php@update_meta_value

For the early return check the $old_value is fetched from the db which will always be an array of strings due to meta storage, the new value coming in though is an array of integers.
This causes the early return check to not pass and send the data along to update_metadata.

The update_metadata call though passes in wp_slash($value) which casts all of the array elements to strings. So when update_metadata performs its check against the old value in the db it sees two string arrays. These values being equal it returns false and the REST API identifies it as a failure.


From class-wp-rest-meta-fields.php@update_meta_value with incoming $value of [1, 2, 3] and a stored db value of ["1", "2", "3"]

<?php
$old_value = get_metadata( $meta_type, $object_id, $meta_key ); // [["1", "2", "3"]]
...
$sanitized = sanitize_meta( $meta_key, $value, $meta_type, $subtype ); // [1, 2, 3]
...
if ( $sanitized === $old_value[0] ) { // Check fails
  return true
}
...
if ( ! update_metadata( $meta_type, $object_id, wp_slash( $meta_key ), wp_slash( $value ) ) ) { // wp_slash([1, 2, 3]) = ["1", "2", "3"]
  ...
}

Attachments (3)

48264.diff (6.2 KB) - added by TimothyBlynJacobs 5 years ago.
48264.2.diff (9.6 KB) - added by TimothyBlynJacobs 5 years ago.
48264.3.diff (10.9 KB) - added by TimothyBlynJacobs 5 years ago.

Download all attachments as: .zip

Change History (6)

#1 @TimothyBlynJacobs
5 years ago

  • Component changed from General to Options, Meta APIs
  • Focuses rest-api added
  • Keywords has-patch has-unit-tests added
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to TimothyBlynJacobs
  • Status changed from new to assigned

Thanks for reporting this @augustuswm!

The intention was to be storing the data in the DB as a serialized array where the primitive types were respected. I mistakenly thought that wp_slash only applied slashes to string values, but as you pointed out it casts everything to strings.

I think the change here is to only apply slashing to the string values. If I call update_post_meta( $post_id, $meta_key, [ 1, 2, 3 ] ); directly, I get back an array of ints. I think this should behave the same in the REST API.

I've attached a patch that makes those changes. Could you verify the patch fixes your issue @augustuswm?

CC: @kadamwhite

#2 @augustuswm
5 years ago

Thanks for looking into this! Just applied them to our test env and saving looks to be working.

Here is the db storage before and after.

Before:

| 4200 | 536 | writers | a:3:{i:0;s:3:"297";i:1;s:3:"299";i:2;s:3:"301";} |

After:

| 4200 | 536 | writers | a:3:{i:0;i:297;i:1;i:301;i:2;i:299;} |

Also did not mention it in the initial report, but we really appreciate the work going into adding this functionality. It is extremely useful.

#3 @kadamwhite
5 years ago

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

In 46454:

REST API: Do not addslash when receiving meta arrays of non-string values.

Slashing non-string data caused PUT requests containing unmodified meta arrays of integers to fail the check against the existing stored meta value, causing a 500 when posting an unmodified response body back to the server.

Props TimothyBlynJacobs, augustuswm.
Fixes #48264.

Note: See TracTickets for help on using tickets.