#48818 closed defect (bug) (fixed)
REST API does not check nested required properties
Reported by: | TimothyBlynJacobs | Owned by: | TimothyBlynJacobs |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | REST API | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
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 } } } } }
Change History (13)
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
5 years ago
#3
@
5 years ago
@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.
This ticket was mentioned in PR #225 on WordPress/wordpress-develop by sorenbronsted.
4 years ago
#6
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
TimothyBJacobs commented on PR #225:
4 years ago
#8
@sorenbronsted thanks for the patch! Could you rebase this so we only have the commits for your changes?
This ticket was mentioned in PR #234 on WordPress/wordpress-develop by sorenbronsted.
4 years ago
#9
A rebased version of the previous 48818 PR
Trac ticket: https://core.trac.wordpress.org/ticket/48818
This ticket was mentioned in PR #244 on WordPress/wordpress-develop by sorenbronsted.
4 years ago
#10
Fixed according to TimothyBJacobs
Trac ticket: https://core.trac.wordpress.org/ticket/48818
This ticket was mentioned in PR #262 on WordPress/wordpress-develop by TimothyBJacobs.
4 years ago
#11
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
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' ), );