From 30095770156ebec27968cf71d240dfa460013288 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:01:02 +0300 Subject: [PATCH 1/7] chore(deps): bump enshrined/svg-sanitize from 0.22.0 to 1.0.0 (#1142) Bumps [enshrined/svg-sanitize](https://github.com/darylldoyle/svg-sanitizer) from 0.22.0 to 1.0.0. - [Release notes](https://github.com/darylldoyle/svg-sanitizer/releases) - [Changelog](https://github.com/darylldoyle/svg-sanitizer/blob/master/CHANGELOG.md) - [Commits](https://github.com/darylldoyle/svg-sanitizer/compare/0.22.0...1.0.0) --- updated-dependencies: - dependency-name: enshrined/svg-sanitize dependency-version: 1.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- composer.json | 2 +- composer.lock | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index 5e727a2a..98bf344b 100644 --- a/composer.json +++ b/composer.json @@ -59,6 +59,6 @@ "php": ">=7.4", "codeinwp/themeisle-sdk": "^3.3", "codeinwp/optimole-sdk": "^1.2", - "enshrined/svg-sanitize": "^0.22.0" + "enshrined/svg-sanitize": "^1.0.0" } } diff --git a/composer.lock b/composer.lock index 2cc90509..0c300585 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "982a4078faab475dd9f9f90a3f065675", + "content-hash": "9236121e91b49149c045b0f11d2dbc13", "packages": [ { "name": "codeinwp/optimole-sdk", @@ -105,16 +105,16 @@ }, { "name": "enshrined/svg-sanitize", - "version": "0.22.0", + "version": "1.0.0", "source": { "type": "git", "url": "https://github.com/darylldoyle/svg-sanitizer.git", - "reference": "0afa95ea74be155a7bcd6c6fb60c276c39984500" + "reference": "f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/0afa95ea74be155a7bcd6c6fb60c276c39984500", - "reference": "0afa95ea74be155a7bcd6c6fb60c276c39984500", + "url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff", + "reference": "f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff", "shasum": "" }, "require": { @@ -144,9 +144,9 @@ "description": "An SVG sanitizer for PHP", "support": { "issues": "https://github.com/darylldoyle/svg-sanitizer/issues", - "source": "https://github.com/darylldoyle/svg-sanitizer/tree/0.22.0" + "source": "https://github.com/darylldoyle/svg-sanitizer/tree/1.0.0" }, - "time": "2025-08-12T10:13:48+00:00" + "time": "2026-09-01T09:35:47+00:00" }, { "name": "symfony/polyfill-php80", From 449ad4a29da0a636a944c7ac46f36ce59f14c48d Mon Sep 17 00:00:00 2001 From: Girish Panchal <79647963+girishpanchal30@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:31:28 +0530 Subject: [PATCH 2/7] fix: ensure action is string to prevent fatal (#1148) --- inc/manager.php | 2 +- tests/test-ajax-request-detection.php | 104 ++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/test-ajax-request-detection.php diff --git a/inc/manager.php b/inc/manager.php index cb3cea3f..1fa34eae 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -393,7 +393,7 @@ public static function is_ajax_request() { if ( ! wp_doing_ajax() ) { return false; } - if ( isset( $_REQUEST['action'] ) && strpos( $_REQUEST['action'], 'wpmdb' ) !== false ) { + if ( isset( $_REQUEST['action'] ) && is_string( $_REQUEST['action'] ) && strpos( $_REQUEST['action'], 'wpmdb' ) !== false ) { return false; } diff --git a/tests/test-ajax-request-detection.php b/tests/test-ajax-request-detection.php new file mode 100644 index 00000000..914af0af --- /dev/null +++ b/tests/test-ajax-request-detection.php @@ -0,0 +1,104 @@ +had_action = isset( $_REQUEST['action'] ); + + if ( $this->had_action ) { + $this->original_action = $_REQUEST['action']; + } + + // DOING_AJAX cannot be defined per test, the filter is the supported way in. + add_filter( 'wp_doing_ajax', '__return_true' ); + + wp_set_current_user( 0 ); + } + + public function tearDown(): void { + remove_filter( 'wp_doing_ajax', '__return_true' ); + + if ( $this->had_action ) { + $_REQUEST['action'] = $this->original_action; + } else { + unset( $_REQUEST['action'] ); + } + + $this->had_action = false; + $this->original_action = null; + + parent::tearDown(); + } + + /** + * An array-valued action, i.e. `action[]=wpmdb`, must not fatal. + */ + public function test_array_action_does_not_fatal() { + $_REQUEST['action'] = [ 'wpmdb' ]; + + $this->assertTrue( Optml_Manager::is_ajax_request() ); + } + + /** + * A nested array action is handled the same way. + */ + public function test_nested_array_action_does_not_fatal() { + $_REQUEST['action'] = [ 'a' => [ 'wpmdb' ] ]; + + $this->assertTrue( Optml_Manager::is_ajax_request() ); + } + + /** + * WP Migrate DB requests stay excluded, the guarantee from 02df0774. + */ + public function test_wpmdb_action_still_excluded() { + $_REQUEST['action'] = 'wpmdb_verify_connection_to_remote_site'; + + $this->assertFalse( Optml_Manager::is_ajax_request() ); + } + + /** + * An unrelated AJAX action is still treated as a replaceable request. + */ + public function test_unrelated_action_is_ajax_request() { + $_REQUEST['action'] = 'woocommerce_get_refreshed_fragments'; + + $this->assertTrue( Optml_Manager::is_ajax_request() ); + } + + /** + * A request with no action at all is still treated as a replaceable request. + */ + public function test_missing_action_is_ajax_request() { + unset( $_REQUEST['action'] ); + + $this->assertTrue( Optml_Manager::is_ajax_request() ); + } +} From 7a24d2aa04f82715bd98391137fac296cf69f793 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:01:21 +0300 Subject: [PATCH 3/7] chore(deps): bump codeinwp/themeisle-sdk from 3.3.61 to 3.3.62 (#1154) Bumps [codeinwp/themeisle-sdk](https://github.com/Codeinwp/themeisle-sdk) from 3.3.61 to 3.3.62. - [Release notes](https://github.com/Codeinwp/themeisle-sdk/releases) - [Changelog](https://github.com/Codeinwp/themeisle-sdk/blob/v3.3.62/CHANGELOG.md) - [Commits](https://github.com/Codeinwp/themeisle-sdk/compare/v3.3.61...v3.3.62) --- updated-dependencies: - dependency-name: codeinwp/themeisle-sdk dependency-version: 3.3.62 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- composer.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/composer.lock b/composer.lock index 0c300585..8f192862 100644 --- a/composer.lock +++ b/composer.lock @@ -64,16 +64,16 @@ }, { "name": "codeinwp/themeisle-sdk", - "version": "3.3.61", + "version": "3.3.62", "source": { "type": "git", "url": "https://github.com/Codeinwp/themeisle-sdk.git", - "reference": "9fe698b52dec768a0dd8b500fb51efe40962ee99" + "reference": "8363c9cab1a233095a76cd48e96fb64ce1b29ef8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/Codeinwp/themeisle-sdk/zipball/9fe698b52dec768a0dd8b500fb51efe40962ee99", - "reference": "9fe698b52dec768a0dd8b500fb51efe40962ee99", + "url": "https://api.github.com/repos/Codeinwp/themeisle-sdk/zipball/8363c9cab1a233095a76cd48e96fb64ce1b29ef8", + "reference": "8363c9cab1a233095a76cd48e96fb64ce1b29ef8", "shasum": "" }, "require-dev": { @@ -99,9 +99,9 @@ ], "support": { "issues": "https://github.com/Codeinwp/themeisle-sdk/issues", - "source": "https://github.com/Codeinwp/themeisle-sdk/tree/v3.3.61" + "source": "https://github.com/Codeinwp/themeisle-sdk/tree/v3.3.62" }, - "time": "2026-08-24T15:59:27+00:00" + "time": "2026-09-17T17:08:21+00:00" }, { "name": "enshrined/svg-sanitize", From 97661edbea078dc0587c75b4f5740e50f6892edf Mon Sep 17 00:00:00 2001 From: Marius Cristea Date: Fri, 18 Sep 2026 19:01:14 +0300 Subject: [PATCH 4/7] fix: insert Groovy Menu markup into the page captured at shutdown (#1150) Groovy Menu's auto-integration grabs the top output buffer at shutdown priority 0. Since 4.2.12 our capture at PHP_INT_MIN leaves it an empty re-armed buffer, so the menu markup was dropped. Apply its final-output filter to the captured page before image replacement and take over its shutdown step only when our capture runs. Fixes #1149 Co-authored-by: Claude Fable 5.1 --- inc/compatibilities/groovy_menu.php | 69 +++++++++ inc/manager.php | 19 ++- tests/test-zz-groovy-menu.php | 215 ++++++++++++++++++++++++++++ 3 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 inc/compatibilities/groovy_menu.php create mode 100644 tests/test-zz-groovy-menu.php diff --git a/inc/compatibilities/groovy_menu.php b/inc/compatibilities/groovy_menu.php new file mode 100644 index 00000000..2e61cf01 --- /dev/null +++ b/inc/compatibilities/groovy_menu.php @@ -0,0 +1,69 @@ +. 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 and drops the menu. + * + * 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. + */ +class Optml_groovy_menu extends Optml_compatibility { + /** + * Groovy Menu's shutdown callback that grabs the top output buffer. + */ + const GROOVY_SHUTDOWN_CALLBACK = 'groovy_menu_pre_shutdown'; + + /** + * Groovy Menu's filter that inserts the menu markup into the page HTML. + */ + const GROOVY_OUTPUT_FILTER = 'groovy_menu_final_output'; + + /** + * Should we load the integration logic. + * + * @return bool Should we load. + */ + public function should_load() { + return function_exists( self::GROOVY_SHUTDOWN_CALLBACK ) + && has_action( 'shutdown', self::GROOVY_SHUTDOWN_CALLBACK ) !== false; + } + + /** + * Register integration details. + * + * @return void + */ + public function register() { + add_filter( 'optml_captured_page_html', [ $this, 'insert_menu' ] ); + } + + /** + * Insert the Groovy Menu markup into the captured page and take over its shutdown step. + * + * @param string $html The captured page HTML, before image replacement. + * + * @return string + */ + public function insert_menu( $html ) { + $priority = has_action( 'shutdown', self::GROOVY_SHUTDOWN_CALLBACK ); + if ( $priority === false ) { + return $html; + } + // Our capture ran, so Groovy Menu must not ob_get_clean() the re-armed empty buffer afterwards. + remove_action( 'shutdown', self::GROOVY_SHUTDOWN_CALLBACK, $priority ); + + // Same guard as Groovy Menu's own shutdown callback. + if ( ! defined( 'GROOVY_MENU_SCRIPTS_INIT' ) ) { + return $html; + } + + return apply_filters( self::GROOVY_OUTPUT_FILTER, $html ); // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Groovy Menu's own filter. + } +} diff --git a/inc/manager.php b/inc/manager.php index 1fa34eae..6de2afa6 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -116,6 +116,7 @@ final class Optml_Manager { 'hummingbird', 'aruba_hsc', 'spc', + 'groovy_menu', ]; /** * The current state of the buffer. @@ -1029,7 +1030,7 @@ public function close_final_buffer() { if ( ! self::$ob_started ) { return; } - $this->capture_and_process_buffer(); + $this->capture_and_process_buffer( false ); } /** @@ -1039,9 +1040,11 @@ public function close_final_buffer() { * buffer another plugin opened at the same level after ours was closed is * never captured or closed by us. * + * @param bool $is_page Whether this is the page capture (true) or the late shutdown output (false). + * * @return bool Whether our buffer was found and consumed. */ - private function capture_and_process_buffer() { + private function capture_and_process_buffer( $is_page = true ) { if ( self::$ob_level === 0 || ob_get_level() !== self::$ob_level ) { return false; } @@ -1054,6 +1057,18 @@ private function capture_and_process_buffer() { self::$ob_processed = true; ob_end_clean(); if ( $html !== false && $html !== '' ) { + if ( $is_page ) { + /** + * Filters the captured page HTML before Optimole processes it. + * + * Runs once per request, on the buffer captured at shutdown, outside of + * PHP's display-handler context. Late output echoed by other shutdown + * callbacks is not passed through this filter. + * + * @param string $html The full page HTML. + */ + $html = apply_filters( 'optml_captured_page_html', $html ); + } echo $this->replace_content( $html, self::is_ajax_request() ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- full page HTML, escaping would break the page. } return true; diff --git a/tests/test-zz-groovy-menu.php b/tests/test-zz-groovy-menu.php new file mode 100644 index 00000000..a6790cc3 --- /dev/null +++ b/tests/test-zz-groovy-menu.php @@ -0,0 +1,215 @@ +. + * + * @param string $output Page HTML. + * + * @return string + */ + function groovy_menu_add_after_body( $output ) { + return preg_replace( '#(\)#i', '$1' . Test_Groovy_Menu::MENU, $output, 1 ); + } +} + +/** + * Class Test_Groovy_Menu. + */ +class Test_Groovy_Menu extends WP_UnitTestCase { + const MENU = '
'; + const PAGE = ''; + + /** + * The output-buffer nesting level before each test. + * + * @var int + */ + private $base_level = 0; + + /** + * The compatibility under test. + * + * @var Optml_groovy_menu + */ + private $compatibility; + + public function setUp(): void { + parent::setUp(); + if ( ! defined( 'GROOVY_MENU_SCRIPTS_INIT' ) ) { + define( 'GROOVY_MENU_SCRIPTS_INIT', true ); + } + $settings = new Optml_Settings(); + $settings->update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com', 'example.org' ], + ] ); + $settings->update( 'lazyload', 'disabled' ); + $settings->update( 'cdn', 'enabled' ); + Optml_Url_Replacer::instance()->init(); + Optml_Tag_Replacer::instance()->init(); + Optml_Manager::instance()->init(); + + $GLOBALS['gm_test_shutdown_calls'] = 0; + add_action( 'shutdown', 'groovy_menu_pre_shutdown', 0 ); + add_filter( 'groovy_menu_final_output', 'groovy_menu_add_after_body' ); + $this->compatibility = new Optml_groovy_menu(); + + $this->reset_buffer_state(); + $this->base_level = ob_get_level(); + } + + public function tearDown(): void { + $this->reset_buffer_state( true ); + while ( ob_get_level() > $this->base_level ) { + // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged + if ( ! @ob_end_clean() ) { + break; + } + } + $this->reset_buffer_state(); + remove_action( 'shutdown', 'groovy_menu_pre_shutdown', 0 ); + 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' ); + parent::tearDown(); + } + + /** + * Reset Optml_Manager buffer statics between tests. + * + * @param bool $processed Value for the processed flag. + */ + private function reset_buffer_state( $processed = false ) { + $reflection = new ReflectionClass( Optml_Manager::class ); + foreach ( [ 'ob_started' => false, 'ob_level' => 0, 'ob_processed' => $processed ] as $property => $value ) { + $prop = $reflection->getProperty( $property ); + $prop->setAccessible( true ); + $prop->setValue( null, $value ); + } + } + + /** + * The compatibility loads only when Groovy Menu's auto-integration hooked its shutdown step. + */ + public function test_loads_only_with_auto_integration() { + $this->assertTrue( $this->compatibility->should_load() ); + remove_action( 'shutdown', 'groovy_menu_pre_shutdown', 0 ); + $this->assertFalse( $this->compatibility->should_load() ); + } + + /** + * Groovy Menu buffer opened on init sits below ours: the menu is inserted and optimized. + */ + public function test_menu_inserted_when_groovy_buffer_is_below_ours() { + $this->compatibility->register(); + $manager = Optml_Manager::instance(); + ob_start(); + ob_start(); // Groovy Menu's own buffer, opened on init. + $manager->process_template_redirect_content(); + echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_buffer(); + + $this->assertFalse( has_action( 'shutdown', 'groovy_menu_pre_shutdown' ), 'Groovy Menu shutdown step is taken over.' ); + + $manager->close_final_buffer(); + ob_end_flush(); // Core flushes Groovy Menu's buffer at shutdown. + $out = ob_get_clean(); + + $this->assertSame( 1, substr_count( $out, 'gm-navbar' ) ); + $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); + $this->assertStringNotContainsString( '"http://example.org/wp-content/uploads/gm-logo.jpg', $out ); + $this->assertSame( 0, $GLOBALS['gm_test_shutdown_calls'] ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * Groovy Menu buffer stacked above ours is flushed through, and the menu is still inserted. + */ + public function test_menu_inserted_when_groovy_buffer_is_above_ours() { + $this->compatibility->register(); + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + ob_start(); // Groovy Menu's buffer opened after ours. + echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_buffer(); + $manager->close_final_buffer(); + $out = ob_get_clean(); + + $this->assertFalse( has_action( 'shutdown', 'groovy_menu_pre_shutdown' ) ); + $this->assertSame( 1, substr_count( $out, 'gm-navbar' ) ); + $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * In legacy 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' ); + $this->compatibility->register(); + $manager = Optml_Manager::instance(); + ob_start(); + ob_start(); // Groovy Menu's buffer. + $manager->process_template_redirect_content(); + echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_buffer(); + + $this->assertSame( 0, has_action( 'shutdown', 'groovy_menu_pre_shutdown' ), 'Groovy Menu shutdown step is left in place.' ); + + groovy_menu_pre_shutdown(); // Groovy Menu at shutdown priority 0. + $out = ob_get_clean(); + + $this->assertSame( 1, $GLOBALS['gm_test_shutdown_calls'] ); + $this->assertSame( 1, substr_count( $out, 'gm-navbar' ) ); + $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * When third-party code flushes our buffer before shutdown, Groovy Menu keeps its own shutdown step. + */ + public function test_early_flush_keeps_groovy_shutdown_step() { + $this->compatibility->register(); + $manager = Optml_Manager::instance(); + ob_start(); + ob_start(); // Groovy Menu's buffer. + $manager->process_template_redirect_content(); + echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + ob_end_flush(); // Third-party force flush of our buffer. + $manager->close_buffer(); + $manager->close_final_buffer(); + + $this->assertSame( 0, has_action( 'shutdown', 'groovy_menu_pre_shutdown' ) ); + + groovy_menu_pre_shutdown(); + $out = ob_get_clean(); + + $this->assertSame( 1, substr_count( $out, 'gm-navbar' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } +} From ca34eb5218a3b7622c47d79dab0e87aa52bb9442 Mon Sep 17 00:00:00 2001 From: selul Date: Mon, 21 Sep 2026 13:09:53 +0300 Subject: [PATCH 5/7] fix: leave third-party output buffers usable at shutdown Since 4.2.12 close_buffer() re-armed an empty capture buffer and force-flushed buffers stacked above ours at shutdown PHP_INT_MIN. Code that opens a buffer early and reads it back with ob_get_clean() on shutdown priority 0 (FacetWP refresh, Groovy Menu) received an empty string. - Stop re-arming a buffer after the capture, so a buffer opened before ours is on top again and holds the processed page. - When third-party buffers sit above ours, defer the capture until the priority 0 shutdown callbacks ran instead of force-flushing them. - Guard the Groovy Menu compatibility against a double menu insertion when its own shutdown step runs before the deferred capture. Fixes #1156 Co-Authored-By: Claude Fable 5.1 --- inc/compatibilities/groovy_menu.php | 43 +++++++-- inc/manager.php | 120 +++++++++++++++++-------- tests/test-zz-buffer.php | 133 ++++++++++++++++++++++++---- tests/test-zz-groovy-menu.php | 18 ++-- 4 files changed, 244 insertions(+), 70 deletions(-) diff --git a/inc/compatibilities/groovy_menu.php b/inc/compatibilities/groovy_menu.php index 2e61cf01..2eee5e6f 100644 --- a/inc/compatibilities/groovy_menu.php +++ b/inc/compatibilities/groovy_menu.php @@ -4,15 +4,16 @@ * * @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 . 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 and drops the menu. + * on top to insert the menu markup after . * - * 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. + * When its buffer sits below ours, we capture and process the page at the start + * of shutdown. We apply Groovy Menu's final-output filter to that page first, + * before image replacement, and unhook its own shutdown step at that moment, so + * the menu images are optimized too. When its buffer sits above ours the capture + * is deferred, Groovy Menu's own shutdown step runs first and we only make sure + * the menu is not inserted twice. When our capture does not run at all (legacy + * `optml_capture_at_shutdown` mode, or a third-party flush of our buffer) Groovy + * Menu keeps its own shutdown step. */ class Optml_groovy_menu extends Optml_compatibility { /** @@ -25,6 +26,13 @@ class Optml_groovy_menu extends Optml_compatibility { */ const GROOVY_OUTPUT_FILTER = 'groovy_menu_final_output'; + /** + * Whether Groovy Menu's final-output filter already ran for this request. + * + * @var bool + */ + private $menu_inserted = false; + /** * Should we load the integration logic. * @@ -42,6 +50,23 @@ public function should_load() { */ public function register() { add_filter( 'optml_captured_page_html', [ $this, 'insert_menu' ] ); + add_filter( self::GROOVY_OUTPUT_FILTER, [ $this, 'mark_menu_inserted' ], PHP_INT_MAX ); // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Groovy Menu's own filter. + } + + /** + * Remember that Groovy Menu's final-output filter already ran for this request. + * + * When third-party buffers sit above ours the capture is deferred, and Groovy + * Menu's own shutdown step runs first. The menu must not be inserted twice. + * + * @param string $html The page HTML. + * + * @return string + */ + public function mark_menu_inserted( $html ) { + $this->menu_inserted = true; + + return $html; } /** @@ -53,7 +78,7 @@ public function register() { */ public function insert_menu( $html ) { $priority = has_action( 'shutdown', self::GROOVY_SHUTDOWN_CALLBACK ); - if ( $priority === false ) { + if ( $priority === false || $this->menu_inserted ) { return $html; } // Our capture ran, so Groovy Menu must not ob_get_clean() the re-armed empty buffer afterwards. diff --git a/inc/manager.php b/inc/manager.php index 6de2afa6..5da44c72 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -427,7 +427,6 @@ public function register_hooks() { add_action( 'template_redirect', [ $this, 'register_after_setup' ] ); add_action( 'rest_api_init', [ $this, 'process_template_redirect_content' ], PHP_INT_MIN ); add_action( 'shutdown', [ $this, 'close_buffer' ], PHP_INT_MIN ); - add_action( 'shutdown', [ $this, 'close_final_buffer' ], PHP_INT_MAX ); foreach ( self::$loaded_compatibilities as $registered_compatibility ) { $registered_compatibility->register(); } @@ -923,7 +922,7 @@ public function process_template_redirect_content() { * * 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 + * a request that exits before our shutdown capture runs. A named method * is used instead of a closure so the buffer can be identified as ours via * ob_get_status()['name']. * @@ -997,40 +996,92 @@ public function close_buffer() { } /* - * Flush the buffers other plugins stacked on top of ours so their - * handlers still transform the page before we process it, preserving - * the same order as a full top-down flush at request shutdown. + * Buffers stacked above ours belong to code that may still need them at + * shutdown: FacetWP, Groovy Menu and others open a buffer early and call + * ob_get_clean() on shutdown priority 0. Force-flushing those buffers now + * would leave them with nothing, so we wait until the priority 0 callbacks + * are done. Registering during the hook run places us after every callback + * already queued on priority 0, and still ahead of core's + * wp_ob_end_flush_all() on priority 1. */ - while ( ob_get_level() > self::$ob_level ) { - // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- a non-flushable buffer must not raise a notice; we stop on failure. - if ( ! @ob_end_flush() ) { - break; - } + if ( $this->has_foreign_buffers_above() ) { + add_action( 'shutdown', [ $this, 'close_deferred_buffer' ], 0 ); + return; } - if ( ! $this->capture_and_process_buffer() ) { - do_action( 'optml_log', 'Optimole buffer was closed earlier by third-party code.' ); + $this->flush_and_capture_buffer(); + } + + /** + * Close the buffer after the shutdown priority 0 callbacks, when third-party buffers were stacked above ours. + * + * @return void + */ + public function close_deferred_buffer() { + if ( ! self::$ob_started ) { return; } + $this->flush_and_capture_buffer(); + } - /* - * Re-arm the capture so output echoed by later shutdown callbacks is - * still processed and unguarded third-party flush calls find a buffer - * to close instead of raising a notice. + /** + * The output handlers that may sit above our buffer without delaying the capture. + * + * Core's template enhancement buffer (WordPress 6.9+) is opened on every + * front-end request after ours and nothing reads it back at shutdown. + */ + const FLUSHABLE_OB_HANDLERS = [ 'wp_finalize_template_enhancement_output_buffer' ]; + + /** + * Check whether third-party output buffers are stacked above ours. + * + * @return bool + */ + private function has_foreign_buffers_above() { + if ( self::$ob_level === 0 || ob_get_level() <= self::$ob_level ) { + return false; + } + /** + * Filters the output handler names that can be flushed at the start of + * shutdown without delaying the page capture. + * + * @param string[] $handlers Output handler names, as reported by ob_get_status(). */ - $this->start_capture_buffer(); + $flushable = (array) apply_filters( 'optml_flushable_ob_handlers', self::FLUSHABLE_OB_HANDLERS ); + foreach ( array_slice( ob_get_status( true ), self::$ob_level ) as $buffer ) { + if ( ! in_array( $buffer['name'] ?? '', $flushable, true ) ) { + return true; + } + } + + return false; } /** - * Close the re-armed buffer at the very end of shutdown. + * Flush the buffers stacked above ours, then capture and process our own. + * + * No buffer is opened afterwards: code that started buffering before us and + * reads its buffer back later in shutdown must find its own buffer on top, + * holding the processed page, not an empty one of ours. * * @return void */ - public function close_final_buffer() { - if ( ! self::$ob_started ) { - return; + private function flush_and_capture_buffer() { + /* + * Flush the buffers other plugins stacked on top of ours so their + * handlers still transform the page before we process it, preserving + * the same order as a full top-down flush at request shutdown. + */ + while ( ob_get_level() > self::$ob_level ) { + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- a non-flushable buffer must not raise a notice; we stop on failure. + if ( ! @ob_end_flush() ) { + break; + } + } + + if ( ! $this->capture_and_process_buffer() ) { + do_action( 'optml_log', 'Optimole buffer was closed earlier by third-party code.' ); } - $this->capture_and_process_buffer( false ); } /** @@ -1040,11 +1091,9 @@ public function close_final_buffer() { * buffer another plugin opened at the same level after ours was closed is * never captured or closed by us. * - * @param bool $is_page Whether this is the page capture (true) or the late shutdown output (false). - * * @return bool Whether our buffer was found and consumed. */ - private function capture_and_process_buffer( $is_page = true ) { + private function capture_and_process_buffer() { if ( self::$ob_level === 0 || ob_get_level() !== self::$ob_level ) { return false; } @@ -1057,18 +1106,15 @@ private function capture_and_process_buffer( $is_page = true ) { self::$ob_processed = true; ob_end_clean(); if ( $html !== false && $html !== '' ) { - if ( $is_page ) { - /** - * Filters the captured page HTML before Optimole processes it. - * - * Runs once per request, on the buffer captured at shutdown, outside of - * PHP's display-handler context. Late output echoed by other shutdown - * callbacks is not passed through this filter. - * - * @param string $html The full page HTML. - */ - $html = apply_filters( 'optml_captured_page_html', $html ); - } + /** + * Filters the captured page HTML before Optimole processes it. + * + * Runs once per request, on the buffer captured at shutdown, outside of + * PHP's display-handler context. + * + * @param string $html The full page HTML. + */ + $html = apply_filters( 'optml_captured_page_html', $html ); echo $this->replace_content( $html, self::is_ajax_request() ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- full page HTML, escaping would break the page. } return true; diff --git a/tests/test-zz-buffer.php b/tests/test-zz-buffer.php index 15425d5c..e105eaba 100644 --- a/tests/test-zz-buffer.php +++ b/tests/test-zz-buffer.php @@ -12,6 +12,19 @@ * @license http://opensource.org/licenses/gpl-2.0.php GNU Public License */ +if ( ! function_exists( 'optml_test_flushable_handler' ) ) { + /** + * Pass-through output handler with a stable name for ob_get_status(). + * + * @param string $content Buffer content. + * + * @return string + */ + function optml_test_flushable_handler( $content ) { + return $content; + } +} + /** * Class Test_Buffer. */ @@ -53,6 +66,7 @@ public function tearDown(): void { } } $this->reset_buffer_state(); + remove_action( 'shutdown', [ Optml_Manager::instance(), 'close_deferred_buffer' ], 0 ); parent::tearDown(); } @@ -94,7 +108,6 @@ function ( $html ) use ( &$probed ) { $manager->process_template_redirect_content(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertSame( 1, $probed ); @@ -103,10 +116,11 @@ function ( $html ) use ( &$probed ) { } /** - * A buffer another plugin stacks on top of ours is flushed through its own - * handler first, and we process its transformed output — never swallow it. + * A buffer another plugin stacks on top of ours is not force-flushed at the + * start of shutdown. The capture waits for the priority 0 callbacks, then the + * foreign handler runs first and we process its transformed output. */ - public function test_foreign_buffer_above_is_flushed_first() { + public function test_foreign_buffer_above_defers_capture() { $manager = Optml_Manager::instance(); ob_start(); $manager->process_template_redirect_content(); @@ -116,9 +130,14 @@ function ( $content ) { return $content . ''; } ); + $foreign_level = ob_get_level(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - $manager->close_final_buffer(); + + $this->assertSame( $foreign_level, ob_get_level(), 'The foreign buffer is left alone at the start of shutdown.' ); + $this->assertSame( 0, has_action( 'shutdown', [ $manager, 'close_deferred_buffer' ] ) ); + + $manager->close_deferred_buffer(); $out = ob_get_clean(); // Both the page image and the one appended by the foreign handler are optimized. @@ -127,6 +146,89 @@ function ( $content ) { $this->assertSame( $this->base_level, ob_get_level() ); } + /** + * Code that opened a buffer after ours and reads it back on shutdown + * priority 0 (FacetWP, Groovy Menu with an early Optimole buffer) still + * finds its own buffer, and what it echoes back is processed. + */ + public function test_foreign_buffer_above_can_be_read_back_at_shutdown() { + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + ob_start(); // Third-party buffer opened after ours. + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_buffer(); + + // The third party reads its buffer back on shutdown priority 0. + $page = ob_get_clean(); + $this->assertStringContainsString( 'wp-custom-header', $page ); + echo '' . $page; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + + $manager->close_deferred_buffer(); + $out = ob_get_clean(); + + $this->assertStringContainsString( '', $out ); + $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * Code that opened a buffer before ours and reads it back on shutdown + * priority 0 finds its own buffer on top, holding the processed page. We + * leave no empty buffer of ours above it. + */ + public function test_foreign_buffer_below_receives_processed_page() { + $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(), 'No buffer of ours is left above the foreign one.' ); + $this->assertFalse( has_action( 'shutdown', [ $manager, 'close_deferred_buffer' ] ) ); + + // The third party reads its buffer back on shutdown priority 0. + $page = ob_get_clean(); + $this->assertSame( 1, substr_count( $page, 'i.optimole.com' ) ); + $this->assertSame( '', ob_get_clean() ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * Handlers listed as flushable, like core's template enhancement buffer, + * do not delay the capture. + */ + public function test_flushable_buffer_above_does_not_defer_capture() { + $manager = Optml_Manager::instance(); + add_filter( 'optml_flushable_ob_handlers', [ $this, 'allow_test_handler' ] ); + ob_start(); + $manager->process_template_redirect_content(); + ob_start( 'optml_test_flushable_handler' ); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_buffer(); + remove_filter( 'optml_flushable_ob_handlers', [ $this, 'allow_test_handler' ] ); + + $this->assertFalse( has_action( 'shutdown', [ $manager, 'close_deferred_buffer' ] ) ); + $out = ob_get_clean(); + $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * Mark the test handler as flushable. + * + * @param string[] $handlers Handler names. + * + * @return string[] + */ + public function allow_test_handler( $handlers ) { + $handlers[] = 'optml_test_flushable_handler'; + + return $handlers; + } + /** * When third-party code force-flushes our buffer before shutdown, the * content is passed through UNPROCESSED: running the filter graph inside a @@ -141,7 +243,6 @@ public function test_third_party_flush_passes_content_through() { ob_end_flush(); // Third-party force flush of our buffer. $this->assertSame( $this->base_level + 1, ob_get_level() ); $manager->close_buffer(); - $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertStringNotContainsString( 'i.optimole.com', $out ); @@ -171,7 +272,6 @@ function ( $html ) { echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ob_end_flush(); // Would exit(255) if the handler ran the filter graph. $manager->close_buffer(); - $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertStringContainsString( 'themes/twentyseventeen/assets/images/header.jpg', $out ); @@ -190,7 +290,6 @@ public function test_foreign_buffer_at_same_level_is_not_consumed() { ob_start(); // ...and opens its own at the same level. echo 'FOREIGN'; $manager->close_buffer(); - $manager->close_final_buffer(); $this->assertSame( $this->base_level + 1, ob_get_level() ); $this->assertSame( 'default output handler', ob_get_status()['name'] ); @@ -211,7 +310,6 @@ public function test_buffer_started_once() { $this->assertSame( $level, ob_get_level() ); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); @@ -225,28 +323,28 @@ public function test_empty_buffer_no_output() { ob_start(); $manager->process_template_redirect_content(); $manager->close_buffer(); - $manager->close_final_buffer(); $this->assertSame( '', ob_get_clean() ); $this->assertSame( $this->base_level, ob_get_level() ); } /** - * Output echoed by shutdown callbacks running after close_buffer() is - * captured by the re-armed buffer and still processed. + * Output echoed by shutdown callbacks running after close_buffer() is not + * buffered by us any more: it goes straight to whatever is below. */ - public function test_late_shutdown_output_is_processed() { + public function test_late_shutdown_output_is_not_buffered() { $manager = Optml_Manager::instance(); ob_start(); $manager->process_template_redirect_content(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped - $manager->close_final_buffer(); + + $this->assertSame( $this->base_level + 1, ob_get_level() ); + echo 'late'; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $out = ob_get_clean(); - $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); - $this->assertSame( $this->base_level, ob_get_level() ); + $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); + $this->assertStringEndsWith( 'late', $out ); } /** @@ -284,7 +382,6 @@ public function test_legacy_in_handler_mode() { $manager->process_template_redirect_content(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); diff --git a/tests/test-zz-groovy-menu.php b/tests/test-zz-groovy-menu.php index a6790cc3..7e0e98b0 100644 --- a/tests/test-zz-groovy-menu.php +++ b/tests/test-zz-groovy-menu.php @@ -94,6 +94,8 @@ public function tearDown(): void { remove_action( 'shutdown', 'groovy_menu_pre_shutdown', 0 ); remove_filter( 'groovy_menu_final_output', 'groovy_menu_add_after_body' ); remove_filter( 'optml_captured_page_html', [ $this->compatibility, 'insert_menu' ] ); + remove_filter( 'groovy_menu_final_output', [ $this->compatibility, 'mark_menu_inserted' ], PHP_INT_MAX ); + remove_action( 'shutdown', [ Optml_Manager::instance(), 'close_deferred_buffer' ], 0 ); remove_filter( 'optml_capture_at_shutdown', '__return_false' ); parent::tearDown(); } @@ -135,7 +137,6 @@ public function test_menu_inserted_when_groovy_buffer_is_below_ours() { $this->assertFalse( has_action( 'shutdown', 'groovy_menu_pre_shutdown' ), 'Groovy Menu shutdown step is taken over.' ); - $manager->close_final_buffer(); ob_end_flush(); // Core flushes Groovy Menu's buffer at shutdown. $out = ob_get_clean(); @@ -147,9 +148,11 @@ public function test_menu_inserted_when_groovy_buffer_is_below_ours() { } /** - * Groovy Menu buffer stacked above ours is flushed through, and the menu is still inserted. + * Groovy Menu buffer stacked above ours (our buffer opened on init, e.g. with + * SiteGround Optimizer): its own shutdown step runs first, the menu is + * inserted once and then optimized by the deferred capture. */ - public function test_menu_inserted_when_groovy_buffer_is_above_ours() { + public function test_menu_inserted_once_when_groovy_buffer_is_above_ours() { $this->compatibility->register(); $manager = Optml_Manager::instance(); ob_start(); @@ -157,10 +160,14 @@ public function test_menu_inserted_when_groovy_buffer_is_above_ours() { ob_start(); // Groovy Menu's buffer opened after ours. echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - $manager->close_final_buffer(); + + $this->assertSame( 0, has_action( 'shutdown', 'groovy_menu_pre_shutdown' ), 'Groovy Menu keeps its shutdown step while the capture is deferred.' ); + groovy_menu_pre_shutdown(); // Groovy Menu at shutdown priority 0. + + $manager->close_deferred_buffer(); $out = ob_get_clean(); - $this->assertFalse( has_action( 'shutdown', 'groovy_menu_pre_shutdown' ) ); + $this->assertSame( 1, $GLOBALS['gm_test_shutdown_calls'] ); $this->assertSame( 1, substr_count( $out, 'gm-navbar' ) ); $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); $this->assertSame( $this->base_level, ob_get_level() ); @@ -202,7 +209,6 @@ public function test_early_flush_keeps_groovy_shutdown_step() { echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ob_end_flush(); // Third-party force flush of our buffer. $manager->close_buffer(); - $manager->close_final_buffer(); $this->assertSame( 0, has_action( 'shutdown', 'groovy_menu_pre_shutdown' ) ); From 55ecbb301a6f6158fb6e47e38a39644bb495498b Mon Sep 17 00:00:00 2001 From: selul Date: Mon, 21 Sep 2026 14:02:52 +0300 Subject: [PATCH 6/7] Revert "fix: leave third-party output buffers usable at shutdown" This reverts commit ca34eb5218a3b7622c47d79dab0e87aa52bb9442. --- inc/compatibilities/groovy_menu.php | 43 ++------- inc/manager.php | 120 ++++++++----------------- tests/test-zz-buffer.php | 133 ++++------------------------ tests/test-zz-groovy-menu.php | 18 ++-- 4 files changed, 70 insertions(+), 244 deletions(-) diff --git a/inc/compatibilities/groovy_menu.php b/inc/compatibilities/groovy_menu.php index 2eee5e6f..2e61cf01 100644 --- a/inc/compatibilities/groovy_menu.php +++ b/inc/compatibilities/groovy_menu.php @@ -4,16 +4,15 @@ * * @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 . + * on top to insert the menu markup after . 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 and drops the menu. * - * When its buffer sits below ours, we capture and process the page at the start - * of shutdown. We apply Groovy Menu's final-output filter to that page first, - * before image replacement, and unhook its own shutdown step at that moment, so - * the menu images are optimized too. When its buffer sits above ours the capture - * is deferred, Groovy Menu's own shutdown step runs first and we only make sure - * the menu is not inserted twice. When our capture does not run at all (legacy - * `optml_capture_at_shutdown` mode, or a third-party flush of our buffer) Groovy - * Menu keeps its own shutdown step. + * 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. */ class Optml_groovy_menu extends Optml_compatibility { /** @@ -26,13 +25,6 @@ class Optml_groovy_menu extends Optml_compatibility { */ const GROOVY_OUTPUT_FILTER = 'groovy_menu_final_output'; - /** - * Whether Groovy Menu's final-output filter already ran for this request. - * - * @var bool - */ - private $menu_inserted = false; - /** * Should we load the integration logic. * @@ -50,23 +42,6 @@ public function should_load() { */ public function register() { add_filter( 'optml_captured_page_html', [ $this, 'insert_menu' ] ); - add_filter( self::GROOVY_OUTPUT_FILTER, [ $this, 'mark_menu_inserted' ], PHP_INT_MAX ); // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Groovy Menu's own filter. - } - - /** - * Remember that Groovy Menu's final-output filter already ran for this request. - * - * When third-party buffers sit above ours the capture is deferred, and Groovy - * Menu's own shutdown step runs first. The menu must not be inserted twice. - * - * @param string $html The page HTML. - * - * @return string - */ - public function mark_menu_inserted( $html ) { - $this->menu_inserted = true; - - return $html; } /** @@ -78,7 +53,7 @@ public function mark_menu_inserted( $html ) { */ public function insert_menu( $html ) { $priority = has_action( 'shutdown', self::GROOVY_SHUTDOWN_CALLBACK ); - if ( $priority === false || $this->menu_inserted ) { + if ( $priority === false ) { return $html; } // Our capture ran, so Groovy Menu must not ob_get_clean() the re-armed empty buffer afterwards. diff --git a/inc/manager.php b/inc/manager.php index 5da44c72..6de2afa6 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -427,6 +427,7 @@ public function register_hooks() { add_action( 'template_redirect', [ $this, 'register_after_setup' ] ); add_action( 'rest_api_init', [ $this, 'process_template_redirect_content' ], PHP_INT_MIN ); add_action( 'shutdown', [ $this, 'close_buffer' ], PHP_INT_MIN ); + add_action( 'shutdown', [ $this, 'close_final_buffer' ], PHP_INT_MAX ); foreach ( self::$loaded_compatibilities as $registered_compatibility ) { $registered_compatibility->register(); } @@ -922,7 +923,7 @@ public function process_template_redirect_content() { * * The attached handler is only a fallback for buffers flushed outside of * close_buffer() — third-party force-flush loops, ob_flush() streaming, or - * a request that exits before our shutdown capture runs. A named method + * 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']. * @@ -995,78 +996,6 @@ public function close_buffer() { return; } - /* - * Buffers stacked above ours belong to code that may still need them at - * shutdown: FacetWP, Groovy Menu and others open a buffer early and call - * ob_get_clean() on shutdown priority 0. Force-flushing those buffers now - * would leave them with nothing, so we wait until the priority 0 callbacks - * are done. Registering during the hook run places us after every callback - * already queued on priority 0, and still ahead of core's - * wp_ob_end_flush_all() on priority 1. - */ - if ( $this->has_foreign_buffers_above() ) { - add_action( 'shutdown', [ $this, 'close_deferred_buffer' ], 0 ); - return; - } - - $this->flush_and_capture_buffer(); - } - - /** - * Close the buffer after the shutdown priority 0 callbacks, when third-party buffers were stacked above ours. - * - * @return void - */ - public function close_deferred_buffer() { - if ( ! self::$ob_started ) { - return; - } - $this->flush_and_capture_buffer(); - } - - /** - * The output handlers that may sit above our buffer without delaying the capture. - * - * Core's template enhancement buffer (WordPress 6.9+) is opened on every - * front-end request after ours and nothing reads it back at shutdown. - */ - const FLUSHABLE_OB_HANDLERS = [ 'wp_finalize_template_enhancement_output_buffer' ]; - - /** - * Check whether third-party output buffers are stacked above ours. - * - * @return bool - */ - private function has_foreign_buffers_above() { - if ( self::$ob_level === 0 || ob_get_level() <= self::$ob_level ) { - return false; - } - /** - * Filters the output handler names that can be flushed at the start of - * shutdown without delaying the page capture. - * - * @param string[] $handlers Output handler names, as reported by ob_get_status(). - */ - $flushable = (array) apply_filters( 'optml_flushable_ob_handlers', self::FLUSHABLE_OB_HANDLERS ); - foreach ( array_slice( ob_get_status( true ), self::$ob_level ) as $buffer ) { - if ( ! in_array( $buffer['name'] ?? '', $flushable, true ) ) { - return true; - } - } - - return false; - } - - /** - * Flush the buffers stacked above ours, then capture and process our own. - * - * No buffer is opened afterwards: code that started buffering before us and - * reads its buffer back later in shutdown must find its own buffer on top, - * holding the processed page, not an empty one of ours. - * - * @return void - */ - private function flush_and_capture_buffer() { /* * Flush the buffers other plugins stacked on top of ours so their * handlers still transform the page before we process it, preserving @@ -1081,7 +1010,27 @@ private function flush_and_capture_buffer() { if ( ! $this->capture_and_process_buffer() ) { do_action( 'optml_log', 'Optimole buffer was closed earlier by third-party code.' ); + return; } + + /* + * Re-arm the capture so output echoed by later shutdown callbacks is + * still processed and unguarded third-party flush calls find a buffer + * to close instead of raising a notice. + */ + $this->start_capture_buffer(); + } + + /** + * Close the re-armed buffer at the very end of shutdown. + * + * @return void + */ + public function close_final_buffer() { + if ( ! self::$ob_started ) { + return; + } + $this->capture_and_process_buffer( false ); } /** @@ -1091,9 +1040,11 @@ private function flush_and_capture_buffer() { * buffer another plugin opened at the same level after ours was closed is * never captured or closed by us. * + * @param bool $is_page Whether this is the page capture (true) or the late shutdown output (false). + * * @return bool Whether our buffer was found and consumed. */ - private function capture_and_process_buffer() { + private function capture_and_process_buffer( $is_page = true ) { if ( self::$ob_level === 0 || ob_get_level() !== self::$ob_level ) { return false; } @@ -1106,15 +1057,18 @@ private function capture_and_process_buffer() { self::$ob_processed = true; ob_end_clean(); if ( $html !== false && $html !== '' ) { - /** - * Filters the captured page HTML before Optimole processes it. - * - * Runs once per request, on the buffer captured at shutdown, outside of - * PHP's display-handler context. - * - * @param string $html The full page HTML. - */ - $html = apply_filters( 'optml_captured_page_html', $html ); + if ( $is_page ) { + /** + * Filters the captured page HTML before Optimole processes it. + * + * Runs once per request, on the buffer captured at shutdown, outside of + * PHP's display-handler context. Late output echoed by other shutdown + * callbacks is not passed through this filter. + * + * @param string $html The full page HTML. + */ + $html = apply_filters( 'optml_captured_page_html', $html ); + } echo $this->replace_content( $html, self::is_ajax_request() ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- full page HTML, escaping would break the page. } return true; diff --git a/tests/test-zz-buffer.php b/tests/test-zz-buffer.php index e105eaba..15425d5c 100644 --- a/tests/test-zz-buffer.php +++ b/tests/test-zz-buffer.php @@ -12,19 +12,6 @@ * @license http://opensource.org/licenses/gpl-2.0.php GNU Public License */ -if ( ! function_exists( 'optml_test_flushable_handler' ) ) { - /** - * Pass-through output handler with a stable name for ob_get_status(). - * - * @param string $content Buffer content. - * - * @return string - */ - function optml_test_flushable_handler( $content ) { - return $content; - } -} - /** * Class Test_Buffer. */ @@ -66,7 +53,6 @@ public function tearDown(): void { } } $this->reset_buffer_state(); - remove_action( 'shutdown', [ Optml_Manager::instance(), 'close_deferred_buffer' ], 0 ); parent::tearDown(); } @@ -108,6 +94,7 @@ function ( $html ) use ( &$probed ) { $manager->process_template_redirect_content(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertSame( 1, $probed ); @@ -116,11 +103,10 @@ function ( $html ) use ( &$probed ) { } /** - * A buffer another plugin stacks on top of ours is not force-flushed at the - * start of shutdown. The capture waits for the priority 0 callbacks, then the - * foreign handler runs first and we process its transformed output. + * A buffer another plugin stacks on top of ours is flushed through its own + * handler first, and we process its transformed output — never swallow it. */ - public function test_foreign_buffer_above_defers_capture() { + public function test_foreign_buffer_above_is_flushed_first() { $manager = Optml_Manager::instance(); ob_start(); $manager->process_template_redirect_content(); @@ -130,14 +116,9 @@ function ( $content ) { return $content . ''; } ); - $foreign_level = ob_get_level(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - - $this->assertSame( $foreign_level, ob_get_level(), 'The foreign buffer is left alone at the start of shutdown.' ); - $this->assertSame( 0, has_action( 'shutdown', [ $manager, 'close_deferred_buffer' ] ) ); - - $manager->close_deferred_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); // Both the page image and the one appended by the foreign handler are optimized. @@ -146,89 +127,6 @@ function ( $content ) { $this->assertSame( $this->base_level, ob_get_level() ); } - /** - * Code that opened a buffer after ours and reads it back on shutdown - * priority 0 (FacetWP, Groovy Menu with an early Optimole buffer) still - * finds its own buffer, and what it echoes back is processed. - */ - public function test_foreign_buffer_above_can_be_read_back_at_shutdown() { - $manager = Optml_Manager::instance(); - ob_start(); - $manager->process_template_redirect_content(); - ob_start(); // Third-party buffer opened after ours. - echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped - $manager->close_buffer(); - - // The third party reads its buffer back on shutdown priority 0. - $page = ob_get_clean(); - $this->assertStringContainsString( 'wp-custom-header', $page ); - echo '' . $page; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped - - $manager->close_deferred_buffer(); - $out = ob_get_clean(); - - $this->assertStringContainsString( '', $out ); - $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); - $this->assertSame( $this->base_level, ob_get_level() ); - } - - /** - * Code that opened a buffer before ours and reads it back on shutdown - * priority 0 finds its own buffer on top, holding the processed page. We - * leave no empty buffer of ours above it. - */ - public function test_foreign_buffer_below_receives_processed_page() { - $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(), 'No buffer of ours is left above the foreign one.' ); - $this->assertFalse( has_action( 'shutdown', [ $manager, 'close_deferred_buffer' ] ) ); - - // The third party reads its buffer back on shutdown priority 0. - $page = ob_get_clean(); - $this->assertSame( 1, substr_count( $page, 'i.optimole.com' ) ); - $this->assertSame( '', ob_get_clean() ); - $this->assertSame( $this->base_level, ob_get_level() ); - } - - /** - * Handlers listed as flushable, like core's template enhancement buffer, - * do not delay the capture. - */ - public function test_flushable_buffer_above_does_not_defer_capture() { - $manager = Optml_Manager::instance(); - add_filter( 'optml_flushable_ob_handlers', [ $this, 'allow_test_handler' ] ); - ob_start(); - $manager->process_template_redirect_content(); - ob_start( 'optml_test_flushable_handler' ); - echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped - $manager->close_buffer(); - remove_filter( 'optml_flushable_ob_handlers', [ $this, 'allow_test_handler' ] ); - - $this->assertFalse( has_action( 'shutdown', [ $manager, 'close_deferred_buffer' ] ) ); - $out = ob_get_clean(); - $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); - $this->assertSame( $this->base_level, ob_get_level() ); - } - - /** - * Mark the test handler as flushable. - * - * @param string[] $handlers Handler names. - * - * @return string[] - */ - public function allow_test_handler( $handlers ) { - $handlers[] = 'optml_test_flushable_handler'; - - return $handlers; - } - /** * When third-party code force-flushes our buffer before shutdown, the * content is passed through UNPROCESSED: running the filter graph inside a @@ -243,6 +141,7 @@ public function test_third_party_flush_passes_content_through() { ob_end_flush(); // Third-party force flush of our buffer. $this->assertSame( $this->base_level + 1, ob_get_level() ); $manager->close_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertStringNotContainsString( 'i.optimole.com', $out ); @@ -272,6 +171,7 @@ function ( $html ) { echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ob_end_flush(); // Would exit(255) if the handler ran the filter graph. $manager->close_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertStringContainsString( 'themes/twentyseventeen/assets/images/header.jpg', $out ); @@ -290,6 +190,7 @@ public function test_foreign_buffer_at_same_level_is_not_consumed() { ob_start(); // ...and opens its own at the same level. echo 'FOREIGN'; $manager->close_buffer(); + $manager->close_final_buffer(); $this->assertSame( $this->base_level + 1, ob_get_level() ); $this->assertSame( 'default output handler', ob_get_status()['name'] ); @@ -310,6 +211,7 @@ public function test_buffer_started_once() { $this->assertSame( $level, ob_get_level() ); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); @@ -323,28 +225,28 @@ public function test_empty_buffer_no_output() { ob_start(); $manager->process_template_redirect_content(); $manager->close_buffer(); + $manager->close_final_buffer(); $this->assertSame( '', ob_get_clean() ); $this->assertSame( $this->base_level, ob_get_level() ); } /** - * Output echoed by shutdown callbacks running after close_buffer() is not - * buffered by us any more: it goes straight to whatever is below. + * Output echoed by shutdown callbacks running after close_buffer() is + * captured by the re-armed buffer and still processed. */ - public function test_late_shutdown_output_is_not_buffered() { + public function test_late_shutdown_output_is_processed() { $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() ); - echo 'late'; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_final_buffer(); $out = ob_get_clean(); - $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); - $this->assertStringEndsWith( 'late', $out ); + $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); } /** @@ -382,6 +284,7 @@ public function test_legacy_in_handler_mode() { $manager->process_template_redirect_content(); echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); $this->assertSame( 1, substr_count( $out, 'i.optimole.com' ) ); diff --git a/tests/test-zz-groovy-menu.php b/tests/test-zz-groovy-menu.php index 7e0e98b0..a6790cc3 100644 --- a/tests/test-zz-groovy-menu.php +++ b/tests/test-zz-groovy-menu.php @@ -94,8 +94,6 @@ public function tearDown(): void { remove_action( 'shutdown', 'groovy_menu_pre_shutdown', 0 ); remove_filter( 'groovy_menu_final_output', 'groovy_menu_add_after_body' ); remove_filter( 'optml_captured_page_html', [ $this->compatibility, 'insert_menu' ] ); - remove_filter( 'groovy_menu_final_output', [ $this->compatibility, 'mark_menu_inserted' ], PHP_INT_MAX ); - remove_action( 'shutdown', [ Optml_Manager::instance(), 'close_deferred_buffer' ], 0 ); remove_filter( 'optml_capture_at_shutdown', '__return_false' ); parent::tearDown(); } @@ -137,6 +135,7 @@ public function test_menu_inserted_when_groovy_buffer_is_below_ours() { $this->assertFalse( has_action( 'shutdown', 'groovy_menu_pre_shutdown' ), 'Groovy Menu shutdown step is taken over.' ); + $manager->close_final_buffer(); ob_end_flush(); // Core flushes Groovy Menu's buffer at shutdown. $out = ob_get_clean(); @@ -148,11 +147,9 @@ public function test_menu_inserted_when_groovy_buffer_is_below_ours() { } /** - * Groovy Menu buffer stacked above ours (our buffer opened on init, e.g. with - * SiteGround Optimizer): its own shutdown step runs first, the menu is - * inserted once and then optimized by the deferred capture. + * Groovy Menu buffer stacked above ours is flushed through, and the menu is still inserted. */ - public function test_menu_inserted_once_when_groovy_buffer_is_above_ours() { + public function test_menu_inserted_when_groovy_buffer_is_above_ours() { $this->compatibility->register(); $manager = Optml_Manager::instance(); ob_start(); @@ -160,14 +157,10 @@ public function test_menu_inserted_once_when_groovy_buffer_is_above_ours() { ob_start(); // Groovy Menu's buffer opened after ours. echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped $manager->close_buffer(); - - $this->assertSame( 0, has_action( 'shutdown', 'groovy_menu_pre_shutdown' ), 'Groovy Menu keeps its shutdown step while the capture is deferred.' ); - groovy_menu_pre_shutdown(); // Groovy Menu at shutdown priority 0. - - $manager->close_deferred_buffer(); + $manager->close_final_buffer(); $out = ob_get_clean(); - $this->assertSame( 1, $GLOBALS['gm_test_shutdown_calls'] ); + $this->assertFalse( has_action( 'shutdown', 'groovy_menu_pre_shutdown' ) ); $this->assertSame( 1, substr_count( $out, 'gm-navbar' ) ); $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); $this->assertSame( $this->base_level, ob_get_level() ); @@ -209,6 +202,7 @@ public function test_early_flush_keeps_groovy_shutdown_step() { echo self::PAGE; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ob_end_flush(); // Third-party force flush of our buffer. $manager->close_buffer(); + $manager->close_final_buffer(); $this->assertSame( 0, has_action( 'shutdown', 'groovy_menu_pre_shutdown' ) ); From d844ce28b008266c4560928260126954858e17d0 Mon Sep 17 00:00:00 2001 From: selul Date: Mon, 21 Sep 2026 14:07:42 +0300 Subject: [PATCH 7/7] fix: process the page inside the output handler by default again The shutdown capture added in 4.2.12 flushes the buffers stacked above ours and opens a new buffer afterwards. Code that opens a buffer early and reads it back with ob_get_clean() on shutdown priority 0 (FacetWP refresh, Groovy Menu) received an empty string. Restore the 4.2.11 in-handler processing as the default and keep the shutdown capture as an opt-in through optml_capture_at_shutdown. An exception thrown during the replacement still never breaks the page. Fixes #1156 Co-Authored-By: Claude Fable 5.1 --- inc/compatibilities/groovy_menu.php | 15 +++--- inc/manager.php | 74 ++++++++++++++++++----------- tests/test-zz-buffer.php | 70 +++++++++++++++++++++++++++ tests/test-zz-groovy-menu.php | 10 ++-- 4 files changed, 129 insertions(+), 40 deletions(-) diff --git a/inc/compatibilities/groovy_menu.php b/inc/compatibilities/groovy_menu.php index 2e61cf01..34fe0c83 100644 --- a/inc/compatibilities/groovy_menu.php +++ b/inc/compatibilities/groovy_menu.php @@ -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 . 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 and drops the menu. + * on top to insert the menu markup after . * - * 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 { /** diff --git a/inc/manager.php b/inc/manager.php index 6de2afa6..587fe756 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -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 */ @@ -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). @@ -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 ) { @@ -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(); } @@ -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 ); diff --git a/tests/test-zz-buffer.php b/tests/test-zz-buffer.php index 15425d5c..a7f8e2fa 100644 --- a/tests/test-zz-buffer.php +++ b/tests/test-zz-buffer.php @@ -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(); } @@ -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(); } @@ -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 ); + } } diff --git a/tests/test-zz-groovy-menu.php b/tests/test-zz-groovy-menu.php index a6790cc3..0ea900dc 100644 --- a/tests/test-zz-groovy-menu.php +++ b/tests/test-zz-groovy-menu.php @@ -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(); } @@ -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(); } @@ -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();