REST API does not check nested required properties

[TimothyBlynJacobs]

WP_REST_Server checks if a request is valid using WP_REST_Request::has_valid_params. That function first checks if all required parameters are supplied. A parameter is required if it has the required property specified. The REST API only checks for the existence of the top-level properties. Nested properties are not checked.

For example, given the following schema:

{
  "type": "object",
  "properties": {
    "my_obj": {
      "type": "object",
      "required": true,
      "properties": {
        "my_prop": {
          "type": "string",
          "required": true
        }
      }
    },
    "my_arr": {
      "type": "array",
      "required": true,
      "items": {
        "type": "object",
        "properties": {
          "my_prop": {
            "type": "string",
            "required": true
          }
        }
      }
    }
  }
}

The following data is valid.

{
  "my_obj": {
    "other_prop": "hi"
  },
  "my_arr": [
    {
      "other_prop": "hi"
    }
  ]
}

We could fix this in either WP_REST_Request::has_valid_params or rest_validate_value_from_schema.

Doing it in the request object would have the benefits of consistency with top-level required properties. It would also allow checking all required properties before checking for validity and returning it in one WP_Error object. However, there could be a BC issue if a user has specified a schema using required for child parameters, but then provided a custom validate_callback that didn't actually check that the parameter is required.

If it is in rest_validate_value_from_schema we wouldn't have the BC issue. It'd also be easier to implement since that function already recurses.

We'd also want to make sure that if a parent property isn't required, that child properties aren't required if the parent value is missing entirely. For instance given the following schema, an empty request object should be valid, but {my_prop: {}} wouldn't be.

{
  "type": "object",
  "properties": {
    "my_obj": {
      "type": "object",
      "properties": {
        "my_prop": {
          "type": "string",
          "required": true
        }
      }
    }
  }
}

[circuitbit]

Just thought it would be important to mention that according to JSON-Schema spec it's also possible to have a required field as an array of strings containing the properties which are required for that object, so that would be, sort of, expected.

$schema = array(
   'type'       => 'object',
   'properties' => array(
      'first_name' => array( 'type' => 'string' ),
      'last_name'  => array( 'type' => 'string' ),
      'email'      => array( 'type' => 'string' ),
   ),
   'required'   => array( 'first_name', 'last_name', 'email' ),
);

[TimothyBlynJacobs]

@circuitbit this is a great point. In WordPress we use v4 of the JSON Schema except for required where we use the v3 syntax. This syntax is nice to work with for core because we do a lot of conditional adding/removing of properties as well as the addition of new properties through things like additional properties and metadata. The simpler boolean syntax allows the requiredness of the property to be entirely communicated through one array definition instead of having to modify the base schema.

That being said,. I do think supporting the array format would be quite nice, particularly for cases where you are documenting a single object property on a schema, as opposed to the schema for the entire resource. I think we could add support for that syntax in the patch for this feature.

[sorenbronsted]

I will work on this.

I have added the required schema check for version 3 and 4 in rest_validate_value_from_schema.

Trac ticket: https://core.trac.wordpress.org/ticket/48818

[prbot]

​TimothyBJacobs commented on ​PR #225:

@sorenbronsted thanks for the patch! Could you rebase this so we only have the commits for your changes?

A rebased version of the previous 48818 PR

Trac ticket: https://core.trac.wordpress.org/ticket/48818

Fixed according to TimothyBJacobs

Trac ticket: https://core.trac.wordpress.org/ticket/48818

Previously, the WP_REST_Request object validated that top-level properties were defined, but this did not extend to those object's required properties. This adds validation to rest_validate_value_from_schema() directly.

Both the v3 and v4 JSON Schema syntax for required properties is supported.

Props sorenbronsted.
Fixes #48818.

Trac ticket: https://core.trac.wordpress.org/ticket/48818

[TimothyBlynJacobs]

In 47809:

REST API: Check required properties are provided when validating an object.

Previously, the WP_REST_Request object validated that top-level properties were defined, but this did not extend to those object's required properties. This adds validation to rest_validate_value_from_schema() directly.

Both the v3 and v4 JSON Schema syntax for required properties is supported.

Props sorenbronsted.
Fixes #48818.

[TimothyBlynJacobs]

In 47811:

REST API: Add @since entries for rest_validate_value_from_schema().

See #49572, #48818, #44949, #50053, #48820, #49720, #42961, #44975, #43392, #38583.