Support for arrays in schema validation and sanitization

[joehoyle]

Leading into ​https://github.com/WP-API/WP-API/issues/2859, the main thing to solve here is sanitization / validation support for arrays. If you specify type => array in endpoint args, then an additional items => array( type => string ) is required to specify the schema for the items in the array.

I've implemented a patch to support arrays (and easily objects in the future) by abstracting our schema validation / sanitization a little to supper them being called recursively, which means array validation is quite simple.

Also, I added a good amount of tests for exactly what happens for each type / format in validation.

There was a few leaks that I uncovered when doing this, such as Settings allowing null values, and some places where we were specify type => array in endpoint args, but were not defined the items property. This means we'll get a little better validation off the bat. A couple of other fixes came along with this such as numeric => number typo for min / max support, sanitization for type => number which we had previously missed.

I'd like to get some eyes on this from @rachebaker and @rmccue, this was a little simpler than I had though, so I think we might be able to get this in 4.7.

Once this is in, it will just be a case of allowing type => array to the settings and meta endpoints. Also, this operates at the endpiont args level, so type => array is also supported in collection params.

[ChopinBach]

This should go into 4.7, minus the var_dump() lines. This is a big step forward for the way the validate/sanitize callbacks are being implemented currently.

[pento]

Oh, this is nice.

I saw there was a bit of discussion around adding CSV and object support here, I'm inclined to bring in this patch as is, then we can have new tickets for adding to it.

38531.2.diff​ just does a little bit of tidying. I like the look of this change, but I'd like a second opinion from @rachelbaker or @rmccue first. :-)

[rachelbaker]

Removed var_dump() uses and fixed typo in the header of tests/phpunit/tests/rest-api/rest-schema-validation.php

[rachelbaker]

@pento Looks good to me. I did a little more clean-up in 38531.3.diff​ that removes the var_dump() debugging, fixes a typo, and some docblock spacing.

[rachelbaker]

I created #38583 for handling objects in our schema.

[pento]

In 39046:

REST API: Add support for arrays in schema validation and sanitization.

By allowing more fine-grained validation and sanitisation of endpoint args, we can ensure the correct data is being passed to endpoints.

This can easily be extended to support new data types, such as CSV fields or objects.

Props joehoyle, rachelbaker, pento.
Fixes #38531.

[joehoyle]

In the latest patch I think from @rachelbaker we are skipping sanitization if nothing is specified in the items property:

if ( empty( $args['items'] ) ) { 
    return (array) $value; 
}

This means that we are "passing thru by default" which I think is a bad idea for a sanitizing function. IMO if the items is not present we should return an empty, and maybe fire a doing_it_wrong. With this, it would be easy (and some places in the rest api already do this) to specify type => array, and therefor get no sanitization.

Looking for thoughts from @rachelbaker :)

[TimothyBlynJacobs]

Doing it this way precludes the function from ever supporting things like $ref which would be immensely useful. Its not too difficult to add, but it would require the full schema to work off of.

[joehoyle]

Ping @rachelbaker, I wanted to know your thoughts on the "default to no sanitization", also if there's anything else that needs doing on this ticket?

[tharsheblows]

If you want to use rest_validate_value_from_schema for meta values, empty values need to be ok as they're included in the response.

Attached is that change with meta using rest_validate_value_from_schema and rest_sanitize_value_from_schema and allowing arrays, including multiple meta keys with type='array'. The allowed types are whitelisted in WP_REST_Meta_Fields so it fixes this too ​https://github.com/WP-API/WP-API/issues/2798

[tharsheblows]

add in support for meta values

[tharsheblows]

Unit tests showing unknown types falling through validation and sanitization. nb: I'm not exactly sure what the sanitization return value should be and have not included 'type' => 'object' because I'm not sure how that should be handled.

Currently validating / sanitizing php associative arrays work with a schema 'type' => 'array' even though they're json objects. Not sure if that's an issue or will be confusing but thought I'd mention.

[tharsheblows]

unit tests for assoc arrays, unknown types and no types in schemas

[joehoyle]

Currently validating / sanitizing php associative arrays work with a schema 'type' => 'array' even though they're json objects. Not sure if that's an issue or will be confusing but thought I'd mention.

Ahh, I think we should error on this case. I'll add a patch. If you send a JSON object for an array, that is. I think we have some kind of wp_is_numeric_array() function somewhere.

[tharsheblows]

Attached are unit tests with post meta tests included - they work slightly differently so need separate tests.

In test_type_array_is_associative the schema isn't correct for what the value is but it's an error people might make and sanitization and validation aren't working correctly with that incorrect type.

Use this patch for unit tests instead of the previous one as I corrected some other bits too.

[tharsheblows]

unit tests with post meta and fewer typos

[rmccue]

Replying to joehoyle:

This means that we are "passing thru by default" which I think is a bad idea for a sanitizing function. IMO if the items is not present we should return an empty, and maybe fire a doing_it_wrong. With this, it would be easy (and some places in the rest api already do this) to specify type => array, and therefor get no sanitization.

Per ​the JSON Schema spec:

Successful validation of an array instance with regards to [additionalItems and items] is determined as follows:

• if "items" is not present, or its value is an object, validation of the instance always succeeds, regardless of the value of "additionalItems";
• [...]
• if the value of "additionalItems" is boolean value false and the value of "items" is an array, the instance is valid if its size is less than, or equal to, the size of "items".

If either keyword is absent, it may be considered present with an empty schema.

Hence, I believe the default behaviour here is correct. It would only fail if items was present, an array, and also additionalItems was set to false.

[rmccue]

This needs shepherding, and final changes here. It appears there are two things left to handle in this ticket:

1. Using the new validation for meta as well.
2. array allows JSON objects currently.

Correct me if I'm wrong and there's something I'm missing here.

[jnylen0]

I believe the default behaviour here is correct.

In that case, we should probably add an item type to the few array args/properties that don't have one:

array arg without item type:
  /wp/v2/posts
  slug
  {"required":false,"description":"Limit result set to posts with one or more specific slugs.","type":"array"}

array arg without item type:
  /wp/v2/pages
  slug
  {"required":false,"description":"Limit result set to posts with one or more specific slugs.","type":"array"}

array arg without item type:
  /wp/v2/media
  slug
  {"required":false,"description":"Limit result set to posts with one or more specific slugs.","type":"array"}

array property in schema without item type:
  /wp/v2/taxonomies
  capabilities
  {"description":"All capabilities used by the resource.","type":"array","context":["edit"],"readonly":true}

array property in schema without item type:
  /wp/v2/taxonomies/(?P<taxonomy>[\w-]+)
  capabilities
  {"description":"All capabilities used by the resource.","type":"array","context":["edit"],"readonly":true}

Source: ​https://gist.github.com/nylen/9021f6d19157a023a0a8a607a3adb28a

[joehoyle]

Added a patch to address the following:

• rest_validate_value_from_schema should fail if passed assoc. arrays, as this means it was an object in JSON.
• rest_sanitize_value_from_schema will now convert arrays to numeric arrays, to not allow unwanted data to slip in via the keys.
• rest_sanitize_value_from_schema will now cast data to string for type => string.
• Meta fields are now properly excluded if they are not registered with a valid type.
• Meta fields' type is now a fall back to the registered meta show_in_rest's type. Seems this was an oversight.
• Updated tests for meta to include the type in registration.
• Schemas with unknown types will pass through rest_validate_value_from_schema and rest_sanitize_value_from_schema unchanged. As this is a developer decision, we ideally want to support using different types, whereby developers implement their own sanitization for those. Because we use rest_parse_request_arg in the case of _any_ type being supplied, I think this makes the most sense.

[joehoyle]

Updated patch with:

• Unit tests for validation and sanitization of post meta fields.
• WP_REST_Meta_Fields will now validate and sanitize meta values from the schema.
• WP_REST_Meta_Fields::get_value will now return an array rather than object. This is consistent with other "objects" in API responses, such as content on Posts.

[tharsheblows]

fix custom schema update, unit tests for that

[tharsheblows]

Custom schema updates were not validating, latest patch fixes that and adds unit tests

[joehoyle]

In 39222:

REST API: Validate and Sanitize registered meta based off the schema.

With the addition of Array support in our schema validation functions, it's now possible to use these in the meta validation and sanitization steps. Also, this increases the test coverage of using registered via meta the API significantly.

Fixes #38531.
Props rachelbaker, tharsheblows.