REST API: Attempts to output headers without confirming headers have not been sent

[kraftbj]

The rest_handle_deprecated_function and rest_handle_deprecated_argument functions fire header() without confirming if headers have already been sent or not.

This can result in PHP warnings, which depending on the site's settings, will output into the REST API response, which breaks JSON.

Error: Warning: Cannot modify header information - headers already sent by (output started at /public_html//wp-includes/functions.php:3720) in /public_html/wp-includes/rest-api.php on line 492

[kraftbj]

Check if headers_sent is false before firing header

[kraftbj]

In 40787.diff, I opted to check for headers_sent right before header() instead of earlier in the file in the event that the functions are expanded to do more than only the headers in the future.

[jnylen0]

Excellent catch, this has bitten me a large number of times and I'm not sure why I didn't think to patch it. I think we should get this in 4.8.

[matt]

Fine with this coming in if it's before beta 2.

[kadamwhite]

+1 from me, nice catch indeed. No strong opinion on wrapping each individual header() call vs making one check earlier in the file.

[dd32]

What scenario's would these headers be output, after the output has started in a rest context? Is it embedded responses? It's not clear how one would reproduce this other than outputting something before the rest api does.

[jnylen0]

There are various filters that run after the API sends its output. One example from WP.com used shutdown I think. This was pretty annoying to work around.

I would actually​ prefer to remove these headers entirely, because I don't think we should be exposing this information about the site. They were originally introduced during the migration from WP-API v1 to v2 (​ref), and they've finished serving that purpose long ago.

Marking as dev-feedback to get a 2nd opinion on just ripping this code out.

[rmccue]

Makes sense to only send if ! headers_sent()

Deprecated information is still useful for reporting internally-deprecated things (it was intended to be a permanent change), but I'd say we should follow the regular trigger_error behaviour and only send when WP_DEBUG is on.

[jnylen0]

In 40782:

REST API: Do not set X-WP-Deprecated* headers as often.

Only set these headers if (1) WP_DEBUG is enabled and (2) headers have not already been sent.

Previously, this code could generate warnings by trying to set a header after response data has already been sent. This happens when code attached to the shutdown filter calls a deprecated function, for example.

Also, these headers are unlikely to be useful in the majority of cases; let's only send them if WP_DEBUG is enabled.

Props kraftbj, jnylen0, ocean90, rmccue.
Fixes #40787.