REST API: Add the rest_allowed_cors_methods filter and stop discarding a response's method list. by moonmeister · Pull Request #13151 · WordPress/wordpress-develop

moonmeister · 2026-08-18T23:02:56Z

Trac ticket: https://core.trac.wordpress.org/ticket/46992 - background, prior art and the caveat are there.

Approach discussed in person with @adamsilverstein, who closed the ticket in 2019 and has since agreed on option C.

Description refreshed by Claude to match the current head, 674e883.

Changes

1. Don't clobber a value set on the response. rest_send_cors_headers() runs on rest_pre_serve_request, after the response's own headers have been sent, and header() replaces by default - so the hardcoded list discarded anything set via rest_post_dispatch. A response value now seeds the method list instead, narrowed to the methods in the default list. Header names and method names are matched without regard to case.

$response_headers = ( $result instanceof WP_HTTP_Response ) ? $result->get_headers() : array();
$response_headers = array_change_key_case( $response_headers );

$allow_methods = array( 'OPTIONS', 'GET', 'POST', 'PUT', 'PATCH', 'DELETE' );
if ( isset( $response_headers['access-control-allow-methods'] ) ) {
	$allow_methods = array_values( array_intersect( $allow_methods, array_map( 'strtoupper', wp_parse_list( $response_headers['access-control-allow-methods'] ) ) ) );
}

get_headers() rather than headers_list(): it reads what the response intends rather than what PHP already emitted, and works under CLI.

2. Add rest_allowed_cors_methods. Same shape as the sibling CORS list filters rest_exposed_cors_headers and rest_allowed_cors_headers - array in, imploded out, $request in context. rest_pre_serve_request already passes $request, hence add_filter( ..., 10, 3 ).

$allow_methods = apply_filters( 'rest_allowed_cors_methods', $allow_methods, $request instanceof WP_REST_Request ? $request : null );

if ( ! empty( $allow_methods ) ) {
	header( 'Access-Control-Allow-Methods: ' . implode( ', ', $allow_methods ) );
} else {
	header_remove( 'Access-Control-Allow-Methods' );
}

The filter always runs, so a callback can add methods outside the default list, narrow the list, or replace a value the response set. An empty list omits the header, and removes one the response already sent.

Both are needed: (1) alone forces a plugin to restate the whole list, so two plugins conflict instead of composing; (2) alone still gets discarded when a response sets its own value.

$result and $request are untyped, so an unexpected value passed through rest_pre_serve_request is ignored rather than fatal. The default output is unchanged for every site.

Caveat: under ?_envelope=1, envelope_response() moves the response's headers into the body before this runs, so a response value is not seen and the filtered default is sent.

Tests

rest_send_cors_headers() had no coverage at all. New file, 11 tests: default list, no Origin, response value passed through and narrowed, case-insensitive header and method matching, unexpected arguments ignored, filter add and override, the request in context when serving, and the envelope caveat.

PHP does not record header() under the CLI SAPI, so the tests read the final list from a rest_allowed_cors_methods callback at PHP_INT_MAX instead of from the wire. No Xdebug needed, and nothing skips silently.
That also means the header_remove() branch for an empty list has no test. Xdebug doesn't track header_remove() either, so there is no way to observe it under CLI.
--group restapi: 3746 tests, no failures.

Proposed commit message

Getting this one ready to land, here is the message I plan to commit it with.

Claude drafted this from the PR and the ticket history:

REST API: Introduce the `rest_allowed_cors_methods` filter.

Add a filter to `rest_send_cors_headers()` so the `Access-Control-Allow-Methods` header sent with CORS responses can be adjusted. The list of methods is a fixed default and is not derived from the registered routes, so a site serving routes that accept other methods can now advertise them. The filter receives the method list as an array and the request in context, matching the shape of the sibling `rest_exposed_cors_headers` and `rest_allowed_cors_headers` filters, so a plugin can append to or narrow the list rather than restate it. An empty list omits the header.

The filter is seeded from an `Access-Control-Allow-Methods` header already set on the response, limited to the methods in the default list, falling back to the default list itself. A value set through `rest_post_dispatch` was previously discarded: `rest_send_cors_headers()` runs on `rest_pre_serve_request`, after the response's own headers have been emitted, and `header()` replaces by default.

The default output is unchanged for sites that neither set the header nor use the filter. Under `?_envelope=1` a response header is relocated into the response body before this runs, so it is not seen here and the filtered default is sent. This also adds the first test coverage for `rest_send_cors_headers()`.

Follow-up to [56096].

Props moonmeister, sudar, TimothyBlynJacobs, aaemnnosttv, westonruter.
Fixes #46992.

Shout if the props list is missing anyone, or if the framing is off.

AI Use

My follow-up commits on this branch and the refreshed description came out of a session with 🤖 Claude Code. I will review and test.

`Access-Control-Allow-Methods` is the only CORS list header REST API responses send that has no extension point. Its two siblings gained one in 5.5.0 -- `rest_exposed_cors_headers` and `rest_allowed_cors_headers`, both array-filtered then imploded in `WP_REST_Server::send_headers()`, and both given the request in context in 6.3.0. This one was missed each time, because it is sent from `rest_send_cors_headers()` rather than from the server. Adds a filter with the same shape, so a site can compose the list rather than unhook `rest_send_cors_headers` and reimplement it. `$request` is available because `rest_pre_serve_request` already passes it; the filter accepts three arguments to reach it. The filter runs only when the response has not set the header itself, which is the preceding change on this branch. A value on the response is the more specific instruction, and there is no default left to shape once it is present. Adds three tests to the existing CORS header coverage. See #46992.

Keeps the two facts the code does not show -- that this runs after the response's own headers have been sent, and that `header()` replaces by default -- and drops the sentence restating the conditional below it. See #46992. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-08-18T23:03:54Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props moonmeister, adamsilverstein, westonruter, sudar.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2026-08-18T23:07:19Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

adamsilverstein · 2026-08-31T21:28:50Z

Thanks for the continued effort here. Overall this looks good to me @moonmeister!

Claude went over the PR against the ticket discussion, here is most of what came back:

The filter can add methods but it cannot take any away. When the response has set Access-Control-Allow-Methods, rest_allowed_cors_methods never runs at all, which test_response_value_takes_precedence_over_the_filter pins down with assertFalse( $filter_ran ).

That rules out the hardening case. A site narrowing the list to OPTIONS, GET is silently overridden by any plugin that sets the header on its own response, with no way to observe it. It also diverges from the two sibling filters this one is modeled on: rest_exposed_cors_headers and rest_allowed_cors_headers always apply.

Comment 11 on the ticket did not settle precedence either way, so this looks open. Seeding the filter with the response value rather than skipping it keeps option C intact and gives the site the last word:
$allow_methods = isset( $response_headers['access-control-allow-methods'] )
	? array_map( 'trim', explode( ',', $response_headers['access-control-allow-methods'] ) )
	: array( 'OPTIONS', 'GET', 'POST', 'PUT', 'PATCH', 'DELETE' );

$allow_methods = apply_filters( 'rest_allowed_cors_methods', $allow_methods, $request );
header( 'Access-Control-Allow-Methods: ' . implode( ', ', $allow_methods ) );
The response still wins over the default, and the one test above inverts.

Smaller things:

@param WP_HTTP_Response $result should be WP_HTTP_Response|null since it defaults to null, matching the WP_REST_Request|null on the next line. Column alignment does not change, both are 21 characters.

Under ?_envelope=1 the guard sees an empty header list. envelope_response() runs after rest_post_dispatch and builds a fresh response with the old headers relocated into the body, so a plugin's value lands in the JSON and core's default goes on the wire. That is arguably what _envelope is for, so a docblock line seems like enough here rather than a code change.

What do you think - is there a reason to keep the response winning outright that I'm not seeing?

…d list. The filter previously ran only when the response had not set `Access-Control-Allow-Methods` itself, on the reasoning that a value on the response is the more specific instruction. That reverses here, per review: the filter now always runs, seeded with the response's value when there is one and with the default list otherwise. Seeding is a strict superset of the previous behavior. It can express all three outcomes -- filter wins by returning its own list, response wins by returning `$allow_methods` untouched or by registering nothing, and the two compose by appending. Skipping the filter made "filter overrides a response value" unreachable at any priority, and gave the callback no way to observe that it had been bypassed. Default output is unchanged either way. Parsing the header back into an array also normalizes the append case: `WP_HTTP_Response::header()` comma-joins when called with `$replace` false, so `explode()` and `trim()` recover the individual methods. Also types `$result` as `WP_HTTP_Response|null`, matching its default, and notes that `envelope_response()` relocates the response's headers into the body before this runs, so under `?_envelope=1` a response value is not seen here. Inverts the test that pinned the old precedence and adds two: one asserting the filter receives the response's value rather than the default, one covering the envelope case. See #46992. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

moonmeister · 2026-09-15T22:50:49Z

Thanks for the review @adamsilverstein!

No, there's no reason I can find to keep it — taking your suggestion as written. The filter now always runs, seeded with the response's value when available.

The argument I'd hang it on is that seeding is a strict superset of the current behavior. With it, you can express all three outcomes: filter wins (return your own list), response wins (return $methods untouched, or register nothing at all), and compose ($methods[] = 'X'). With "skip-when-set", the "filter overrides a response value" is unreachable at any priority, and the callback can't even observe that it was bypassed. Default output is identical either way, so it's a free gain in expressiveness. Secondary support: nothing in WP privileges a value already set on an object over a filter that runs later, and rest_allowed_cors_methods on rest_pre_serve_request is later than rest_post_dispatch — so the skip was the deviation, not the fix.

Small bonus from parsing the header back out: WP_HTTP_Response::header( $key, $value, false ) comma-joins on append, so explode + trim normalizes the multi-append case for free.

On _envelope: agreed, docblock only. It's narrower than it first looks, too — Access-Control-Allow-Methods is only ever read by the browser off a preflight response, so the enveloped case only bites on an OPTIONS preflight to a URL already carrying ?_envelope=1. And it's a pre-existing property of the envelope that relocates all headers, not something this change introduces. Documented it and added a test so the behavior is at least pinned as intentional.

$result is now WP_HTTP_Response|null. Both small ones done.

…meister/wordpress-develop into fix/rest-cors-allow-methods-clobber * 'fix/rest-cors-allow-methods-clobber' of github.com:moonmeister/wordpress-develop: Docs: Add missing descriptions and declare types for `Translation_Entry` properties. Docs: Remove a blank line between `@param` and `@return` in core docblocks. Build/Test Tools: Stop committing changes to tracked files in pull requests. Code Quality: Resolve return.empty PHPStan errors in baseline. Build/Test Tools: Make `SLACK_GHA_TIMEOUT_WEBHOOK` optional. Filesystem API: Reject UNC and device paths in `validate_file()`. Tests: Use `assertIsCallable()` for callable assertions. Build/Test Tools: Distinguish between cancelled and timed out workflow runs. Build/Test Tools: Remove all `npx` commands. Build/Test Tools: Allow text compression in the local environment.

sudar · 2026-10-01T00:15:01Z

Really happy to see progress on a ticket that I opened in trac 7 years ago.

adamsilverstein

Nice work.

westonruter · 2026-10-19T04:49:30Z

+ * serve requests through a real WP_REST_Server and read the emitted headers with
+ * xdebug_get_headers(), following tests/phpunit/tests/oembed/headers.php.
+ *
+ * @runTestsInSeparateProcesses


Tests that run in a separate process are very slow. We should avoid them if possible.

westonruter · 2026-10-19T04:56:36Z

+		ob_end_clean();
+
+		/** @var list<string> $headers */
+		$headers = xdebug_get_headers();


This function is problematic to use because it's not always available. Not sure if there is an alternative.

hmm, maybe we can use headers-list instead? https://www.php.net/manual/en/function.headers-list.php

Is this available at all in our test builds? maybe we could only run the tests if it is available.

The problem is that output has already been sent, so header() returns false and headers_list() surely wouldn't include anything.

I think the main problem is that we're calling header() in the first place. It seems like we should actually be setting the headers on the $response object. Then we can inspect the headers on $result->get_headers() without ever attempting to send them, right?

Note this is what rest_send_allow_header() does already. So I think we should refactor to follow that same model.

For testing, note also that Spy_REST_Server has a $sent_headers class property.

Agreed that calling header() directly is the root of the testing pain. One timing problem with setting the headers on $result, though: by the time rest_pre_serve_request fires, serve_request() has already sent the response's headers (class-wp-rest-server.php L473–474), so anything added to $result here never reaches the client.

rest_send_allow_header() gets away with it because it runs on rest_post_dispatch, before that point. Were you thinking of moving rest_send_cors_headers() there too? If so, I'd be wary of two things:

Sites that disable core CORS with remove_filter( 'rest_pre_serve_request', 'rest_send_cors_headers' ) would silently get the headers back.

A later rest_post_dispatch callback could overwrite Access-Control-Allow-Origin and Access-Control-Allow-Credentials, which currently can't be set from the response.

An alternative that I think gets you the testability without moving the hook: rest_pre_serve_request already passes the server as its fourth argument, and _oembed_rest_pre_serve_request() already uses it this way to send Content-Type. So we can send through it:

add_filter( 'rest_pre_serve_request', 'rest_send_cors_headers', 10, 4 ); function rest_send_cors_headers( $value, $result = null, $request = null, $server = null ) { $server = $server instanceof WP_REST_Server ? $server : rest_get_server(); // ... $server->send_header( 'Access-Control-Allow-Methods', implode( ', ', $allow_methods ) ); // ... $server->remove_header( 'Access-Control-Allow-Methods' );

Spy_REST_Server already records both into $sent_headers, so the tests could check the real output, including the empty-list removal and the Origin/Credentials headers, without Xdebug or separate processes. On a normal site nothing changes, since send_header() just calls header().

The Vary: Origin line would stay a raw header( ..., false ) because send_header() always replaces and Vary needs to append. That line isn't touched by this PR, but the tests would still need to deal with its warning.

westonruter

I pushed up some fixes for PHPStan errors. I left some feedback as well for what should be addressed.

Co-authored-by: Weston Ruter <westonruter@gmail.com>

westonruter

See https://github.com/WordPress/wordpress-develop/pull/13151/changes#r4315653386

adamsilverstein · 2026-10-19T22:00:04Z

@moonmeister I'm going to push some changes here from the code review.

The batched review suggestion left the empty() guard without its closing parenthesis. Also update the filter docs to describe the narrowed seed list and the omitted header.

xdebug_get_headers() is often unavailable, and the tests needed a separate process each, which is slow. Read the final method list from a late rest_allowed_cors_methods callback instead, and ignore the header() warnings PHP raises once PHPUnit has sent output. The tests now run in process without Xdebug, so the ext-xdebug suggestion in composer.json goes too. Wire-level checks of headers this change does not touch (Vary, Allow-Origin, Allow-Credentials) are dropped, since they cannot be observed without Xdebug.

@SInCE

Native type hints on a hooked callback turn an unexpected value passed through rest_pre_serve_request into a fatal TypeError, so rely on the existing instanceof checks instead. Match methods in a response's Access-Control-Allow-Methods value case-insensitively, as the Fetch standard normalizes them, so 'get' is not silently dropped. Also trim the inline comment and the @SInCE note, which no longer described the narrowed list.

…pty. The response's own headers are sent before this runs, so skipping the write left a response-supplied list on the wire. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011PkCnymaZCDVy1WUqzxYbN

westonruter · 2026-10-19T22:53:14Z

+		if ( ! empty( $allow_methods ) ) {
+			header( 'Access-Control-Allow-Methods: ' . implode( ', ', $allow_methods ) );
+		} else {
+			// Remove a value the response already sent.
+			header_remove( 'Access-Control-Allow-Methods' );
+		}


Instead of calling header() and header_remove(), why not work on $result instead to set the headers there?

Same question as the thread on the tests. Replied there: #13151 (comment)

…default list. Intersecting a response's `Access-Control-Allow-Methods` value with the default list meant a response could only remove methods, never add one. A route handling `PROPFIND`, or `QUERY`, had that method silently dropped, though advertising such methods is part of what this ticket is for. Keep only entries that are valid method tokens per RFC 9110, section 9.1, so junk is still rejected without discarding real methods. Methods are case-sensitive, so they now pass through as given rather than being uppercased, which was only needed for the comparison against the default list. See #46992. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

moonmeister and others added 3 commits August 17, 2026 16:02

fix: tests and conditional set of defaults

3f08a51

moonmeister marked this pull request as ready for review August 18, 2026 23:03

Merge branch 'trunk' into fix/rest-cors-allow-methods-clobber

d8de848

adamsilverstein self-requested a review August 31, 2026 18:11

moonmeister and others added 3 commits September 15, 2026 15:52

Merge branch 'trunk' into fix/rest-cors-allow-methods-clobber

4f7a663

Merge branch 'trunk' into fix/rest-cors-allow-methods-clobber

85c3d9b

adamsilverstein approved these changes Oct 16, 2026

View reviewed changes

Merge branch 'trunk' into fix/rest-cors-allow-methods-clobber

c1f3194

adamsilverstein requested review from swissspidy and westonruter October 18, 2026 23:01

adamsilverstein and others added 4 commits October 18, 2026 21:22

Merge branch 'trunk' into fix/rest-cors-allow-methods-clobber

696734b

Add missing ext-xdebug to suggest in composer.json

d2c2f0e

Fix PHPStan errors in tests

86857b2

Add native type hints for params

326b89e

westonruter reviewed Oct 19, 2026

View reviewed changes

Comment thread src/wp-includes/rest-api.php Outdated

westonruter reviewed Oct 19, 2026

View reviewed changes

Comment thread src/wp-includes/rest-api.php Outdated

westonruter reviewed Oct 19, 2026

View reviewed changes

Comment thread src/wp-includes/rest-api.php Outdated

westonruter reviewed Oct 19, 2026

View reviewed changes

westonruter requested changes Oct 19, 2026

View reviewed changes

Apply batched suggestions from code review

f961c3e

Co-authored-by: Weston Ruter <westonruter@gmail.com>

westonruter reviewed Oct 19, 2026

View reviewed changes

Comment thread src/wp-includes/rest-api.php Outdated

Fix syntax error from prior suggestion

f1de1db

adamsilverstein requested a review from westonruter October 19, 2026 20:45

westonruter requested changes Oct 19, 2026

View reviewed changes

adamsilverstein and others added 4 commits October 19, 2026 15:00

REST API: Fix the parse error in rest_send_cors_headers().

f862087

The batched review suggestion left the empty() guard without its closing parenthesis. Also update the filter docs to describe the narrowed seed list and the omitted header.

adamsilverstein changed the title ~~REST API: Stop rest_send_cors_headers() clobbering Access-Control-Allow-Methods, and make the list filterable.~~ REST API: Add the rest_allowed_cors_methods filter and stop discarding a response's method list. Oct 19, 2026

westonruter reviewed Oct 19, 2026

View reviewed changes

moonmeister and others added 2 commits October 19, 2026 16:43

Merge branch 'trunk' into fix/rest-cors-allow-methods-clobber

a8fc56d

Conversation

moonmeister commented Aug 18, 2026 • edited by adamsilverstein Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Proposed commit message

AI Use

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 18, 2026

Test using WordPress Playground

Some things to be aware of

Uh oh!

adamsilverstein commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moonmeister commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sudar commented Oct 1, 2026

Uh oh!

adamsilverstein left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

adamsilverstein commented Oct 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

moonmeister commented Aug 18, 2026 •

edited by adamsilverstein

Loading

github-actions Bot commented Aug 18, 2026 •

edited

Loading

adamsilverstein commented Aug 31, 2026 •

edited

Loading

moonmeister commented Sep 15, 2026 •

edited

Loading