-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add output buffering for the rendered template #8412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 36 commits
39a3907
f576c3c
33b524b
5e49712
8a6f5a7
e9a46aa
2c074fa
7f79ebd
6637ef5
b98b81e
9f07ad0
4e7029b
fd79c2a
322c930
32cc514
59cf047
b755a24
2dabd76
e65811c
69df1d0
853082e
d38dc86
7f309f5
6ee2a45
1b4a2c4
7f43ee5
b242dc0
ad4c8d4
bbfdaa6
80d24ae
16d7928
dfd6e5e
5409e44
44d52df
6bcbf0e
d4a3da3
6333403
611a482
f49563c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -823,3 +823,141 @@ function load_template( $_template_file, $load_once = true, $args = array() ) { | |
| */ | ||
| do_action( 'wp_after_load_template', $_template_file, $load_once, $args ); | ||
| } | ||
|
|
||
| /** | ||
| * Checks whether the template should be output buffered for enhancement. | ||
| * | ||
| * By default, an output buffer is only started if a {@see 'wp_template_enhancement_output_buffer'} filter has been | ||
| * added be the time a template is included at the {@see 'wp_before_include_template'} action. This allows template | ||
| * responses to be streamed as much as possible when no template enhancements are registered to apply. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @return bool Whether the template should be output-buffered for enhancement. | ||
| */ | ||
| function wp_should_output_buffer_template_for_enhancement(): bool { | ||
| /** | ||
| * Filters whether the template should be output-buffered for enhancement. | ||
| * | ||
| * By default, an output buffer is only started if a {@see 'wp_template_enhancement_output_buffer'} filter has been | ||
| * added. For this default to apply, a filter must be added by the time the template is included at the | ||
| * {@see 'wp_before_include_template'} action. This allows template responses to be streamed as much as possible | ||
| * when no template enhancements are registered to apply. This filter allows a site to opt in to adding such | ||
| * template enhancement filters during the rendering of the template. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @param bool $use_output_buffer Whether an output buffer is started. | ||
| */ | ||
| return (bool) apply_filters( 'wp_should_output_buffer_template_for_enhancement', has_filter( 'wp_template_enhancement_output_buffer' ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Starts the template enhancement output buffer. | ||
| * | ||
| * This function is called immediately before the template is included. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @return bool Whether the output buffer successfully started. | ||
| */ | ||
| function wp_start_template_enhancement_output_buffer(): bool { | ||
| if ( ! wp_should_output_buffer_template_for_enhancement() ) { | ||
| return false; | ||
| } | ||
|
|
||
| $started = ob_start( | ||
| 'wp_finalize_template_enhancement_output_buffer', | ||
| 0, // Unlimited buffer size so that entire output is passed to the filter. | ||
| /* | ||
| * Instead of the default PHP_OUTPUT_HANDLER_STDFLAGS (cleanable, flushable, and removable) being used for | ||
| * flags, the PHP_OUTPUT_HANDLER_FLUSHABLE flag must be omitted. If the buffer were flushable, then each time | ||
| * that ob_flush() is called, a fragment of the output would be sent into the output buffer callback. This | ||
| * output buffer is intended to capture the entire response for processing, as indicated by the chunk size of 0. | ||
| * So the buffer does not allow flushing to ensure the entire buffer can be processed, such as for optimizing an | ||
| * entire HTML document, where markup in the HEAD may need to be adjusted based on markup that appears late in | ||
| * the BODY. | ||
| * | ||
| * If this ends up being problematic, then PHP_OUTPUT_HANDLER_FLUSHABLE could be added to the $flags and the | ||
| * output buffer callback could check if the phase is PHP_OUTPUT_HANDLER_FLUSH and abort any subsequent | ||
| * processing while also emitting a _doing_it_wrong(). | ||
| * | ||
| * The output buffer needs to be removable because WordPress calls wp_ob_end_flush_all() and then calls | ||
| * wp_cache_close(). If the buffers are not all flushed before wp_cache_close() is closed, then some output buffer | ||
| * handlers (e.g. for caching plugins) may fail to be able to store the page output in the object cache. | ||
| * See <https://github.com/WordPress/performance/pull/1317#issuecomment-2271955356>. | ||
| */ | ||
| PHP_OUTPUT_HANDLER_STDFLAGS ^ PHP_OUTPUT_HANDLER_FLUSHABLE | ||
| ); | ||
|
|
||
| if ( $started ) { | ||
| /** | ||
| * Fires when the template enhancement output buffer has started. | ||
| * | ||
| * @since 6.9.0 | ||
| */ | ||
| do_action( 'wp_template_enhancement_output_buffer_started' ); | ||
| } | ||
|
|
||
| return $started; | ||
| } | ||
|
|
||
| /** | ||
| * Finalizes the template enhancement output buffer. | ||
| * | ||
| * @since 6.9.0 | ||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * | ||
| * @see wp_start_template_enhancement_output_buffer() | ||
| * | ||
| * @param string $output Output buffer. | ||
| * @param int $phase Phase. | ||
| * @return string Finalized output buffer. | ||
| */ | ||
| function wp_finalize_template_enhancement_output_buffer( string $output, int $phase ): string { | ||
| // When the output is being cleaned (e.g. pending template is replaced with error page), do not send it through the filter. | ||
| if ( ( $phase & PHP_OUTPUT_HANDLER_CLEAN ) !== 0 ) { | ||
| return $output; | ||
| } | ||
|
|
||
| // Detect if the response is an HTML content type. | ||
| $is_html_content_type = null; | ||
| $html_content_types = array( 'text/html', 'application/xhtml+xml' ); | ||
| foreach ( headers_list() as $header ) { | ||
| $header_parts = preg_split( '/\s*[:;]\s*/', strtolower( $header ) ); | ||
| if ( | ||
| is_array( $header_parts ) && | ||
| count( $header_parts ) >= 2 && | ||
| 'content-type' === $header_parts[0] | ||
| ) { | ||
| $is_html_content_type = in_array( $header_parts[1], $html_content_types, true ); | ||
| break; // PHP only sends the first Content-Type header in the list. | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There’s no in any case, I think we could make our check more explicit. we know we’re looking for the content type header, so we don’t need to split it apart. $is_html = false;
foreach ( $headers_list as $header ) {
if ( 1 === preg_match( '~^content-type:(?:(?:\r\n)?[ \t]+)(?:text/html|application/xhtml\+xml)(?:$|;)~i', $header ) ) {
$is_html = true;
break;
}
}
if ( ! $is_html && in_array( ini_get( 'default_mimetype' ), array( 'text/html', 'application/xhtml+xml' ), true ) ) {
$is_html = true;
}I guess that code doesn’t check for duplicate headers, and could probably be broken into some helper functions to separate the act of parsing headers from the act of checking for HTML mime types. just think it would be worth being strict on the HTTP header parsing since PHP isn’t and since that’s a common opportunity for mistakes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had thought that the last add_action( 'template_redirect', static function () {
header( 'Content-Type: text/plain', false );
header( 'Content-Type: text/html', false );
header( 'Content-Type: application/json', false );
header( 'Content-Type: text/plain', false );
var_dump( headers_list() );
exit;
} );The result is: If I use So it seems it is the first See also the RFC 9110 section on Content-Type:
So it is defined as a singleton so I suppose this is why PHP is only serving one of the headers, but PHP's behavior seems to vary with what is described above by serving the first valid
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about 6ee2a45?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. meh. it’s actually worse. PHP will send up to two header( 'Content-Type: text/plain' );
header( 'Content-Type: text/html' );
header( 'Content-Type: text/xml' );
header( 'Content-Type: application/xml', true );
header( 'Content-Type: application/octect-stream', false );
header( 'Content-Type: application/xml+svg', true );
header( 'Content-Type: application/json', false );This sends the following response. HTTP/1.1 200 OK
Host: localhost:8000
Date: Wed, 15 Oct 2025 17:20:32 GMT
Connection: close
X-Powered-By: PHP/8.4.13
Content-Type: application/xml+svg
Content-Type: application/json
I think it’s still going to work well enough for most happy-path cases, but it could help to step back and this about this from the HTTP perspective.
We can probably debate how important it is to safeguard this code, but I wouldn’t want to rule out subtle ways that code could force-enable or force-disable the output filtering. what we have here is something I see a lot, which is a kind of elaboration on code to try and cover more edge cases, but the approach isn’t stemming from the language of HTTP. we could start from that language and it would involve a comparable amount of code. I believe that the PCRE pattern I provided is accurate (though may be it’s wrong. Alternatively, a simple
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dmsnell OK, I don't actually see two This WP PHP code: add_action( 'template_redirect', static function () {
header( 'Content-Type: text/plain' );
header( 'Content-Type: text/html' );
header( 'Content-Type: text/xml' );
header( 'Content-Type: application/xml', true );
header( 'Content-Type: application/octect-stream', false );
header( 'Content-Type: application/xml+svg', true );
header( 'Content-Type: application/json', false );
var_dump( headers_list() );
exit;
} );Results in: Maybe Nginx is stripping out the second response header?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've modified the parsing of the headers in a follow-up PR, although I didn't go with the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could be. I added your code above but put it inside the the unifying factor here seems to be that this is inconsistent and I think that should encourage us to consider the potential for unexpected for messed up headers. it seems very likely that |
||
| } | ||
| if ( null === $is_html_content_type ) { | ||
| $is_html_content_type = in_array( ini_get( 'default_mimetype' ), $html_content_types, true ); | ||
| } | ||
|
|
||
| // If the content type is not HTML, short-circuit since it is not relevant for enhancement. | ||
| if ( ! $is_html_content_type ) { | ||
| return $output; | ||
| } | ||
|
|
||
| $filtered_output = $output; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what’s the point of creating this copy of the variable? is it for documenting the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but it was also because there were multiple filters applying so it needed to have the diff --git a/src/wp-includes/template.php b/src/wp-includes/template.php
index 35a171e2a8..452edb1f5c 100644
--- a/src/wp-includes/template.php
+++ b/src/wp-includes/template.php
@@ -913,8 +913,6 @@ function wp_finalize_template_optimization_output_buffer( string $output, int $p
return $output;
}
- $filtered_output = $output;
-
/**
* Filters the template optimization output buffer prior to sending to the client.
*
@@ -931,5 +929,5 @@ function wp_finalize_template_optimization_output_buffer( string $output, int $p
* @param string $output Original HTML template output buffer.
* @return string HTML template optimization output buffer.
*/
- return (string) apply_filters( 'wp_template_optimization_output_buffer', $filtered_output, $output );
+ return (string) apply_filters( 'wp_template_optimization_output_buffer', $output, $output );
}But having the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a quality issue of course. I figured it was this reason but never saw the multiple filters.
it jumps out less to me than creating a variable copy¹ and then doing nothing with it but pass it alongside the parent. still, it was clear that the purpose was to communicate something through its name. I have seen the duplicated variable and Core has one with either way, it’s fine. it just stood out to me and I wanted to ask if it was an oversight. ¹ using simplified language because I don’t know how to briefly say “have PHP create a new binding to the variable”
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed the copy of the variable in #10293 |
||
|
|
||
| /** | ||
| * Filters the template enhancement output buffer prior to sending to the client. | ||
| * | ||
| * This filter only applies the HTML output of an included template. This filter is a progressive enhancement | ||
| * intended for applications such as optimizing markup to improve frontend page load performance. Sites must not | ||
| * depend on this filter applying since they may opt to stream the responses instead. Callbacks for this filter are | ||
| * highly discouraged from using regular expressions to do any kind of replacement on the output. Use the HTML API | ||
| * (either `WP_HTML_Tag_Processor` or `WP_HTML_Processor`), or else use {@see DOM\HtmlDocument} as of PHP 8.4 which | ||
| * fully supports HTML5. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @param string $filtered_output HTML template enhancement output buffer. | ||
| * @param string $output Original HTML template output buffer. | ||
| */ | ||
| return (string) apply_filters( 'wp_template_enhancement_output_buffer', $filtered_output, $output ); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.