[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:  major                       |  Resolution:
 Keywords:  has-unit-tests needs-patch  |     Focuses:
----------------------------------------+-----------------------

Comment (by mnelson4):

 @joehoyle

 >We are only able to accept the data we do because of type information we
 get via the registered schema.

 I think this is the crux of the issue: as I understand it, JSON schema
 places constraints to define what JSON is valid, not allowances on what's
 valid. E.g., the simplest JSON schema `{}` allows ANY JSON
 (https://spacetelescope.github.io/understanding-json-schema/basics.html),
 and then all additions to the schema narrow the range of what JSON is
 considered valid.

 So if a registered schema is empty, eg `{}`, it should accept ANY JSON;
 whereas you'd like it to accept NONE.

 Basically, JSON schema has more of a "blacklist" approach (everything's
 ok, unless the schema says it isn't), but you're wanting more of a
 "whitelist" approach (nothing is ok unless the schema says it is).

 A "whitelist" approach is good for security and having well-defined data.
 The difficulty is that's not JSON schema. So we have two ideals at ends
 here: stick to the JSON spec vs whitelist data instead of blacklisting it.

 Does that sound accurate?

 >  I don't see a case where we _would_ wholesale accept objects with
 properties of unknown data types and insert them into the database.

 I think @westonruter's link to using the WP API for the customizer is an
 application, isn't it? https://github.com/WP-API/wp-api-customize-
 endpoints/pull/10#pullrequestreview-43035491

 Also, elsewhere in our plugin we have an endpoint for changelog entries
 related to data in our custom tables, where we can store log entry items
 in a PHP array. E.g.,
 https://github.com/eventespresso/event-espresso-core/blob/master/docs/C
 --REST-API/ee4-rest-api-writing-data.md#serialized-data is a bit of
 documentation and http://dev2.eventespresso.com/wp-
 json/ee/v4.8.36/change_logs?_method=options has the route data, look for
 "LOG_message".

 So it seems to me like there is application for accepting an object with
 undefined properties.

 > we could just set additionalProperties: false to be spec-compliant.

 This might end up being the best option.

 > I'm not 100% sure if you are just wanting an object to be validated by
 the schema, and respect additionalProperties, or are you _also_ wanting to
 use register_meta / register_rest_field et al to actually support that
 from an API client, whereby the server should accept the additional data,
 store it, and return it on fetching that object?

 In our plugin we aren't using register_rest_field- we're just using custom
 endpoints for retrieving data from custom database tables.

 So I think I was originally just wanting the former, however I kinda think
 the latter would be good too. It would be nice if `register_rest_field`
 and `register_meta` just stuck to pure JSON schema (we wouldn't need to
 document any execptions etc). But again I understand it's probably safer
 to only accept data using a whitelist approach.

 @TimothyBlynJacobs

 > If we only want whitelisted properties to be in settings and meta when
 an object type is passed, we should set additionalProperties to false as a
 default argument and merge it with the provided schema.

 I think this approach has the best of both worlds: we're sticking to the
 spec, but settings and meta APIs can have more of a whitelist approach.

 The main downside to fully respecting JSON schema's `additionalProperties`
 is that anywhere we define the schema for an object, and we don't accept
 any other properties other than those listed, we now need to add
 `additionalProperties:false`. And I think there's a lot of those, right?

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