Make WordPress Core

Opened 19 hours ago

Last modified 8 hours ago

#65905 reviewing defect (bug)

`register_rest_route()` ignores comma-separated methods when `methods` is an array

Reported by: moonmeister Owned by: westonruter
Priority: normal Milestone: 7.2
Component: REST API Version: 4.4
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

WP_REST_Server::register_route() accepts methods as either a string or an array, but only the
string form is split on commas. Any array element containing one becomes a single unmatchable
method key, so array( READABLE, EDITABLE ) silently registers a route where POST, PUT and
PATCH return 404 — with no notice at registration time.

From the endpoint normalization loop in src/wp-includes/rest-api/class-wp-rest-server.php:

// Allow comma-separated HTTP methods.
if ( is_string( $handler['methods'] ) ) {
  $methods = explode( ',', $handler['methods'] );
} elseif ( is_array( $handler['methods'] ) ) {
  $methods = $handler['methods'];
} else {
  $methods = array();
}

$handler['methods'] = array();

foreach ( $methods as $method ) {
  $method = strtoupper( trim( $method ) );
  $handler['methods'][ $method ] = true;
}

Both forms are supported by register_route(), and core uses the array form itself in
WP_REST_Block_Renderer_Controller. They are not equivalent, which is what makes this a bug
rather than a limitation.

Full reproduction:

register_rest_route( 'my/v1', '/thing', array(
  'methods' => array( WP_REST_Server::READABLE, WP_REST_Server::EDITABLE ),
  'callback' => 'my_callback',
  'permission_callback' => '\_\_return_true',
) );

// GET /wp-json/my/v1/thing → 200
// POST /wp-json/my/v1/thing → 404

Confirmed on unmodified trunk with a three-case unit test: array of single-method constants
passes, string form passes, array containing EDITABLE fails on POST/PUT/PATCH and asserts the registered keys as exactly array( 'GET', 'POST, PUT, PATCH' ).

Impact

Core is not currently broken. Its only array-form registration, WP_REST_Block_Renderer_Controller, uses READABLE and CREATABLE — both single-method.

I searched the plugin directory rather than guess at the scale:

Pattern Lines Plugins Active installs
register_rest_route( 69,835 8,748 178.9M
methods given as an array 7,024 921 53.3M
WP_REST_Server::EDITABLE anywhere 6,986 1,367 76.5M
array form + a multi-method constant 4 3 2,000

So the array form is in wide use, but it is rarely combined with the constants — only 87 of those 7,024 lines reference a WP_REST_Server:: constant at all. The rest pass plain strings like array( 'POST' ), which are unaffected. That is why a bug this old has stayed quiet.

This is a small bug, and I'd rather lead with that than oversell it. Three plugins are affected, totalling 2,000 installs. What makes it worth fixing is not the count but the shape of the failure. All four occurrences are array( CREATABLE, EDITABLE ) in one order or the other, which registers the keys array( 'POST, PUT, PATCH', 'POST' ) — so POST works, because CREATABLE supplies it, while PUT and PATCH 404. The route half-functions. One of the three carries the inline comment // PUT/POST. next to a PUT that was never registered, which is the whole problem in one line: the author had no way to find out.

The rest of the case:

  • The two forms of a supported argument silently disagree. array( READABLE, EDITABLE ) and READABLE . ', ' . EDITABLE should register the same route, and don't.
  • Nothing surfaces it — no notice at registration, no warning, just a 404 against a route that reads as correctly declared.
  • EDITABLE is intentionally multi-method and staying that way (#43744, wontfix), so this does not age out. All four real-world hits are people composing the constants — the thing the array form invites and the string form makes awkward.
  • It forecloses any future multi-method constant. Adding one would break every array-form route using it on the day it landed — at least 27 plugins and 1.2M installs for READABLE, on the same-line count alone.

Why this is being filed now

I found this while measuring what it would take to support the QUERY method (RFC 10008) in the REST API — #65616. The two are independent, no QUERY decision rests on this, and it should land or not land on its own.

The overlap is worth knowing, though, because it turns the last bullet above from hypothetical into measured. #65616 discusses adding QUERY to the method constants, and one option on the table gives READABLE a second method. Doing that in a local build turns this latent bug into 14 failures in core's own test suite — every REST_Block_Renderer_Controller_Test case, all reporting 404 is identical to 200, because the block renderer registers array( READABLE, CREATABLE ) and READABLE stops matching the moment it contains a comma.

So core is one multi-method constant away from breaking its own routes, and this is the fix that removes that constraint. Landing it is worth doing regardless of what happens to QUERY; leaving it in place quietly forecloses an option in a separate discussion.

Suggested fix

Split the array branch as well:

} elseif ( is_array( $handler['methods'] ) ) {
$methods = array();

    foreach ( $handler['methods'] as $method ) {
        $methods = array_merge( $methods, explode( ',', $method ) );
    }

}

This is safe to do unconditionally: a comma is a delimiter in the RFC 9110 token grammar, so no valid HTTP method name can contain one. Splitting can only turn a permanently-dead method key into working ones.

Happy to put up a PR with the failing test above once there's agreement on the approach.

Change History (3)

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


18 hours ago
#1

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

Registering a REST route with 'methods' => array( WP_REST_Server::READABLE, WP_REST_Server::EDITABLE ) returns 404 for POST, PUT and PATCH.

WP_REST_Server::get_routes() splits the methods argument on commas only when it is a string. An array element containing one, such as the multi-method EDITABLE constant, becomes a single method key that no request can match, with no notice at registration. The patch splits each array element too, so both forms register the same route.

The Allow header is unchanged — it reassembles the key by imploding on commas, which is why the mismatch stayed invisible.

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

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the failure against trunk, measuring the array and string forms side by side, and drafting the regression test. All changes were reviewed and validated by me.

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


17 hours ago
#2

## Problem

WP_REST_Server::register_route() accepts methods as a string or an array, but only the string form is split on commas. Any array element containing a comma becomes a single unmatchable method key.

register_rest_route( 'ns/v1', '/thing', array(
        'methods'             => array( WP_REST_Server::READABLE, WP_REST_Server::EDITABLE ),
        'callback'            => '__return_true',
        'permission_callback' => '__return_true',
) );

registers the keys 'GET' and 'POST, PUT, PATCH'. GET works; POST, PUT and PATCH all 404. The same route written as the string 'GET, POST, PUT, PATCH' works. Nothing warns at registration time.

Dates to the array support added in 4.4.

## Fix

Split the array branch as well, merging each element's comma-separated parts.

A comma is a delimiter in the RFC 9110 token grammar, so no valid HTTP method name can contain one. Splitting can therefore only turn a permanently-dead method key into working ones — there is no case where the current behavior is the desired one.

The loop is deliberate rather than explode( ',', implode( ',', $handler['methods'] ) ): imploding an empty array yields '', which would register an empty-string method key. test_route_method_empty_array() guards that.

## Tests

Four tests added to the existing test_route_method_* group, which covered an array of methods and a comma-separated string but not their intersection:

Test On trunk
test_route_method_array_with_comma_separated_values fails
test_route_method_array_with_multi_method_constant fails
test_route_method_array_with_multi_method_constant_dispatches fails (404 instead of 200)
test_route_method_empty_array passes — regression guard

--group restapi: 3554 tests / 16246 assertions, no new failures against a 3550 / 16239 baseline (identical pre-existing error and warning profile).

## Impact

A plugin-directory regex sweep found 4 occurrences across 3 plugins (~2,000 installs) — all array( CREATABLE, EDITABLE ), where POST works and the intended PUT/PATCH silently 404. The array form itself is common (921 plugins), but only 87 of 7,024 array-form methods lines name a WP_REST_Server:: constant at all, which is why the bug has stayed quiet.

The larger reason to fix it is forward-looking. This was found while measuring what would happen if a method constant gained a second method — as it would if QUERY (RFC 10008) were added to READABLE or ALLMETHODS. Every array-form registration naming that constant would break at once, with no error. Landing this removes that constraint before the question comes up.

#3 @westonruter
8 hours ago

  • Milestone Awaiting Review7.2
  • Owner set to westonruter
  • Status newreviewing
Note: See TracTickets for help on using tickets.