Skip to content

Template: close the output buffer when a template throws (→ 3.3.1) - #8

Open
daedeloth wants to merge 1 commit into
masterfrom
fix/template-close-buffer-on-throw
Open

daedeloth wants to merge 1 commit into
masterfrom
fix/template-close-buffer-on-throw

Conversation

@daedeloth

Copy link
Copy Markdown
Member

Problem

Template::parse(), template() and combine() do ob_start(); include …; ob_get_contents(); ob_end_clean(); with nothing around the include. If the template throws, the buffer stays open and swallows whatever the caller outputs afterwards.

On PHP 8.x this happens a lot more, because deprecations in templates (trim(null) and the like) become ErrorExceptions in apps that fail on them. Under PHPUnit 9 it snowballs. stopOutputBuffering() sees the unbalanced level, cleans down, and throws "risky" before clearing outputBufferingActive, so getActualOutput() returns the whole enclosing buffer and the printer writes it back into that buffer. In the accounts integration suite on PHP 8.5, the output doubled with every failing page until PHPUnit ran out of memory (4.7 GB allocation), so the 8.5 suite couldn't report anything.

Change

Wrap the include in try { … } catch (\Throwable $ctlbtmplterror) { ob_end_clean(); throw $ctlbtmplterror; } in all three methods. The success path is unchanged. The variable follows the file's $ctlbtmplt… convention because it lives in the template's variable scope. It stays within the PHP 7.4 minimum (php -l on 7.4).

Tests

TemplateOutputBufferTest: a throwing template reached via parse(), via $this->template() and via $this->combine() leaves ob_get_level() unchanged, and normal rendering still returns its output.

  • Old Template.php: 3 failures (and PHPUnit flags them risky for unclosed buffers).
  • New: 4 OK. Full suite --exclude-group=database: 203 OK on PHP 8.5.

Effect on accounts (PHP 8.5, this file mounted over vendor/)

Before: the integration run died with an out-of-memory error. After: 776 tests run, 617 pass. The 159 failures are real 8.5 findings in the app and vendors.

Release as v3.3.1 (patch).

🤖 Generated with Claude Code

Template::parse(), template() and combine() open an output buffer, include
the template and close the buffer only on success. When the template throws
(e.g. a PHP 8.x deprecation turned into an ErrorException), the buffer stays
open and swallows everything the caller prints afterwards.

Under PHPUnit 9 this snowballs. stopOutputBuffering() sees the unbalanced
level and throws "risky" before clearing outputBufferingActive, so the
printer re-prints the whole enclosing buffer as the test's output. In the
accounts integration suite on PHP 8.5 that doubled the output with every
failing page until PHPUnit ran out of memory.

Catch \Throwable around the include, close the buffer and rethrow. The new
variable follows the file's $ctlbtmplt... naming, because it lives in the
template's scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant