fix: process page HTML outside PHP's output-buffer display handler - #1132
Open
selul wants to merge 4 commits into
Open
fix: process page HTML outside PHP's output-buffer display handler#1132selul wants to merge 4 commits into
selul wants to merge 4 commits into
Conversation
Running replace_content() as the ob_start() display handler meant any output-buffering call from third-party code hooked into our filters was a fatal error, and any real fatal during processing (e.g. memory exhaustion) was masked as "Cannot use output buffering in output buffering display handlers" with a misleading crash location. The buffer is now a plain capture: close_buffer() flushes third-party buffers stacked above ours, captures our own by its recorded nesting level (never popping someone else's buffer), processes the HTML in normal execution context and re-arms the capture so late shutdown output is still handled. The attached handler remains only as a fallback that keeps the previous behavior when third-party code flushes our buffer mid-request. Also replaces the per-URL full-page preg_replace() loop with chunked single-pass replacement to reduce peak memory on large pages, the likely trigger of the masked production fatals. Fixes #1126 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
A chunk of 200 very long URLs (e.g. signed CDN URLs with kilobyte-sized query strings) could exceed PCRE's ~64KB compiled-pattern limit, failing the whole chunk and leaving those URLs unreplaced. Chunks now flush when the accumulated quoted pattern reaches 24KB, so compilation always succeeds regardless of URL length, and a failed chunk is logged via optml_log instead of being silently skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Building every chunk's bookkeeping up front held all origin/replacement maps in memory at once, which cost about 1MB extra on pages with thousands of URLs. Each chunk is now applied as soon as it fills, so only one chunk's bookkeeping exists at a time; peak memory is now at or below the old per-URL loop at every scale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
P1: the fallback handler no longer runs replace_content() when a third party flushes our buffer early. The ob-in-handler fatal is an engine E_ERROR that catch (Throwable) cannot intercept, so processing there reintroduced the crash this rework removes; early-flushed content is now passed through unprocessed and logged. Only the explicit legacy mode (optml_capture_at_shutdown false) keeps in-handler processing. P2: buffer ownership is now verified by handler identity, not nesting level alone. The capture buffer uses a named method handler so ob_get_status()['name'] reports Optml_Manager::handle_buffer_fallback, and capture_and_process_buffer() refuses any buffer that does not carry it — a foreign buffer at our recorded level is never consumed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Production sites crashed with
Optml_Manager::replace_content(): Cannot use output buffering in output buffering display handlers(issue #1126). The full-page replacement now runs outside PHP's display-handler context, where nested output buffering is legal and fatal errors keep their real message.What changed
Capture buffer —
process_template_redirect_content()starts a capture buffer whose handler is a named method (handle_buffer_fallback). When a third party flushes the buffer early, the handler passes the content through unprocessed and logs it: running the filter graph inside a PHP display handler would turn any third-partyob_*()call into an uncatchable engine fatal — the crash this rework removes. Only the legacy mode keeps in-handler processing.close_buffer()— flushes third-party buffers stacked above ours, captures our own buffer, processes the HTML in normal execution context, and echoes the result.Buffer ownership — before:
ob_end_flush()popped whichever buffer was on top. After: we only consume the buffer we started, verified by nesting level and handler identity (ob_get_status()['name']must reportOptml_Manager::handle_buffer_fallback), so a foreign buffer at our recorded level is never captured or closed.close_final_buffer()— a re-armed buffer captures output echoed by later shutdown callbacks and processes it atshutdownpriorityPHP_INT_MAX.do_url_replacement()— replaces extracted URLs in one pass per size-bounded chunk instead of one full-pagepreg_replace()per URL. Benchmark on a 1.2 MB page: 300 URLs in 4 ms instead of 77 ms, 900 URLs in 13 ms instead of 216 ms. Peak memory stays the same; the win is CPU time and allocation churn. Chunks flush at 200 URLs or 24KB of quoted pattern, so the compiled regex stays within PCRE's ~64KB limit even for kilobyte-long signed CDN URLs.Escape hatch — return
falsefrom the newoptml_capture_at_shutdownfilter to restore the legacy in-handler flow.Note
The crash location in the telemetry (
FormatProperty.php:1) was an artifact. PHP masks a fatal error inside a display handler with the "Cannot use output buffering" message and an unrelated location. After this change, such fatals report their real message and location.Shutdown flow
flowchart LR A[Changed:<br/>page renders into<br/>plain capture buffer]:::changed --> B[shutdown:<br/>close_buffer] B --> C[New:<br/>flush third-party<br/>buffers above]:::added C --> D{New:<br/>our buffer<br/>intact?}:::added D -- Yes --> E[Changed:<br/>process HTML<br/>outside handler]:::changed E --> F[Echo optimized page] F --> G[New:<br/>re-arm for late<br/>shutdown output]:::added D -- No --> H[Stand down:<br/>early-flushed content<br/>passed through, logged] classDef added fill:#1a7f37,color:#fff,stroke:#116329,stroke-width:3px classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3QA
Connect the site in
WP Admin → Media → Optimole. Createwp-content/mu-plugins/ob-probe.phpwith:Open any frontend page.
Expect: the page renders, image URLs point to
i.optimole.com, andwp-content/debug.logcontains noCannot use output bufferingfatal. Without this fix, the page terminates with that fatal.Remove the mu-plugin. Add
add_filter( 'optml_capture_at_shutdown', '__return_false' );to the theme'sfunctions.php. Open a frontend page and view the source.Expect: image URLs still point to
i.optimole.com(legacy in-handler flow).Remove the filter. Install and activate TranslatePress. Translate a page that contains images and open the translated page.
Expect: the translated text and
i.optimole.comimage URLs appear together.🤖 Generated with Claude Code