Skip to content

Fix FacetWP refresh and other shutdown buffer readers: process in the output handler by default again #1158

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.

Sign up for GitHub

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

Merged
selul merged 3 commits into from
Sep 21, 2026
Merged

Fix FacetWP refresh and other shutdown buffer readers: process in the output handler by default again #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca34eb5
fix: leave third-party output buffers usable at shutdown
selul Sep 21, 2026
55ecbb3
Revert "fix: leave third-party output buffers usable at shutdown"
selul Sep 21, 2026
d844ce2
fix: process the page inside the output handler by default again
selul Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions inc/compatibilities/groovy_menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@
*
* @reason Groovy Menu's auto-integration opens its own output buffer on `init`
* and, on `shutdown` at priority 0, calls ob_get_clean() on whichever buffer is
* on top to insert the menu markup after <body>. Since 4.2.12 we capture and
* process our buffer at `shutdown` (PHP_INT_MIN) and re-arm an empty one, so
* Groovy Menu receives an empty string, finds no <body> and drops the menu.
* on top to insert the menu markup after <body>.
*
* We apply Groovy Menu's final-output filter to the page we capture, before
* image replacement, and unhook its own shutdown step at that moment. The menu
* is inserted regardless of buffer order and its images are optimized too. When
* our capture does not run (legacy `optml_capture_at_shutdown` mode, or a
* third-party flush of our buffer) Groovy Menu keeps its own shutdown step.
* This only matters with the `optml_capture_at_shutdown` opt-in, where we capture
* and process our buffer at `shutdown` (PHP_INT_MIN) and re-arm an empty one, so
* Groovy Menu would receive an empty string and drop the menu. In that mode we
* apply Groovy Menu's final-output filter to the page we capture, before image
* replacement, and unhook its own shutdown step at that moment. In the default
* in-handler mode our capture never runs and Groovy Menu keeps its own step.
*/
class Optml_groovy_menu extends Optml_compatibility {
/**
Expand Down
74 changes: 45 additions & 29 deletions inc/manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,19 +913,22 @@ public function process_template_redirect_content() {
}

/**
* Start an output buffer that captures the page HTML.
* Start the output buffer that holds the page HTML.
*
* On normal requests the buffer is captured and processed by close_buffer()
* at shutdown, outside of PHP's display-handler context, so callbacks hooked
* into our filters are free to use output buffering themselves and fatal
* errors raised during processing keep their real message instead of being
* masked by "Cannot use output buffering in output buffering display handlers".
* By default the page is processed by the attached handler when the buffer
* is flushed, the way it worked up to 4.2.11. We never flush other buffers
* and open no buffer at shutdown, so code that opens a buffer early and reads
* it back with ob_get_clean() at shutdown (FacetWP, Groovy Menu) keeps working.
*
* The attached handler is only a fallback for buffers flushed outside of
* close_buffer() — third-party force-flush loops, ob_flush() streaming, or
* core's wp_ob_end_flush_all() reaching the re-armed buffer. A named method
* is used instead of a closure so the buffer can be identified as ours via
* ob_get_status()['name'].
* When the optml_capture_at_shutdown filter returns true, close_buffer()
* captures and processes the buffer at shutdown instead, outside of PHP's
* display-handler context. Callbacks hooked into our filters can then use
* output buffering themselves, and fatal errors raised during processing
* keep their real message instead of being masked by "Cannot use output
* buffering in output buffering display handlers".
*
* A named method is used instead of a closure so the buffer can be
* identified as ours via ob_get_status()['name'].
*
* @return void
*/
Expand All @@ -941,15 +944,35 @@ private function start_capture_buffer() {
const OB_HANDLER_NAME = 'Optml_Manager::handle_buffer_fallback';

/**
* Output-buffer handler attached to our capture buffer.
* Whether the page is captured and processed at shutdown, outside of PHP's display-handler context.
*
* @return bool
*/
private function captures_at_shutdown() {
/**
* Filters whether the page is captured and processed at shutdown, outside
* of PHP's display-handler context, instead of inside the output-buffer
* handler.
*
* Off by default: the shutdown capture flushes the buffers
* stacked above ours and opens a new buffer afterwards, which breaks code
* that reads its own buffer back at shutdown. Return true to opt in.
*
* @param bool $capture_at_shutdown Whether to process the buffer at shutdown.
*/
return apply_filters( 'optml_capture_at_shutdown', false ) === true;
}

/**
* Output-buffer handler attached to our buffer.
*
* Runs only when the buffer is flushed outside of close_buffer(). Content is
* passed through UNPROCESSED here: running the replacement filter graph
* inside a PHP display handler would turn any third-party ob_*() call into
* an uncatchable fatal ("Cannot use output buffering in output buffering
* display handlers") — the very crash this rework removes. The only
* exception is the legacy mode selected via the optml_capture_at_shutdown
* filter, which explicitly restores the previous in-handler processing.
* By default this is where the page is processed. An exception thrown by the
* replacement never breaks the page: the content is returned untouched.
*
* With the optml_capture_at_shutdown opt-in the handler runs only when the
* buffer is flushed outside of close_buffer(), and the content is passed
* through unprocessed, because running the replacement filter graph inside a
* display handler is what that mode exists to avoid.
*
* @param string $content The buffered content.
* @param int $phase PHP's output-handler phase bitmask (unused; keeps replace_content()'s $partial parameter shielded from it).
Expand All @@ -960,7 +983,7 @@ public function handle_buffer_fallback( $content, $phase = 0 ) {
if ( self::$ob_processed || $content === '' ) {
return $content;
}
if ( apply_filters( 'optml_capture_at_shutdown', true ) === false ) {
if ( ! $this->captures_at_shutdown() ) {
try {
return $this->replace_content( $content, self::is_ajax_request() );
} catch ( Throwable $t ) {
Expand All @@ -982,14 +1005,7 @@ public function close_buffer() {
return;
}

/**
* Filters whether the captured page is processed at shutdown, outside of
* PHP's display-handler context. Return false to restore the legacy
* behavior of processing inside the output-buffer handler.
*
* @param bool $capture_at_shutdown Whether to process the buffer at shutdown.
*/
if ( apply_filters( 'optml_capture_at_shutdown', true ) === false ) {
if ( ! $this->captures_at_shutdown() ) {
if ( ob_get_length() ) {
ob_end_flush();
}
Expand Down Expand Up @@ -1027,7 +1043,7 @@ public function close_buffer() {
* @return void
*/
public function close_final_buffer() {
if ( ! self::$ob_started ) {
if ( ! self::$ob_started || ! $this->captures_at_shutdown() ) {
return;
}
$this->capture_and_process_buffer( false );
Expand Down
70 changes: 70 additions & 0 deletions tests/test-zz-buffer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public function setUp(): void {
Optml_Tag_Replacer::instance()->init();
Optml_Manager::instance()->init();

// The shutdown capture is opt-in; most tests below exercise it.
add_filter( 'optml_capture_at_shutdown', '__return_true' );

$this->reset_buffer_state();
$this->base_level = ob_get_level();
}
Expand All @@ -53,6 +56,8 @@ public function tearDown(): void {
}
}
$this->reset_buffer_state();
remove_filter( 'optml_capture_at_shutdown', '__return_true' );
remove_filter( 'optml_capture_at_shutdown', '__return_false' );
parent::tearDown();
}

Expand Down Expand Up @@ -290,4 +295,69 @@ public function test_legacy_in_handler_mode() {
$this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) );
$this->assertSame( $this->base_level, ob_get_level() );
}

/**
* Without the opt-in the page is processed inside the output handler, as up to 4.2.11.
*/
public function test_default_processes_in_handler() {
remove_filter( 'optml_capture_at_shutdown', '__return_true' );
$manager = Optml_Manager::instance();
ob_start();
$manager->process_template_redirect_content();
echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
$manager->close_buffer();

$this->assertSame( $this->base_level + 1, ob_get_level(), 'No buffer is opened at shutdown.' );
$manager->close_final_buffer();
$out = ob_get_clean();

$this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) );
$this->assertSame( $this->base_level, ob_get_level() );
}

/**
* By default, code that opened a buffer before ours and reads it back on
* shutdown priority 0 (FacetWP refresh, Groovy Menu) finds its own buffer on
* top, holding the processed page.
*/
public function test_default_foreign_buffer_below_can_be_read_back() {
remove_filter( 'optml_capture_at_shutdown', '__return_true' );
$manager = Optml_Manager::instance();
ob_start();
ob_start(); // Third-party buffer opened on init, before ours.
$foreign_level = ob_get_level();
$manager->process_template_redirect_content();
echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
$manager->close_buffer();

$this->assertSame( $foreign_level, ob_get_level(), 'The foreign buffer is on top again.' );

// The third party reads its buffer back on shutdown priority 0.
$page = ob_get_clean();
$manager->close_final_buffer();

$this->assertSame( 1, substr_count( $page, 'i.optimole.com' ) );
$this->assertSame( '', ob_get_clean() );
$this->assertSame( $this->base_level, ob_get_level() );
}

/**
* By default an exception thrown during the replacement never breaks the page.
*/
public function test_default_exception_passes_content_through() {
remove_filter( 'optml_capture_at_shutdown', '__return_true' );
$thrower = function () {
throw new RuntimeException( 'broken third-party callback' );
};
add_filter( 'optml_url_pre_process', $thrower );
$manager = Optml_Manager::instance();
ob_start();
$manager->process_template_redirect_content();
echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
$manager->close_buffer();
$out = ob_get_clean();
remove_filter( 'optml_url_pre_process', $thrower );

$this->assertSame( self::IMG_TAGS, $out );
}
}
10 changes: 7 additions & 3 deletions tests/test-zz-groovy-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ public function setUp(): void {
add_filter( 'groovy_menu_final_output', 'groovy_menu_add_after_body' );
$this->compatibility = new Optml_groovy_menu();

// The compatibility acts only with the shutdown capture opt-in.
add_filter( 'optml_capture_at_shutdown', '__return_true' );

$this->reset_buffer_state();
$this->base_level = ob_get_level();
}
Expand All @@ -95,6 +98,7 @@ public function tearDown(): void {
remove_filter( 'groovy_menu_final_output', 'groovy_menu_add_after_body' );
remove_filter( 'optml_captured_page_html', [ $this->compatibility, 'insert_menu' ] );
remove_filter( 'optml_capture_at_shutdown', '__return_false' );
remove_filter( 'optml_capture_at_shutdown', '__return_true' );
parent::tearDown();
}

Expand Down Expand Up @@ -167,10 +171,10 @@ public function test_menu_inserted_when_groovy_buffer_is_above_ours() {
}

/**
* In legacy in-handler mode Groovy Menu keeps its own shutdown step and still works.
* In the default in-handler mode Groovy Menu keeps its own shutdown step and still works.
*/
public function test_legacy_mode_keeps_groovy_shutdown_step() {
add_filter( 'optml_capture_at_shutdown', '__return_false' );
public function test_default_mode_keeps_groovy_shutdown_step() {
remove_filter( 'optml_capture_at_shutdown', '__return_true' );
$this->compatibility->register();
$manager = Optml_Manager::instance();
ob_start();
Expand Down
Loading