Repository navigation
Preserve caller-owned output buffers when a 2.x renderer closes capture #112
Description
Delivery target and current plan
2.x maintenance — implemented and independently reviewed in PR #114, awaiting owner review. Native PHP CI passed at e00b98c. The correction preserves the released 2.x fallback, false-like output semantics and PHP range; it does not import the major renderer/lifecycle redesign from PR #3. Migration #113 must convert this PR's patch note if it is pending when release tooling changes. No merge or release has been performed.
Prior investigation, scope and evidence
Summary and released baseline
The released 2.x ThrowableStreamFactory can copy and remove a caller-owned output buffer when its application-supplied renderer closes the factory's capture buffer and then returns normally. Track a focused, compatibility-preserving maintenance correction separately from the major in PR 3.
Confirmed against the exact factory source at v2.5.1, commit 2d4c26d. GitHub resolves the published tag to that commit. The 2.x manifest retains core PHP ^7.4 || ^8.0 and optional FastForward integration.
Reproduction and observed result
Use only synthetic data and a local test/development autoloader supplying the library and Nyholm PSR-7:
<?php
require __DIR__ . '/vendor/autoload.php';
use CoiSA\ErrorHandler\Handler\CallableThrowableHandler;
use CoiSA\ErrorHandler\Http\Message\ThrowableStreamFactory;
use Nyholm\Psr7\Factory\Psr17Factory;
$entryLevel = ob_get_level();
ob_start();
echo 'private caller marker';
$callerLevel = ob_get_level();
$renderer = new CallableThrowableHandler(static function (Throwable $failure): void {
ob_end_clean(); // Closes the factory capture, then returns normally.
});
try {
$stream = (new ThrowableStreamFactory(new Psr17Factory(), $renderer))
->createStreamFromThrowable(new RuntimeException('synthetic'));
$result = [
'body' => (string) $stream,
'callerDepthBefore' => $callerLevel,
'callerDepthAfter' => ob_get_level(),
];
} finally {
while (ob_get_level() > $entryLevel) {
ob_end_clean();
}
}
echo json_encode($result, JSON_THROW_ON_ERROR), "\n";An isolated PHP 8.5.11 process loading that exact 2.x factory source returned exit 0, empty stderr and:
{"body":"private caller marker","callerDepthBefore":1,"callerDepthAfter":0}The factory calls ob_get_clean() after rendering without checking whether any buffer above its entry level still exists. After the renderer closes capture, this operation reads and removes the caller's buffer. The prior exceptional-path cleanup already bounds removal by entry depth; it does not cover this normal-return path.
Expected: do not read, close or invoke the output callback of a caller-owned buffer. If capture has disappeared, follow the existing 2.x fallback contract, leaving caller bytes and depth intact.
Impact and scope
This is a privacy/robustness bug triggered by a misbalanced application callback. Caller bytes could become the generated response body; caller buffer ownership is also lost. No remotely exploitable path, attacker-controlled callback or real secret exposure has been demonstrated. There is no need to inspect production data or broaden the security claim.
The closed issue 24 concerned leaked buffers after a throwing reporter and preserving primary failures. Its delivered cleanup does not repair the distinct normal-return case demonstrated here. The current major uses a separate capture implementation; its minimal depth guard is evidence for the ownership boundary, not a patch to cherry-pick wholesale into 2.x.
Proposed strategy and alternatives
| Alternative | Benefit | Limitation | Assessment |
|---|---|---|---|
| Keep the current normal-return capture | No code change | Caller bytes and buffer can still be consumed | Does not resolve the reproduced defect |
Check depth before ob_get_clean() and retain the 2.x fallback |
Small ownership correction with existing signatures | Needs exact fallback/body and native-buffer regression checks | Recommended investigation |
| Import the major renderer/lifecycle refactor | Broader redesigned boundaries | Changes defaults, dependencies and runtime support | Outside this maintenance issue |
Investigate the smallest guard before the consuming ob_get_clean() call. Preserve the published 2.x fallback (string) $throwable, including its diagnostic nature and existing false-like-output behavior. Do not silently introduce the major's generic body, string-zero correction, required FastForward dependency, report-before-render middleware or constructor/lifecycle API changes. Applications still own the decision to expose legacy diagnostics.
Acceptance criteria
- Add an organized PHPUnit unit regression that first fails on the released implementation: renderer closes capture and returns; a synthetic private caller marker must not enter the stream, and caller bytes/depth remain unchanged. Include a caller output callback observation to ensure factory capture does not execute or consume it prematurely.
- Verify capture-without-a-caller-buffer reaches the established fallback without native missing-buffer warnings. Preserve successful ordinary/nested capture, primary failure identity, reachable bounded cleanup and protected-buffer behavior.
- Assert exact 2.x fallback/body semantics, including the current treatment of empty output and
"0"; a separate behavior change requires separate review. - Keep public signatures, core PHP
^7.4 || ^8.0, optional integrations and dependency constraints intact. Run the retained PHP 7.4 minimum and supported modern 2.x matrix; the current reproduction on PHP 8.5 alone does not prove all-platform compatibility. - Update affected English PHPDoc, guides/site/example guidance and the canonical
coisa-error-handlerconsumer skill alongside implementation. Describe accompanying behavior directly and preserve source/example parity. - Include a patch Changeset, pass the repository's existing quality/production/archive checks and preserve the coverage policy. Do not add a custom runner, fake native functions, relax coverage or import the major's framework/tooling graph.
- Deliver from the current 2.x maintenance baseline in its own focused PR, with exact SHA/check evidence and independent review. No backport implementation is part of filing this issue.
Issue creation does not authorize merge, release/tag publication or Pages deployment.
Activity