#64126 closed defect (bug) (fixed)
Sending late HTTP headers not facilitated by template enhancement output buffer
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.9 | Priority: | normal |
| Severity: | normal | Version: | 6.9 |
| Component: | General | Keywords: | has-patch has-unit-tests |
| Focuses: | performance | Cc: |
Description (last modified by )
With #43258 there is now an opt-in way for core to output buffer a rendered template. Output buffers are useful not only for modifying the response body, but also for sending late headers, such as Server-Timing:
- https://github.com/WordPress/gutenberg/blob/trunk/packages/e2e-tests/mu-plugins/server-timing.php
- https://github.com/WordPress/performance/blob/trunk/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
The former is currently facilitated via the wp_template_enhancement_output_buffer filter, but the latter is not. There is no dedicated action to fire after the filters have applied and right before the output buffer callback returns to send the HTTP response body.
Plugins which need to send a Server-Timing header after the template has been rendered shouldn't have to resort to hacking the wp_template_enhancement_output_buffer filter to run at PHP_INT_MAX while passing through the $buffer unchanged.
Related: #64108
Change History (17)
This ticket was mentioned in Slack in #core-editor by westonruter. View the logs.
3 months ago
This ticket was mentioned in PR #10381 on WordPress/wordpress-develop by @westonruter.
3 months ago
#2
- Keywords has-patch added
@westonruter commented on PR #10381:
3 months ago
#4
@dmsnell @johnbillion Here's two example use cases for this action.
# Sending Server-Timing
This shows how to measure how long it takes a template to render and to send that as a Server-Timing header. Note that if there is an wp_send_late_headers action added, then the template enhancement buffer will automatically be opened, the same as if there are any wp_template_enhancement_output_buffer filters added:
add_action(
'wp_before_include_template',
static function (): void {
$start_time = hrtime( true );
add_action(
'wp_send_late_headers',
static function () use ( $start_time ): void {
$server_timing_header = sprintf(
'Server-Timing: wpTemplate; desc="Template render"; dur=%f',
( hrtime( true ) - $start_time ) / 1000000
);
header( $server_timing_header, false );
}
);
}
);
The wp_before_include_template action was newly introduced with the template enhancement output buffer.
# HTTP Conditional Requests
The wp_send_late_headers action can be used to implement conditional requests. Since the wp_send_late_headers action proposed in this PR sends the output buffer as an argument this can be used to compute an ETag to send as a header. In the same action, it can check to see if the client has sent that same ETag in an If-None-Match request header, and of so, send back a 304 Not Modified response:
add_action(
'wp_send_late_headers',
static function ( string $response ) {
$etag = '"' . md5( $response ) . '"';
header( "ETag: $etag" );
header( 'Cache-Control: public, max-age=3600' );
if ( isset( $_SERVER['HTTP_IF_NONE_MATCH'] ) ) {
$client_etag = wp_unslash( $_SERVER['HTTP_IF_NONE_MATCH'] );
if ( $client_etag === $etag ) {
status_header( 304 );
}
}
}
);
I find that if PHP sends a 304 response header, that it also omits sending any response body. This prevents the need to try to use the wp_before_include_template filter to empty out the response. But if need be, this could be made explicit:
-
src/wp-includes/template.php
diff --git a/src/wp-includes/template.php b/src/wp-includes/template.php index 035b7e16b9..19984ca9ab 100644
a b function wp_finalize_template_enhancement_output_buffer( string $output, int $ph 998 998 999 999 $do_send_late_headers( $filtered_output ); 1000 1000 1001 if ( 304 === http_response_code() ) { 1002 $filtered_output = ''; 1003 } 1004 1001 1005 return $filtered_output; 1002 1006 }
#5
@
3 months ago
- Keywords has-unit-tests added
In PR #10381:
- Introduce
wp_send_late_headersaction which fires right right afterwp_template_enhancement_output_bufferfilters have applied, and right before the output buffer is flushed. The output buffer is passed as an argument to the function so that plugins may do things like send anETagheader calculated from the content. - The template enhancement output buffer now is enabled by default if _either_ there is a
wp_template_enhancement_output_bufferfilter added _or_ there is awp_send_late_headersaction added. - The
wp_start_template_enhancement_output_buffer()callback for thewp_before_include_templateaction is increased from the default 10 to 1000. This goes with the previous point, so that plugins can add those filters and actions during thewp_before_include_templateaction without having to worry about adding them too late, that is, afterwp_start_template_enhancement_output_buffer()has run. - The
wp_send_late_headersaction fires regardless of whether the buffered response is HTML.
@westonruter commented on PR #10381:
3 months ago
#6
I find that if PHP sends a 304 response header, that it also omits sending any response body. This prevents the need to try to use the
wp_before_include_templatefilter to empty out the response. But if need be, this could be made explicit:
However, this I envision would wreak havoc on caching plugins which have output buffers started much much earlier, e.g. right before plugins_loaded:
WP Super Cache defines `wp_cache_postload()` to by default call `wp_cache_phase2()` which does `ob_start( 'wp_cache_ob_callback' );`. So it is necessary for the output buffer from WP Super Cache to have access to the response body, even in the case of a 304 Not Modified response. I verified this is the case with another plugin that does this, to wrap the template enhancement output buffer with the plugin code above that sense 304:
ob_start( function ( $buffer ) {
error_log( strlen( $buffer ) ); // => 89069
return $buffer;
} );
Even though the HTTP status may be 304, the response body is still available internally before PHP sends the response with the body omitted.
Nevertheless, this will still have the effect of causing WP Super Cache to not cache the response:
} elseif ( isset( $wp_super_cache_query['is_304'] ) ) {
wp_cache_debug( 'HTTP 304 (Not Modified) sent. Caching disabled.' );
$cache_this_page = false;
But that would be something which could be addressed with caching plugins as need be, if someone wants to add a plugin that opts in to sending 304 responses.
This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.
3 months ago
@westonruter commented on PR #10381:
3 months ago
#9
FYI: I further improved the action's phpdoc in r61088 versus what I had pushed to this PR:
-
src/wp-includes/template.php
a b function wp_finalize_template_enhancement_output_buffer( string $output, int $ph 987 987 * 988 988 * This happens immediately before the template enhancement output buffer is flushed. This is in contrast with 989 989 * the {@see 'send_headers'} action which fires after the initial headers have been sent before the template 990 * has begun rendering, and thus does not depend on output buffering. 990 * has begun rendering, and thus does not depend on output buffering. This action does not fire if the "template 991 * enhancement output buffer" was not started. This output buffer is automatically started if this action is added 992 * before {@see wp_start_template_enhancement_output_buffer()} runs at the {@see 'wp_before_include_template'} 993 * action with priority 1000. Before this point, the output buffer will also be started automatically if there was a 994 * {@see 'wp_template_enhancement_output_buffer'} filter added, or if the 995 * {@see 'wp_should_output_buffer_template_for_enhancement'} filter is made to return `true`. 991 996 * 992 997 * @since 6.9.0 993 998 *
@westonruter commented on PR #10381:
3 months ago
#10
@dmsnell replying to your https://github.com/WordPress/wordpress-develop/pull/10381#discussion_r2476656773 outside of an inline comment for better visibility:
is this supposed to be for 6.9 still, or for 7.0? it’s probably great and I am too slow to recognize it, but I worry about haste in choosing the name.
I intended it for 6.9, yes. I'm not dead set on the name, though. If you think it is not ideal, I'm happy to revisit for beta3.
Part of the inspiration for the name also comes from the print_late_styles filter (and print_late_styles() function) which handles printing stylesheets at wp_footer if a stylesheet was enqueued late (after wp_print_styles() runs in wp_head). In this way, there are two places where stylesheets can be printed: standard or late. With Core-64099, these late-printed styles are also hoisted up to wp_head. Likewise, the template enhancement output buffer also enables the same thing for sending late HTTP headers: if the output buffer is open, plugins can continue sending headers up until the wp_send_late_headers action fires in the same way they can continue enqueueing styles up until the wp_print_styles() function runs in wp_footer.
However the action is named, having _some_ action that fires after the filters have applied seems like something important we missed in the original commit for adding the template enhancement output buffer. I want to avoid devs having to hack the wp_template_enhancement_output_buffer filter to send a header (as is done today with the template_include filter) without otherwise modifying the content. Consider a plugin that adds an ETag header but only has the filter:
// Plugin that adds an ETag header.
add_filter(
'wp_template_enhancement_output_buffer',
static function ( $buffer ) {
header( sprintf( 'ETag: "%s"', md5( $buffer ) ) );
return $buffer; // Not touched!
},
1000 // (Nothing could possibly come after this, right?)
);
But then consider another plugin which modifies the output buffer even later:
add_filter(
'wp_template_enhancement_output_buffer',
static function ( $buffer ) {
// Don't do this!
return str_replace(
'Wordpress', // phpcs:ignore WordPress.WP.CapitalPDangit.MisspelledInText -- Hi Dennis!
'WordPress',
$buffer
);
},
99999
);
I want to avoid this.
maybe you had already worked on this for a while, in which case I trust you.
Originally I had thought of send_late_headers but @johnbillion suggested adding the wp_ prefix. I didn't work on the name for _that_ long, but the rationale above shows how I arrived at it.
Since the output is about to be sent to the client, it is essential that _no output_ be emitted from callbacks for this action. This would seem to make mentioning "headers" in the name a worthwhile reminder of that, but this could be made explicit in phpdoc instead.
it sounds like
send_headersbut as you point out, it’s something different.
How?
what I started looking at when I saw it was whether we could use those streaming buffers to capture the first output and send late headers. that is, Core could actually _always_ buffer the output, but only lock that buffer when plugins demand it.
what I started looking at when I saw it was whether we could use those streaming buffers to capture the first output and send late headers. that is, Core could actually _always_ buffer the output, but only lock that buffer when plugins demand it.
Interesting. This is somewhat different than the model I was using relating sending HTTP headers to enqueueing stylesheets.
As I am understanding, what you're suggesting is that the output buffer could always start, but the chunk size would only be set to zero if there is a wp_template_enhancement_output_buffer filter added. This would ensure that the entire buffer can be passed into the filter. Otherwise, if there is no wp_template_enhancement_output_buffer, then an alternate chunk size could be used, like 4KB, and then the first time the output buffer callback is called, there could be an action that fires which allows plugins to send "later" headers based on what was found in the HTML for the initial chunk. Is that right? What headers do you envision being sent here? Perhaps elevating some dns-prefetch/preconnect/preload links?
So you're talking about sending "later" headers as opposed the "latest" headers I'm targeting.
if
template-loaderimmediately prints toSTDOUTthis is moot, of course. if there’s _always_ an immediate flush aftersend_headersthen I see no critique of this filter.
Well, it does print to STDOUT right away by virtue of including the template file. So there would have to be an output buffer opened to be able to send a header after the template has been included. But the chunk size could be set to 4KB as mentioned above, to ensure that the response is streamed.
before_flushing_output_enhancement_bufferthere are plenty of things one might do there, I suppose. are headers the only thing?
Headers wouldn't have to be the only thing done at this action, no. But sending headers would be the only _output_ which could be done at this point. No printing would be allowed. I would think that the shutdown action would be more preferable to use than this, however.
If we didn't want to specifically refer to headers in the action name, since the action runs in the wp_finalize_template_enhancement_output_buffer() action, and given the other hooks wp_template_enhancement_output_buffer and wp_should_output_buffer_template_for_enhancement, some alternative ideas to before_flushing_output_enhancement_buffer:
wp_finalized_template_enhancement_output_buffer(cf.deleted_post,added_option,activated_plugin, and many more)wp_flush_template_enhancement_output_bufferwp_before_flush_template_enhancement_output_bufferwp_before_template_enhancement_output_buffer_flush(cf.wp_before_admin_bar_render,before_delete_post)
also I thought that when I reviewed the other work that the template buffer trapped the template generation, but didn’t also include the
<!DOCTYPE html><html…>content in the HEAD…? I am mistaken, right?
Actually, the template enhancement buffer _does_ trap the entire rendered template, including <!DOCTYPE html><html…>. I think you may be thinking about a block theme's `template-canvas.php` which which buffers the rendered blocks in memory, and then proceeds with printing the root element.
Aside: I just realized the phpdoc for this action and the wp_template_enhancement_output_buffer filter need to specifically call out that callbacks _cannot_ do any calls to ob_start() or else they'll get a fatal error:
PHP Fatal error: ob_start(): Cannot use output buffering in output buffering display handlers in /var/www/src/wp-content/plugins/try-ob-start-during-finalize-template-output-buffer.php on line 9
Example:
add_filter( 'wp_template_enhancement_output_buffer', static function ( $buffer ) { // Note: DO NOT DO THIS!!!! ❌ ob_start(); ?> <script> document.write('Hello World!!!'); </script> <?php $script = wp_remove_surrounding_empty_script_tags( ob_get_clean() ); return $buffer . wp_get_inline_script_tag( $script ); } );
#11
@
3 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
Re-opening based on post-commit PR conversation, to revise the action name.
This ticket was mentioned in PR #10443 on WordPress/wordpress-develop by @westonruter.
3 months ago
#12
@westonruter commented on PR #10381:
3 months ago
#13
Follow-up PR to rename the action: https://github.com/WordPress/wordpress-develop/pull/10443
@westonruter commented on PR #10443:
3 months ago
#14
Without any strong feelings about the names, including the previous one, _after_ stands out more clearly to me than _finalized_, and I think we can see some natural associations with it based on the way the documentation for the filter is worded.
@dmsnell Do you mean wp_finalized_template_enhancement_output_buffer is fine but you would prefer something that incorporated "after"?
There's wp_after_insert_post, wp_after_execute_ability, wp_after_admin_bar_render, and wp_after_load_template as precedent for that but there should then be a verb added after after, so like wp_after_finalize_template_enhancement_output_buffer? Oof. That's getting long.
@westonruter commented on PR #10443:
3 months ago
#16
Committed in r61111.
…
Trac ticket: https://core.trac.wordpress.org/ticket/64126