Make WordPress Core

Opened 3 weeks ago

Closed 2 weeks ago

#65644 closed defect (bug) (fixed)

Abilities REST controllers trigger _doing_it_wrong() on 404 for unknown ability/category names

Reported by: dhrupo Owned by: gziolo
Priority: normal Milestone: 7.1
Component: Abilities API Version: 6.9
Severity: normal Keywords: has-patch has-unit-tests has-screenshots commit
Cc: Focuses:

Description (last modified by gziolo)

The Abilities REST controllers resolve an ability (or category) from the name/slug in the request URL and return a 404 when it does not exist:

$ability = wp_get_ability( $request['name'] );
if ( ! $ability ) {
    return new WP_Error( 'rest_ability_not_found', __( 'Ability not found.' ), array( 'status' => 404 ) );
}

wp_get_ability() / wp_get_ability_category() delegate to WP_Abilities_Registry::get_registered() / WP_Ability_Categories_Registry::get_registered(), which deliberately call _doing_it_wrong() for an unknown name to flag developer misuse. Because the name is client-controlled, a routine 404 for a non-existent ability fires _doing_it_wrong() on every such request — including via the run endpoint's permission_callback, so unauthenticated requests trigger it too.

Core's rest_api_default_filters() suppresses the trigger_error() output during REST requests (see #64260), but the doing_it_wrong_run action still fires, so the notice surfaces in Query Monitor, in doing_it_wrong_run listeners / logging plugins, and in the test suite — the not-found REST controller tests currently carry @expectedIncorrectUsage. Existence-probing a public endpoint should not emit a developer "doing it wrong" warning.

Affected call sites:

  • class-wp-rest-abilities-v1-run-controller.php (execute, permission check, input sanitizer)
  • class-wp-rest-abilities-v1-list-controller.php (get_item)
  • class-wp-rest-abilities-v1-categories-controller.php (get_item)

Steps to reproduce

  1. With a tool that surfaces doing_it_wrong_run (e.g. Query Monitor), send POST /wp-json/wp-abilities/v1/abilities/non/existent/run.
  2. The request returns the expected 404, but _doing_it_wrong() fires: WP_Abilities_Registry::get_registered was called incorrectly. Ability "non/existent" not found.

See the attached before/after screenshots.

Proposed fix

Guard each REST lookup with the non-warning existence helpers before calling the getter:

$ability = wp_has_ability( $request['name'] ) ? wp_get_ability( $request['name'] ) : null;

A missing ability then yields a clean 404 with no _doing_it_wrong(). The registry getters continue to warn on direct misuse (intentional and separately tested). The now-unnecessary @expectedIncorrectUsage annotations are removed from the affected not-found REST tests.

Attachments (2)

abilities-before.png (160.2 KB ) - added by dhrupo 3 weeks ago.
BEFORE the fix: POST to a non-existent ability returns the expected 404, but _doing_it_wrong() fires (WP_Abilities_Registry::get_registered — Ability "non/existent" not found), surfaced here as Query Monitor / the doing_it_wrong_run action would. See Trac #65644.
abilities-after.png (133.7 KB ) - added by dhrupo 3 weeks ago.
AFTER the fix: the same request to a non-existent ability returns a clean 404 with no _doing_it_wrong() — the doing_it_wrong_run action does not fire. See Trac #65644 / the PR.

Download all attachments as: .zip

Change History (5)

@dhrupo
3 weeks ago

BEFORE the fix: POST to a non-existent ability returns the expected 404, but _doing_it_wrong() fires (WP_Abilities_Registry::get_registered — Ability "non/existent" not found), surfaced here as Query Monitor / the doing_it_wrong_run action would. See Trac #65644.

@dhrupo
3 weeks ago

AFTER the fix: the same request to a non-existent ability returns a clean 404 with no _doing_it_wrong() — the doing_it_wrong_run action does not fire. See Trac #65644 / the PR.

This ticket was mentioned in PR #12555 on WordPress/wordpress-develop by @dhrupo.


3 weeks ago
#1

## Summary

The Abilities REST controllers resolve an ability (or category) from the name/slug in the request URL and return a 404 when it does not exist — calling wp_get_ability() / wp_get_ability_category() directly:

$ability = wp_get_ability( $request['name'] );
if ( ! $ability ) {
    return new WP_Error( 'rest_ability_not_found', __( 'Ability not found.' ), array( 'status' => 404 ) );
}

Those functions delegate to WP_Abilities_Registry::get_registered() / WP_Ability_Categories_Registry::get_registered(), which deliberately call _doing_it_wrong() for an unknown name to flag developer misuse. Because the name is client-controlled, a routine 404 for a non-existent ability fires _doing_it_wrong() on every such request — including via the run endpoint's permission_callback, so unauthenticated requests trigger it too.

Core's rest_api_default_filters() suppresses the trigger_error() output during REST requests (see #64260), but the doing_it_wrong_run action still fires, so the notice surfaces in Query Monitor, in doing_it_wrong_run listeners / logging plugins, and in the test suite — the not-found REST controller tests currently carried @expectedIncorrectUsage. Existence-probing a public endpoint should not emit a developer "doing it wrong" warning.

## Reproduction

POST /wp-json/wp-abilities/v1/abilities/non/existent/run returns the expected 404, but (surfaced via Query Monitor / the doing_it_wrong_run action) _doing_it_wrong() fires:

WP_Abilities_Registry::get_registered was called incorrectly. Ability "non/existent" not found.

See the before/after screenshots on the Trac ticket.

## Fix

Guard each REST lookup with the non-warning existence helper before calling the getter:

$ability = wp_has_ability( $request['name'] ) ? wp_get_ability( $request['name'] ) : null;

A missing ability then yields a clean 404 with no _doing_it_wrong(). The registry getters keep warning on direct misuse (that behavior is intentional and separately tested — unchanged here). Applied to all five lookup sites across the run, list, and categories controllers.

## Testing

Removed the now-unnecessary @expectedIncorrectUsage annotations from the four affected not-found REST tests, which now assert a clean 404.

  • Before the fix, those tests fail: Unexpected incorrect usage notice for WP_Abilities_Registry::get_registered.
  • After the fix, they pass.

Full suites green:

Abilities REST controllers: 67 tests
--group abilities-api: 361 tests, 962 assertions

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Locating the affected call sites, reproducing the notice, implementing the guards, and updating the tests. All changes were reviewed by me.

#2 @gziolo
2 weeks ago

  • Description modified (diff)
  • Keywords commit added
  • Milestone Awaiting Review7.1
  • Owner set to gziolo
  • Status newassigned

#3 @gziolo
2 weeks ago

  • Resolutionfixed
  • Status assignedclosed

In 62831:

Abilities API: Avoid _doing_it_wrong() on REST lookups of unknown abilities

The REST controllers resolve an ability or category from a client-controlled name. On a miss, wp_get_ability() / wp_get_ability_category() reach the registry getter, which fires _doing_it_wrong() to flag developer misuse. So a routine 404
emits a spurious notice in Query Monitor, doing_it_wrong_run listeners, and tests, including on the unauthenticated run endpoint.

Guard each of the five lookups with the existence helper (wp_has_ability() / wp_has_ability_category()) first, so a missing name returns a clean 404. Direct misuse of the getters still warns, as intended.

Props dhrupo.
Fixes #65644.

Note: See TracTickets for help on using tickets.