Make WordPress Core

Opened 4 weeks ago

Last modified 4 weeks ago

#65855 new defect (bug)

REST API: Taxonomy REST bases can overwrite existing post item properties despite conflict warning

Reported by: wteam Owned by:
Priority: normal Milestone: Awaiting Review
Component: REST API Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses: rest-api

Description

This is a follow-up to #48401.

#48401 added detection and a _doing_it_wrong() warning when a taxonomy REST base conflicts with an existing property of WP_REST_Posts_Controller. However, after detecting the conflict, the controller continues processing the taxonomy.

As a result, the conflicting taxonomy can still overwrite an existing post property in the item schema and response, and can also participate in item-level term assignment and REST action links.

Steps to reproduce

Register a taxonomy whose REST base conflicts with an existing post item property:

register_taxonomy(
'type',
'post',
array(
'show_in_rest' => true,
'show_ui'      => true,
)
);

Because no explicit rest_base is specified, the taxonomy REST base is also type.

The posts REST resource already contains a built-in type property representing the post type. For a normal post this should be:

{
"type": "post"
}

The schema for that property is a read-only string.

When WP_REST_Posts_Controller::get_item_schema() processes the taxonomy, it detects that type already exists and emits the existing _doing_it_wrong() warning. Processing nevertheless continues, and the taxonomy schema subsequently replaces the existing type property.

The same issue occurs while preparing an item response: the controller first assigns the post type to type, but taxonomy processing can subsequently replace that value with taxonomy term IDs.

Actual behavior

The conflict produces a developer warning, but the conflicting taxonomy can still be exposed as if it were a valid post item REST property.

For example, the schema can contain:

schema.properties.type.type === "array"

instead of:

schema.properties.type.type === "string"

and the response can contain:

post.type === []

instead of:

post.type === "post"

The conflicting taxonomy may also participate in:

  • item-level term assignment;
  • term assignment permission checks;
  • schema links; and
  • wp:action-assign-* / wp:action-create-* links.

This means the warning added in #48401 detects the collision, but does not prevent the collision from affecting the REST item resource.

Expected behavior

When a taxonomy REST base conflicts with an existing post item property:

  • The existing REST property should remain unchanged.
  • The conflicting taxonomy should not be exposed as an item property for that controller.
  • The taxonomy should not expose assignment/create action links for the conflicting item property.
  • The conflicting field should not participate in REST item term writes.
  • The existing _doing_it_wrong() warning should remain so developers are informed that they need to register a non-conflicting REST base.

The taxonomy itself should remain registered and otherwise usable.

Developers can resolve the REST collision by specifying a safe REST base, for example:

register_taxonomy(
'type',
'post',
array(
'show_in_rest' => true,
'rest_base'    => 'type_terms',
)
);

In that case the REST representation can safely contain both properties:

{
"type": "post",
"type_terms": []
}

Proposed fix

Centralize the set of taxonomies that can safely be exposed as properties on the REST item resource.

While the item schema is being built, compare each taxonomy REST base with the existing schema properties.

If a conflict exists:

  • Keep the existing _doing_it_wrong() warning.
  • Exclude the conflicting taxonomy from the set of taxonomies exposed on the item resource.
  • Reuse that conflict-free taxonomy set consistently for item schema generation, responses, term writes, assignment permission checks, action links, and schema links.

The conflict-free list should be initialized from the existing schema properties rather than lazily calling get_item_schema() from the helper. This is important for subclasses such as WP_REST_Menu_Items_Controller, where a lazy schema lookup can introduce recursion through the schema/link call path.

Collection-level taxonomy query behavior is intentionally unchanged.

Regression testing

The issue is reproducible on WordPress development trunk.

Regression coverage verifies that a conflicting taxonomy does not overwrite:

  • the existing type schema property; and
  • the existing type response property.

Before the fix, the regression tests fail with:

Expected: string
Actual: array

and:

Expected: post
Actual: array()

With the proposed fix applied:

npm run test:php -- --filter taxonomy_name_conflicts

passes with:

OK (2 tests, 5 assertions)

The complete WP_Test_REST_Posts_Controller test suite also passes.

The complete Tests_REST_WpRestMenuItemsController test suite also passes.

git diff --check reports no whitespace errors.

Follow-up to #48401.

That ticket added conflict detection and the _doing_it_wrong() warning. This ticket addresses the remaining behavior where the conflicting taxonomy continues to participate in and overwrite parts of the REST item resource after that warning has been issued.

Change History (2)

This ticket was mentioned in PR #13013 on WordPress/wordpress-develop by csergozoltan.


4 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Trac ticket: https://core.trac.wordpress.org/ticket/65855

This PR prevents taxonomies with conflicting REST bases from overwriting existing post item properties.

The existing _doing_it_wrong() warning added for #48401 is preserved, but conflicting taxonomies are excluded from the item-level REST taxonomy set so they no longer participate in:

  • item schema properties;
  • prepared item responses;
  • term assignment;
  • term assignment permission checks;
  • REST action links; and
  • schema links.

The conflict-free taxonomy list is initialized while building the item schema and reused by the relevant item-level paths. This avoids lazily calling get_item_schema() from the helper, which could introduce recursion in subclasses such as WP_REST_Menu_Items_Controller.

Collection-level taxonomy query behavior is intentionally unchanged.

Tests added for:

  • preserving the existing type schema property when a taxonomy name conflicts;
  • preserving the existing type response property;
  • the WP_REST_Menu_Items_Controller subclass schema path.

Testing:

  • npm run test:php -- --group 65855
    • OK (3 tests, 7 assertions)
  • npm run test:php -- --filter WP_Test_REST_Posts_Controller
    • OK (283 tests, 2637 assertions)
  • npm run test:php -- --filter Tests_REST_WpRestMenuItemsController
    • OK (41 tests, 441 assertions)
  • PHPCS passes for all four modified files.
  • git diff --check reports no whitespace errors.

#2 @wteam
4 weeks ago

GitHub pull request for review:
https://github.com/WordPress/wordpress-develop/pull/13013

The PR includes the Core fix and regression coverage for the schema/response collision, plus the WP_REST_Menu_Items_Controller subclass regression test.

Note: See TracTickets for help on using tickets.