Improve rest_(allowed|exposed)_cors_headers filters

[bor0]

In a use case where there are multiple REST endpoints, but we only want to expose specific headers for specific endpoints, the filters rest_allowed_cors_headers and rest_exposed_cors_headers do not have any request in context and are quite limited in that regard.

It would've been ideal if we could write our code as such:

add_filter( 'rest_exposed_cors_headers', function( $headers, $request ) {
	if ( '/foo/bar' === $request->get_route() ) {
		$headers[] = 'X-RateLimit-Limit';
	}
	return $headers;
}, 10, 2 );

[rachelbaker]

@bor0 Would you be able to use the rest_post_dispatch filter for your use case?

[bor0]

Hi @rachelbaker,

We could certainly do the following:

add_filter( 'rest_post_dispatch', function( $result, $server, $request ) {
	$server->send_header( 'Access-Control-Allow-Headers', 'X-RateLimit-Limit' );
	return $result;
}, 10, 3 );

But the problem is that this will overwrite the existing headers set by WordPress WP_REST_Server::serve_request.

In addition, we cannot get the existing headers from $server as the function send_header merely wraps the header function and doesn't store these results anywhere, so we end up having to copy-paste:

add_filter( 'rest_post_dispatch', function( $result, $server, $request ) {
	$allow_headers = array(
		'Authorization',
		'X-WP-Nonce',
		'Content-Disposition',
		'Content-MD5',
		'Content-Type',
	);
	$allow_headers[] = 'X-RateLimit-Limit';
	$this->send_header( 'Access-Control-Allow-Headers', implode( ', ', $allow_headers ) );
	return $result;
}, 10, 3 );

[spacedmonkey]

@rachelbaker With beta 1 on the 27th, will this is be committed in time?

[chaion07]

Thanks @bor0 for reporting this. We reviewed this ticket during a recent bug-scrub session and based on the feedback we are updating the milestone to 6.4!

Props to @oglekler

[kadamwhite]

This approach makes sense to me, given @bor0's argument above about avoiding copy-paste code. I think we should have some tests around it, though -- writing up a basic one now.

Add unit tests to initial patch uploaded to ticket.

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

[kadamwhite]

​https://github.com/WordPress/wordpress-develop/pull/4740 now contains a version of @bor0's patch with unit tests, which assert the second argument is present, is a request object, and matches the unique dispatched route. The tests were pushed first to validate in CI that the tests fail without the patch, then pass with it.

I'm comfortable moving forward with this one, though I think we're just missing the 6.3 cutoff.

@spacedmonkey assigning to you to take forward, if that's alright.

[spacedmonkey]

This patch looks good to go. Marking as ready to commit and assigning to @kadamwhite

[kadamwhite]

In 56096:

REST API: Expose current $request object to cors_header filters in WP_REST_SERVER->serve_request().

Allows headers to be more easily set on a per-response basis when more or less security is needed on a specific route.

Props bor0, rachelbaker, spacedmonkey, chaion07, oglekler, SergeyBiryukov.
Fixes #57752.

Merged in https://core.trac.wordpress.org/changeset/56096 / 68be569925

[johnbillion]

In 56156:

REST API: Correct some filter docblocks.

See #57752