diff --git a/inc/manager.php b/inc/manager.php index ffb713c7..0e9a34ce 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -122,6 +122,23 @@ final class Optml_Manager { * @var boolean Buffer state. */ private static $ob_started = false; + /** + * The output-buffer nesting level of our capture buffer. + * + * Used to make sure we only ever capture or close our own buffer and not + * one started by a third party. + * + * @var int Buffer nesting level, 0 when no capture buffer is armed. + */ + private static $ob_level = 0; + /** + * Whether the captured buffer was already processed at shutdown. + * + * When true, the fallback output handler passes content through untouched. + * + * @var boolean Processed state. + */ + private static $ob_processed = false; /** * Class instance method. @@ -408,6 +425,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(); } @@ -780,13 +798,61 @@ function ( $url ) use ( $upload_resource ) { $urls ); + /* + * Replace all URLs in a single pass per chunk instead of one full-page + * preg_replace() per URL, which scanned and rebuilt the whole page for + * every replaced URL. Chunks are bounded by pattern size, not only + * count, so the compiled regex stays within PCRE's ~64KB limit even + * for very long URLs (e.g. signed CDN URLs with kilobyte-sized query + * strings). Each chunk is applied as soon as it fills, so only one + * chunk's bookkeeping is in memory at a time. + */ + $chunk = []; + $quoted = []; + $quoted_size = 0; foreach ( $urls as $origin => $replace ) { - $html = preg_replace( '/(?= 200 || $quoted_size + strlen( $quoted_origin ) > 24000 ) ) { + $html = $this->replace_urls_chunk( $html, $chunk, $quoted ); + $chunk = []; + $quoted = []; + $quoted_size = 0; + } + $chunk[ $origin ] = $replace; + $quoted[] = $quoted_origin; + $quoted_size += strlen( $quoted_origin ) + 1; + } + if ( ! empty( $chunk ) ) { + $html = $this->replace_urls_chunk( $html, $chunk, $quoted ); } return $html; } + /** + * Replace one chunk of URLs in the content with a single combined pattern. + * + * @param string $html Content to process. + * @param array $chunk Map of origin => replacement URLs. + * @param string[] $quoted The preg_quote()d origins, in the same order. + * + * @return string Processed content, unchanged when the pattern fails. + */ + private function replace_urls_chunk( $html, $chunk, $quoted ) { + $result = preg_replace_callback( + '/(?start_capture_buffer(); + } + + /** + * Start an output buffer that captures 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". + * + * 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']. + * + * @return void + */ + private function start_capture_buffer() { + self::$ob_processed = false; + ob_start( [ $this, 'handle_buffer_fallback' ] ); + self::$ob_level = ob_get_level(); + } + + /** + * The handler name PHP reports for our capture buffer in ob_get_status(). + */ + const OB_HANDLER_NAME = 'Optml_Manager::handle_buffer_fallback'; + + /** + * Output-buffer handler attached to our capture 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. + * + * @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). + * + * @return string The content to output. + */ + public function handle_buffer_fallback( $content, $phase = 0 ) { + if ( self::$ob_processed || $content === '' ) { + return $content; + } + if ( apply_filters( 'optml_capture_at_shutdown', true ) === false ) { + try { return $this->replace_content( $content, self::is_ajax_request() ); + } catch ( Throwable $t ) { + // Never break the page from inside a display handler. + do_action( 'optml_log', 'replace_content failed inside the output handler: ' . $t->getMessage() ); + return $content; } - ); + } + do_action( 'optml_log', 'Optimole buffer was flushed outside close_buffer(); content passed through unprocessed.' ); + + return $content; } /** * Close the buffer and flush the content. */ public function close_buffer() { - if ( self::$ob_started && ob_get_length() ) { - ob_end_flush(); + if ( ! self::$ob_started ) { + 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 ( ob_get_length() ) { + ob_end_flush(); + } + return; + } + + /* + * 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.' ); + 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(); + } + + /** + * Capture our buffer, process it outside the display-handler context and echo the result. + * + * Ownership is verified by both nesting level and handler identity, so a + * buffer another plugin opened at the same level after ours was closed is + * never captured or closed by us. + * + * @return bool Whether our buffer was found and consumed. + */ + private function capture_and_process_buffer() { + if ( self::$ob_level === 0 || ob_get_level() !== self::$ob_level ) { + return false; + } + $status = ob_get_status(); + if ( ( $status['name'] ?? '' ) !== self::OB_HANDLER_NAME ) { + return false; } + $html = ob_get_contents(); + // Set before ob_end_clean() so our handler no-ops during buffer cleanup. + self::$ob_processed = true; + ob_end_clean(); + if ( $html !== false && $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; } /** * Throw error on object clone diff --git a/tests/test-zz-buffer.php b/tests/test-zz-buffer.php new file mode 100644 index 00000000..15425d5c --- /dev/null +++ b/tests/test-zz-buffer.php @@ -0,0 +1,293 @@ +Test '; + + /** + * The output-buffer nesting level before each test. + * + * @var int + */ + private $base_level = 0; + + public function setUp(): void { + parent::setUp(); + $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(); + + $this->reset_buffer_state(); + $this->base_level = ob_get_level(); + } + + public function tearDown(): void { + // Make any leftover capture handler a pass-through before cleaning up. + $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(); + 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 ); + } + } + + /** + * Callbacks on our filters may use output buffering without fataling. + * + * Before processing moved outside the display handler, the nested + * ob_start() below crashed with "Cannot use output buffering in output + * buffering display handlers". + */ + public function test_filter_callbacks_can_use_output_buffering() { + $manager = Optml_Manager::instance(); + $probed = 0; + add_filter( + 'optml_url_pre_process', + function ( $html ) use ( &$probed ) { + ob_start(); + echo 'probe'; + ob_get_clean(); + $probed ++; + return $html; + } + ); + ob_start(); + $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 ); + $this->assertStringContainsString( 'i.optimole.com', $out ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * 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_is_flushed_first() { + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + // Third-party handler started after ours, e.g. a minifier. + ob_start( + function ( $content ) { + return $content . ''; + } + ); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_buffer(); + $manager->close_final_buffer(); + $out = ob_get_clean(); + + // Both the page image and the one appended by the foreign handler are optimized. + $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); + $this->assertStringNotContainsString( '"http://example.org/wp-content/uploads/foreign.jpg', $out ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * When third-party code force-flushes our buffer before shutdown, the + * content is passed through UNPROCESSED: running the filter graph inside a + * display handler would make any third-party ob_*() call an uncatchable + * fatal. close_buffer() detects the loss without side effects. + */ + public function test_third_party_flush_passes_content_through() { + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + 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 ); + $this->assertStringContainsString( 'themes/twentyseventeen/assets/images/header.jpg', $out ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * A third-party flush combined with an output-buffering filter callback + * must not fatal. Processing inside the handler would terminate PHP with + * "Cannot use output buffering in output buffering display handlers", + * which catch ( Throwable ) cannot intercept. + */ + public function test_third_party_flush_with_ob_filter_does_not_fatal() { + $manager = Optml_Manager::instance(); + add_filter( + 'optml_url_pre_process', + function ( $html ) { + ob_start(); + echo 'probe'; + ob_get_clean(); + return $html; + } + ); + ob_start(); + $manager->process_template_redirect_content(); + 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 ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * A foreign buffer that ends up at our recorded nesting level is never + * captured or closed: ownership requires our handler identity, not just + * the level. + */ + public function test_foreign_buffer_at_same_level_is_not_consumed() { + $manager = Optml_Manager::instance(); + $manager->process_template_redirect_content(); + ob_end_clean(); // Third party discards our buffer... + 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'] ); + $this->assertStringContainsString( 'FOREIGN', ob_get_contents() ); + ob_end_clean(); + } + + /** + * Calling process_template_redirect_content() twice must not stack a + * second buffer, and the page is processed exactly once. + */ + public function test_buffer_started_once() { + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + $level = ob_get_level(); + $manager->process_template_redirect_content(); + $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' ) ); + } + + /** + * An empty buffer closes without output or errors. + */ + public function test_empty_buffer_no_output() { + $manager = Optml_Manager::instance(); + 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. + */ + 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(); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_final_buffer(); + $out = ob_get_clean(); + + $this->assertSame( 2, substr_count( $out, 'i.optimole.com' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * Very long URLs (e.g. signed CDN URLs) must not push a replacement + * chunk's compiled pattern over PCRE's size limit. + */ + public function test_long_url_replacement_stays_within_pcre_limits() { + $manager = Optml_Manager::instance(); + add_filter( + 'optml_content_url', + function ( $url ) { + return 'https://replaced.test/marker'; + } + ); + $urls = []; + $html = ''; + for ( $i = 0; $i < 250; $i ++ ) { + $url = 'https://example.org/image-' . $i . '.jpg?X-Signature=' . str_repeat( 'a1b2c3d4', 180 ) . '&i=' . $i; + $urls[] = $url; + $html .= ''; + } + $out = $manager->do_url_replacement( $html, $urls ); + + $this->assertSame( 250, substr_count( $out, 'https://replaced.test/marker' ) ); + $this->assertStringNotContainsString( 'X-Signature', $out ); + } + + /** + * The optml_capture_at_shutdown filter restores the legacy in-handler flow. + */ + public function test_legacy_in_handler_mode() { + add_filter( 'optml_capture_at_shutdown', '__return_false' ); + $manager = Optml_Manager::instance(); + ob_start(); + $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' ) ); + $this->assertSame( $this->base_level, ob_get_level() ); + } +}