[wp-trac] [WordPress Trac] #38583: Support for objects in schema validation and sanitization

[WordPress Trac]

#38583: Support for objects in schema validation and sanitization
---------------------------------------------------+-----------------------
 Reporter:  rachelbaker                            |       Owner:  rmccue
     Type:  enhancement                            |      Status:  reopened
 Priority:  high                                   |   Milestone:  4.9
Component:  REST API                               |     Version:  4.7
 Severity:  normal                                 |  Resolution:
 Keywords:  has-patch has-unit-tests dev-feedback  |     Focuses:
---------------------------------------------------+-----------------------
Changes (by mnelson4):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 I'm sorry if this isn't the right way to do it, but I reopened this
 ticket. [https://wordpress.slack.com/archives/C02RQC26G/p1507669092000076
 In slack] @TimothyBlynJacobs mentioned
 > Yeah, the commit for that needs massaging for unlisted properties. The
 current version in core is the direct opposite of the spec and has the
 potential to break other code

 I found [https://spacetelescope.github.io/understanding-json-
 schema/reference/object.html this article] that gives a fairly simple
 explanation of the spec. It says

 > The additionalProperties keyword is used to control the handling of
 extra stuff, that is, properties whose names are not listed in the
 properties keyword. By default any additional properties are allowed.

 > The additionalProperties keyword may be either a boolean or an object.
 If additionalProperties is a boolean and set to false, no additional
 properties will be allowed.

 ie, the default should be to ALLOW additional properties, not exclude
 them.

 @TimothyBlynJacobs's
 [https://core.trac.wordpress.org/attachment/ticket/38583/38583.1.diff
 original diff] had that behaviour (see lines 1071 and 1210) but @joehoyle
 decided to not include it saying

 > I think we shouldn't support additionalProperties for now, the point of
 the schema is to whitelist data and types into the REST API (and out), so
 I'm not sure I see a long term path for it; but again, let's get basic
 object support before putting a nail in that coffin.

 I appreciate @joehoyle is trying to not add anything unnecessary on this
 commit. But if `additionalProperties` is not supported now, and non-
 whitelised properties are removed, then IMO it does put a nail in the
 coffin: it goes against the JSON schema specification and breaks backward
 compatibility. And if we were to later decide we do want to follow the
 JSON schema specification (namely, allow additional properties by
 default), we'd have to make another breaking change in order to do that.

 So if we don't want to support the `additionProperties` argument yet, the
 default should at least be to default to ALLOW non-whitelisted properties.

 Thoughts?

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