[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 joehoyle):

 Ok so I think we are on the same page in terms of what the options are.

 > We also currently allow non listed properties for the top-level object.

 That's right, this was somewhat controversial and we chose to do it so it
 wasn't required to markup arguments / object properties. In this case we
 are talking about specifically registered fields. If I have code that
 does:


 {{{#!php
 <?php
 register_rest_route( ..., ..., [
     [
         'args' => [
              'an-object' => [
                  'type' => 'object',
                  'properties' => [
                      'value' => [ 'type' => 'string' ],
                   ],
               ],
          ],
      ],
 ] );
 }}}

 In my callback, I _should_ expect to be able to do:

 {{{#!php
 <?php
 function callback( WP_REST_Request $request ) {
     do_something_scary( $request['my-object'] );
 }
 }}}


 The point of this whole exercise of the schema is, practically speaking,
 mainly benefiting the back-end code. If the above actually means I can't
 rely on `$request['my-object']` not including lots of other data then I
 don't think it becomes all that useful. This is _much_ more labor
 intensive for nested objects, so I basically need to write a deep
 sanitizer myself then.

 Forgetting the JSON Schema spec for now (we have already diverged from
 JSON Schema), I feel like whitelisting is the right approach as we are
 being conservative with the data we let through sanitized properties.

 Being "insecure by default" unless a developer adds `additionalProperties
 => false` is, in my mind, asking for security issues. It's all well and
 good that JSON Schema has chosen to do that, but I don't think it's the
 right approach for the WP REST Api.

 I think the final decision will need to come from either @kadamwhite or
 @rmccue, and I'd say if we don't get a decision in the next 24 hours we
 need to revert r41758 and r41727, as @mnelson4 mentioned this is
 effectively making the decision to "whitelist by default" which we need to
 have made a conscious decision on.

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