WordPress.org

Make WordPress Core

Opened 12 months ago

Closed 7 months ago

#48819 closed defect (bug) (fixed)

Filter response by context misses certain schemas

Reported by: TimothyBlynJacobs Owned by: TimothyBlynJacobs
Milestone: 5.5 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch has-unit-tests early
Focuses: Cc:

Description

WP_REST_Controller:filter_response_by_context is used to remove properties from a response that require a different context than requested. ie it removes edit properties if the request doesn't have an edit context. However it misses a few cases.

  • Does not check array items.
  • Does not checked multi nested properties. It only checks the first child property level.
  • Does not handle additionalProperties.
  • Does not handle type being an array of types.

Attachments (2)

48819.diff (11.8 KB) - added by TimothyBlynJacobs 12 months ago.
48819.2.diff (12.3 KB) - added by TimothyBlynJacobs 10 months ago.

Download all attachments as: .zip

Change History (13)

#1 @TimothyBlynJacobs
12 months ago

  • Keywords has-patch has-unit-tests added

I did a first pass patch implementing the changes described above. I ended up extracting it to a standalone function. I think it is still helpful when not subclassing WP_REST_Controller. It also made it a bit easier to test.

This new function also uses recursion to ensure we traverse the whole schema. It seemed like it might be unsafe BC wise to implement recursion with the original method if any controllers are overwriting that method and calling the parent.

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


11 months ago

#3 @TimothyBlynJacobs
10 months ago

I've uploaded a revised version of the patch that does not traverse child schemas unless a context is set at every level. This means the performance impact of traversing the entire object will only occur if the author is utilizing that functionality.

That is, for the following schema.

<?php
array(
        '$schema'    => 'http://json-schema.org/draft-04/schema#',
        'type'       => 'object',
        'properties' => array(
                'parent' => array(
                        'type'       => 'object',
                        'context'    => array( 'view', 'edit' ),
                        'properties' => array(
                                'child' => array(
                                        'type'       => 'object',
                                        'properties' => array(
                                                'grand'  => array(
                                                        'type'    => 'string',
                                                        'context' => array( 'view', 'edit' ),
                                                ),
                                                'hidden' => array(
                                                        'type'    => 'string',
                                                        'context' => array( 'edit' ),
                                                ),
                                        ),
                                ),
                        ),
                ),
        ),
)

and the following data

<?php
array(
        'parent' => array(
                'child' => array(
                        'grand'  => 'hi',
                        'hidden' => 'there',
                ),
        ),
)

In the first patch, the hidden field would be excluded, now it isn't. Notice that no context is specified for the child schema.

#4 @TimothyBlynJacobs
10 months ago

  • Keywords early added
  • Milestone changed from 5.4 to 5.5

Punting to 5.5. I think this is too large a change to land at this point in the cycle.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 months ago

#6 @davidbaumwald
8 months ago

@TimothyBlynJacobs Is this still in work for early inclusion in 5.5?

#7 @TimothyBlynJacobs
8 months ago

@davidbaumwald yep. The work is done, really just needs another pair of eyes before landing. cc @kadamwhite.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 months ago

#11 @TimothyBlynJacobs
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 47758:

REST API: Support more JSON Schemas when filtering a response by context.

The array type, multi-types, and the additional properties keyword are now supported. Additionally, the filter recurses to an infinite depth.

Fixes #48819.

Note: See TracTickets for help on using tickets.