From ac030e4cec3ffa174f69878faecbdf8b2d9061b7 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:38:01 +0000 Subject: [PATCH] Increase tracked `ob_get_level()` only in `ob_start()`'s truthy branch instead of unconditionally - Add `ObStartFunctionTypeSpecifyingExtension` that raises the tracked `ob_get_level()` by one only in the branch where `ob_start()` is known to have returned a truthy value (`if (ob_start())`, `if (!ob_start()) { return; }`, `if (ob_start() === false) { return; }`, ...). An unchecked/bare `ob_start()` no longer assumes an active buffer, so `ob_get_contents()`, `ob_get_clean()`, `ob_get_flush()` and `ob_get_length()` keep `false`/`int|false` in their return type and comparisons like `ob_get_clean() === false` are no longer reported as always-false. - `OutputBufferHelper::opensBufferOnSuccess()` marks the level-incrementing functions; `FuncCallHandler` skips the unconditional level increase for them (the extension handles the truthy branch). Level-decrementing functions keep their unconditional behavior - assuming a close succeeded only reduces narrowing, so it cannot introduce false positives. - The fix covers the whole `ob_get_*()` family at once, since all of them derive their non-`false` narrowing from the tracked buffer level. - Update `output-buffering.php` to establish the active buffer with a checked `ob_start()`, and add `bug-14985.php`. --- src/Analyser/ExprHandler/FuncCallHandler.php | 5 +- .../ExprHandler/Helper/OutputBufferHelper.php | 10 ++ ...ObStartFunctionTypeSpecifyingExtension.php | 70 +++++++++ tests/PHPStan/Analyser/nsrt/bug-14985.php | 26 ++++ .../Analyser/nsrt/output-buffering.php | 146 ++++++++++++++---- 5 files changed, 227 insertions(+), 30 deletions(-) create mode 100644 src/Type/Php/ObStartFunctionTypeSpecifyingExtension.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14985.php diff --git a/src/Analyser/ExprHandler/FuncCallHandler.php b/src/Analyser/ExprHandler/FuncCallHandler.php index 47efb2af7f..c6e9b35911 100644 --- a/src/Analyser/ExprHandler/FuncCallHandler.php +++ b/src/Analyser/ExprHandler/FuncCallHandler.php @@ -571,8 +571,11 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $scope->afterOpenSslCall($functionReflection->getName()); } + // Functions that only open the buffer on success (e.g. ob_start()) are handled + // by ObStartFunctionTypeSpecifyingExtension, so their level change is applied to + // the truthy branch only - an unchecked call may have failed. $outputBufferDelta = $functionReflection !== null ? OutputBufferHelper::getLevelDelta($functionReflection->getName()) : 0; - if ($outputBufferDelta !== 0) { + if ($outputBufferDelta !== 0 && !OutputBufferHelper::opensBufferOnSuccess($functionReflection->getName())) { $scope = OutputBufferHelper::applyLevelDelta($scope, $outputBufferDelta); } diff --git a/src/Analyser/ExprHandler/Helper/OutputBufferHelper.php b/src/Analyser/ExprHandler/Helper/OutputBufferHelper.php index 309ad76c9f..280c06d52e 100644 --- a/src/Analyser/ExprHandler/Helper/OutputBufferHelper.php +++ b/src/Analyser/ExprHandler/Helper/OutputBufferHelper.php @@ -30,6 +30,16 @@ public static function getLevelDelta(string $functionName): int return 0; } + /** + * Whether the function only opens the output buffer when it returns a truthy + * value (e.g. `ob_start()`). The level increase must therefore be applied to + * the truthy branch only, since an unchecked call may have failed. + */ + public static function opensBufferOnSuccess(string $functionName): bool + { + return in_array($functionName, self::LEVEL_INCREMENTING_FUNCTIONS, true); + } + public static function applyLevelDelta(MutatingScope $scope, int $delta): MutatingScope { foreach ([new Name('ob_get_level'), new Name\FullyQualified('ob_get_level')] as $name) { diff --git a/src/Type/Php/ObStartFunctionTypeSpecifyingExtension.php b/src/Type/Php/ObStartFunctionTypeSpecifyingExtension.php new file mode 100644 index 0000000000..5bf6361fad --- /dev/null +++ b/src/Type/Php/ObStartFunctionTypeSpecifyingExtension.php @@ -0,0 +1,70 @@ +typeSpecifier = $typeSpecifier; + } + + public function isFunctionSupported( + FunctionReflection $functionReflection, + FuncCall $node, + TypeSpecifierContext $context, + ): bool + { + return $functionReflection->getName() === 'ob_start' && $context->truthy(); + } + + public function specifyTypes( + FunctionReflection $functionReflection, + FuncCall $node, + Scope $scope, + TypeSpecifierContext $context, + ): SpecifiedTypes + { + $types = new SpecifiedTypes(); + foreach ([new Name('ob_get_level'), new Name\FullyQualified('ob_get_level')] as $name) { + $obGetLevelCall = new FuncCall($name, []); + $newLevelType = $scope->getType(new BinaryOp\Plus( + new TypeExpr($scope->getType($obGetLevelCall)), + new TypeExpr(new ConstantIntegerType(1)), + )); + + $types = $types->unionWith($this->typeSpecifier->create( + $obGetLevelCall, + $newLevelType, + $context, + $scope, + )->setAlwaysOverwriteTypes()); + } + + return $types; + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-14985.php b/tests/PHPStan/Analyser/nsrt/bug-14985.php new file mode 100644 index 0000000000..7cf9bf47af --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14985.php @@ -0,0 +1,26 @@ +', ob_get_level()); + assertType('string|false', ob_get_contents()); + assertType('string|false', ob_get_clean()); + assertType('int|false', ob_get_length()); +} + +function activeBuffer(): void +{ + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_contents()); assertType('int', ob_get_length()); } +function activeBufferInTruthyBranch(): void +{ + if (ob_start()) { + assertType('int<1, max>', ob_get_level()); + assertType('string', ob_get_contents()); + } else { + assertType('int<0, max>', ob_get_level()); + assertType('string|false', ob_get_contents()); + } +} + +function activeBufferCheckedByIdentical(): void +{ + if (ob_start() === false) { + return; + } + assertType('int<1, max>', ob_get_level()); + assertType('string', ob_get_contents()); +} + function obCleanAndFlushKeepBuffer(): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); ob_clean(); assertType('int<1, max>', ob_get_level()); @@ -35,7 +69,9 @@ function obCleanAndFlushKeepBuffer(): void function getCleanClosesBuffer(): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); assertType('int<0, max>', ob_get_level()); @@ -44,7 +80,9 @@ function getCleanClosesBuffer(): void function getFlushClosesBuffer(): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_flush()); assertType('int<0, max>', ob_get_level()); @@ -53,7 +91,9 @@ function getFlushClosesBuffer(): void function endCleanClosesBuffer(): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_contents()); ob_end_clean(); @@ -63,7 +103,9 @@ function endCleanClosesBuffer(): void function endFlushClosesBuffer(): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_contents()); ob_end_flush(); @@ -73,9 +115,13 @@ function endFlushClosesBuffer(): void function nested(): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); - ob_start(); + if (!ob_start()) { + return; + } assertType('int<2, max>', ob_get_level()); assertType('string', ob_get_contents()); ob_end_clean(); @@ -97,7 +143,9 @@ function conditional(bool $cond): void function fullyQualified(): void { - \ob_start(); + if (!\ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); assertType('string', \ob_get_contents()); assertType('string', ob_get_contents()); @@ -132,7 +180,9 @@ function levelNarrowedToUnionInt(): void if (ob_get_level() === 1 || ob_get_level() === 3) { assertType('1|3', ob_get_level()); assertType('string', ob_get_contents()); - ob_start(); + if (!ob_start()) { + return; + } assertType('2|4', ob_get_level()); assertType('string', ob_get_contents()); } @@ -158,7 +208,9 @@ function levelNarrowedToBoundedIntRange(): void function impureCallableForgetsLevel(callable $cb): void { - ob_start(); + if (!ob_start()) { + return; + } assertType('int<1, max>', ob_get_level()); $cb(); // the callable may have closed the buffer, so the level is forgotten @@ -168,7 +220,9 @@ function impureCallableForgetsLevel(callable $cb): void function impureClosureForgetsLevel(\Closure $closure): void { - ob_start(); + if (!ob_start()) { + return; + } $closure(); assertType('int<0, max>', ob_get_level()); assertType('string|false', ob_get_clean()); @@ -177,7 +231,9 @@ function impureClosureForgetsLevel(\Closure $closure): void /** @param pure-callable $cb */ function pureCallableKeepsLevel(callable $cb): void { - ob_start(); + if (!ob_start()) { + return; + } $cb(); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -185,7 +241,9 @@ function pureCallableKeepsLevel(callable $cb): void function impureFunctionForgetsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } impureFunction(); assertType('int<0, max>', ob_get_level()); assertType('string|false', ob_get_clean()); @@ -203,7 +261,9 @@ function impureFunction(): void function pureFunctionKeepsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } $x=pureFunction(); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -234,7 +294,9 @@ public function __invoke(): void function impureMethodForgetsLevel(Service $service): void { - ob_start(); + if (!ob_start()) { + return; + } $service->impureMethod(); assertType('int<0, max>', ob_get_level()); assertType('string|false', ob_get_clean()); @@ -242,7 +304,9 @@ function impureMethodForgetsLevel(Service $service): void function pureMethodKeepsLevel(Service $service): void { - ob_start(); + if (!ob_start()) { + return; + } $x=$service->pureMethod(); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -250,7 +314,9 @@ function pureMethodKeepsLevel(Service $service): void function impureStaticMethodForgetsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } Service::impureStaticMethod(); assertType('int<0, max>', ob_get_level()); assertType('string|false', ob_get_clean()); @@ -258,7 +324,9 @@ function impureStaticMethodForgetsLevel(): void function invokableForgetsLevel(Service $service): void { - ob_start(); + if (!ob_start()) { + return; + } $service(); assertType('int<0, max>', ob_get_level()); assertType('string|false', ob_get_clean()); @@ -266,7 +334,9 @@ function invokableForgetsLevel(Service $service): void function arrayMapPureCallbackKeepsLevel(array $a): void { - ob_start(); + if (!ob_start()) { + return; + } array_map('strtoupper', $a); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -274,7 +344,9 @@ function arrayMapPureCallbackKeepsLevel(array $a): void function laterInvokedCallableKeepsLevel(callable $cb): void { - ob_start(); + if (!ob_start()) { + return; + } register_shutdown_function($cb); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -282,7 +354,9 @@ function laterInvokedCallableKeepsLevel(callable $cb): void function builtinKeepsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } printf('hello'); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -314,7 +388,9 @@ class WithoutConstructor function impureConstructorForgetsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } // the constructor may have opened or closed a buffer new WithImpureConstructor(); assertType('int<0, max>', ob_get_level()); @@ -323,7 +399,9 @@ function impureConstructorForgetsLevel(): void function pureConstructorKeepsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } new WithPureConstructor(); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -331,7 +409,9 @@ function pureConstructorKeepsLevel(): void function noConstructorKeepsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } new WithoutConstructor(); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -340,7 +420,9 @@ function noConstructorKeepsLevel(): void /** @param class-string $className */ function unknownClassForgetsLevel(string $className): void { - ob_start(); + if (!ob_start()) { + return; + } new $className(); assertType('int<0, max>', ob_get_level()); assertType('string|false', ob_get_clean()); @@ -348,7 +430,9 @@ function unknownClassForgetsLevel(string $className): void function builtinConstructorKeepsLevel(): void { - ob_start(); + if (!ob_start()) { + return; + } new \ArrayObject(); assertType('int<1, max>', ob_get_level()); assertType('string', ob_get_clean()); @@ -356,14 +440,18 @@ function builtinConstructorKeepsLevel(): void function withRequire(): void { - ob_start(); + if (!ob_start()) { + return; + } require __DIR__ . '/does-not-matter.php'; assertType('int<0, max>', ob_get_level()); } function withEval(string $function): void { - ob_start(); + if (!ob_start()) { + return; + } eval(sprintf('function %s() {}', $function)); assertType('int<0, max>', ob_get_level()); }