Opened 7 years ago
Last modified 3 hours ago
#46992 reopened enhancement
Add a filter which allows the HTTP headers for REST API Endpoints to be changed
| Reported by: | sudar | Owned by: | adamsilverstein |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | REST API | Version: | 4.4 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
Right now there is no way to modify the HTTP response headers for a REST API Endpoint.
The HTTP headers for non REST API endpoint requests can be changed using the wp_headers filter, but it won't work for REST Endpoints.
I am proposing to add a new filter called wp_rest_headers that allows developers to override the HTTP Response headers of REST API Endpoints.
Attachments (1)
Change History (17)
#3
@
7 years ago
@TimothyBlynJacobs the goal here is to modify the CORS headers in responses from WP REST API Endpoints. It doesn't look like this is possible using the rest_post_dispatch filter you mentioned, am I missing something?
#5
@
7 years ago
But doesn't rest_send_cors_headers() use a regular header function call, it doesn't go through WP_REST_Server?
#6
@
7 years ago
@TimothyBlynJacobs
The rest_send_cors_headers function uses the regular header php function to send the headers, but the headers that are sent are hardcoded in that function. There is no easy way to change the headers that will be passed to the header function.
We have to remove the rest_send_cors_headers from the rest_pre_serve_request filter and then manually perform what is already done by rest_send_cors_headers function and then print the required headers.
On the other hand if you want to change the headers for non rest pages, then all you have to do is hook into the wp_headers function and return the desired headers as array.
This patch introduces a new filter, so that we can easily change the headers for rest api endpoints similar to how we do it for non rest api endpoints.
Please let me know if my explanation is clear or if you still have any other questions.
#7
@
7 years ago
But how will this filter help in that case?
The headers sent by rest_send_cors_headers won't be filterable that proposed filter.
#8
@
7 years ago
@TimothyBlynJacobs is correct - it is currently possible to add/remove/change headers sent by a REST request using the https://developer.wordpress.org/reference/hooks/rest_post_dispatch/ filter.
add_filter( 'rest_post_dispatch', function ( \WP_REST_Response $response ) {
$headers = $response->get_headers();
$headers['X-Added-Header'] = 'Foo';
$response->set_headers( $headers );
return $response;
} );
This filter provides the Response, Server, and Request object instances to the callback which should provide all the context necessary to modify the response as needed.
#9
@
7 years ago
- Milestone 5.3
- Resolution → invalid
- Status assigned → closed
Thanks for the feedback @TimothyBlynJacobs and @aaemnnosttv - I'm closing this as invalid.
Please feel free to re-open @sudar if you feel there is still an outstanding use case not addressed by the approach described above using the rest_post_dispatch filter.
#10
@
4 weeks ago
- Resolution invalid
- Status closed → reopened
- Version 5.2 → 4.4
Came across this issue in relation to #65616
rest_send_cors_headers() sends Access-Control-Allow-Methods with a raw header() call, which replaces by default. It runs on rest_pre_serve_request, which fires after WP_REST_Server::send_headers() has already emitted the response's own headers. So a value set on the response is sent, then overwritten by core's hardcoded list before it reaches the client.
add_filter( 'rest_post_dispatch', function ( $response ) { $response->header( 'Access-Control-Allow-Methods', 'OPTIONS, GET' ); return $response; } );
Request any route with an Origin header. Expected OPTIONS, GET; actual
OPTIONS, GET, POST, PUT, PATCH, DELETE.
Why this despite #46992
This ticket asked for a way to filter these headers and was closed invalid, on the advice to use rest_post_dispatch. That advice is correct for ordinary response headers and does not work for this one, which was raised at the time and never answered.
TimothyBlynJacobs, in comments comment:5:ticket:46992 and comment:7:ticket:46992: "doesn't rest_send_cors_headers() use a regular header function call, it doesn't go through WP_REST_Server?" and "The headers sent by rest_send_cors_headers won't be filterable [by] that proposed filter." The close cites comment comment:8:ticket:46992, which addresses the general case rather than the CORS one.
So the recorded resolution answers a different question than the ticket asked. Nobody argued the header should not be overridable.
To be precise: overriding is not impossible. Hooking rest_pre_serve_request at priority 11 or later and calling header() does win. That is undocumented, order-dependent, and two plugins doing it will conflict. A test that asserts it works today is included, so the claim remains bounded.
See also #57752, which improved the two sibling filters rest_exposed_cors_headers and rest_allowed_cors_headers in 6.3 and left this header out.
#43428 touches the same function but proposes something different — sending the preflight headers only on OPTIONS, dropping OPTIONS from the method list, and adding Access-Control-Max-Age. It does not concern overriding the value, and the two are compatible: if its third proposal lands, this fix simply applies on the OPTIONS path.
#11
@
4 weeks ago
Okay, just thinking through a solution on this. Three options:
A. Make the default conditional. Pass the result into rest_send_cors_headers() and send
the default list only when the response has not already set the header. Adds no hook, and makes
the rest_post_dispatch advice in comment:8:ticket:46992 correct.
B. Add a filter. Access-Control-Expose-Headers and Access-Control-Allow-Headers are each
built as an array, filtered, then imploded — rest_exposed_cors_headers and
rest_allowed_cors_headers, both 5.5.0, both given $request in #57752. A
rest_allowed_cors_methods alongside them would match, and the array form lets plugins append
rather than restate the list. rest_pre_serve_request already passes $request. On its own,
this does not stop a value set via rest_post_dispatch from being discarded.
C. Both — the filter as the extension point, the conditional, so a response-set value is not
silently dropped.
I lean C. B matches the sibling headers and is what this ticket asked for; A alone leaves the
summary unaddressed; B alone leaves the discard. The default output is unchanged in all three.
Happy to open a PR against whichever you prefer, with tests — the function currently has none.
This ticket was mentioned in PR #13151 on WordPress/wordpress-develop by @moonmeister.
4 weeks ago
#12
- Keywords has-unit-tests added
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.
## 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.
$response_headers = ( $result instanceof WP_HTTP_Response ) ? $result->get_headers() : array(); $response_headers = array_change_key_case( $response_headers ); if ( ! isset( $response_headers['access-control-allow-methods'] ) ) { // ...send the default... }
get_headers() rather than headers_list(): it reads what the response intends rather than what PHP already emitted, and needs no Xdebug 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 = 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 filter runs only when the response has not set the header — a value on the response is the more specific instruction, and there is no default left to shape.
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.
The default output is unchanged for every site.
## Tests
rest_send_cors_headers() had no coverage at all. New file, 10 tests / 46 assertions — both changes plus baseline behavior (Origin present and absent, default output, Vary still appending, Access-Control-Allow-Origin and -Credentials not response-settable).
- 6 of 10 fail against stock
rest-api.php. --group restapi: 3560 / 16285, no new failures against a 3550 / 16239 baseline. The one error in both runs (Test_oEmbed_Controller::test_proxy_with_classic_embed_provider) is pre-existing on trunk.
⚠️ The tests read the wire with
xdebug_get_headers()— these headers bypassWP_REST_Server::send_header(), soSpy_REST_Servercannot see them (same approach astests/phpunit/tests/oembed/headers.php). Without Xdebug they skip silently and the suite reports green.
@adamsilverstein commented on PR #13151:
2 weeks ago
#13
Thanks for the continued effort here. Overall this looks good to me.
_Claude went over the PR against the ticket discussion, here is 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_methodsnever runs at all, whichtest_response_value_takes_precedence_over_the_filterpins down withassertFalse( $filter_ran ).
That rules out the hardening case. A site narrowing the list to
OPTIONS, GETis 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_headersandrest_allowed_cors_headersalways 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 $resultshould beWP_HTTP_Response|nullsince it defaults to null, matching theWP_REST_Request|nullon the next line. Column alignment does not change, both are 21 characters.- Under
?_envelope=1the guard sees an empty header list.envelope_response()runs afterrest_post_dispatchand 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_envelopeis 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?
@moonmeister commented on PR #13151:
21 hours ago
#14
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.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
What's the use case for this filter that can't be solved using the
rest_post_dispatchfilter?