diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3793133..49fe48d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3297,7 +3297,7 @@ parameters: - message: '#^Cannot cast ZEngine\\Stub\\TestClass to int\.$#' identifier: cast.int - count: 1 + count: 2 path: tests/Reflection/ReflectionClassTest.php - diff --git a/src/ClassExtension/Hook/CastObjectHook.php b/src/ClassExtension/Hook/CastObjectHook.php index c5b8e90..c45c3a1 100644 --- a/src/ClassExtension/Hook/CastObjectHook.php +++ b/src/ClassExtension/Hook/CastObjectHook.php @@ -41,6 +41,15 @@ class CastObjectHook extends AbstractHook */ protected int $type; + /** + * Status of the last proceed() call within the current handle() invocation, null if none + * + * The engine hands cast_object an UNINITIALIZED retval slot; only a successful original + * handler writes it. Tracking the status is what lets getResult() refuse to read scratch + * memory and lets handle() propagate a fall-through failure to the engine caller. + */ + private ?int $lastProceedStatus = null; + /** * typedef int (*zend_object_cast_t)(zend_object *readobj, zval *retval, int type); * @@ -49,8 +58,17 @@ class CastObjectHook extends AbstractHook public function handle(...$rawArguments): int { [$this->object, $this->returnValue, $this->type] = $rawArguments; + $this->lastProceedStatus = null; $result = ($this->userHandler)($this); + if ($result === null && $this->lastProceedFailed()) { + // The user handler fell through to the original handler and that handler could not + // produce a value. Propagate the failure so the engine caller applies its own + // default behaviour (diagnostic and substitute value for numeric casts, engine + // Error for string casts) instead of silently installing NULL as the cast result + return Core::FAILURE; + } + // The retval slot is uninitialized scratch memory provided by the engine caller, // so there is no previous value to release in it ReflectionValue::fromValueEntry($this->returnValue)->initializeNativeValue($result); @@ -91,10 +109,31 @@ public function getObject(): object } /** - * Returns result of casting (eg from call to proceed) + * Whether the user handler fell through to the original handler and that handler failed + * + * A method rather than an inline comparison on purpose: the property is mutated by + * proceed() from inside the userHandler closure invocation, a side effect static analysis + * cannot see through — inline, the null assignment in handle() would narrow the comparison + * to a compile-time constant. + */ + private function lastProceedFailed(): bool + { + return $this->lastProceedStatus === Core::FAILURE; + } + + /** + * Returns result of casting from a successful call to proceed(), null otherwise + * + * The retval slot is written only by a successful proceed(): reading it before one (or after + * a failed one) would dereference uninitialized scratch memory, so null is returned instead. + * Combined with handle() this makes the naive fall-through — `$hook->proceed(); return + * $hook->getResult();` — behave exactly like an uninstalled handler for every cast type. */ public function getResult() { + if ($this->lastProceedStatus !== Core::SUCCESS) { + return null; + } ReflectionValue::fromValueEntry($this->returnValue)->getNativeValue($result); return $result; @@ -102,14 +141,19 @@ public function getResult() /** * Proceeds with object casting + * + * @return int Core::SUCCESS when the original handler produced a value in the retval slot, + * Core::FAILURE when it could not (numeric casts on plain objects, for example) */ public function proceed() { if (!$this->hasOriginalHandler()) { throw new \LogicException('Original handler is not available'); } - $result = ($this->originalHandler)($this->object, $this->returnValue, $this->type); + $status = ($this->originalHandler)($this->object, $this->returnValue, $this->type); + assert(is_int($status)); + $this->lastProceedStatus = $status; - return $result; + return $this->lastProceedStatus; } } diff --git a/tests/Reflection/ReflectionClassTest.php b/tests/Reflection/ReflectionClassTest.php index 71d3970..9849332 100644 --- a/tests/Reflection/ReflectionClassTest.php +++ b/tests/Reflection/ReflectionClassTest.php @@ -320,6 +320,62 @@ public function testInstallCastObjectHandler(): void $this->markTestIncomplete('Initialization object handler brings segfaults thus run it separately'); } + #[RunInSeparateProcess] + public function testCastObjectHandlerFallsThroughToEngineDefault(): void + { + $handler = Closure::fromCallable([ObjectCreateTrait::class, '__init']); + $this->refClass->setCreateObjectHandler($handler); + $this->refClass->setCastObjectHandler(function (CastObjectHook $hook) { + // The naive fall-through: defer every cast to the engine and hand back its result + $hook->proceed(); + + return $hook->getResult(); + }); + + $instance = new TestClass(); + + // Boolean casts succeed in the default handler, so the fall-through must yield its value + $this->assertTrue(self::convertToBooleanViaEngine($instance)); + + // Numeric casts FAIL in the default handler without writing the retval slot: the + // failure must propagate to the engine caller (which warns and substitutes 1) instead + // of reading uninitialized memory or silently installing null. Capturing every PHP + // diagnostic also proves the fall-through emits no "Undefined variable" corruption noise + $capturedWarnings = []; + set_error_handler(static function (int $code, string $message) use (&$capturedWarnings): bool { + $capturedWarnings[] = $message; + + return true; + }); + try { + $long = (int) $instance; + } finally { + restore_error_handler(); + } + + $this->assertSame(1, $long); + $this->assertSame( + ['Object of class ' . TestClass::class . ' could not be converted to int'], + $capturedWarnings, + ); + $this->markTestIncomplete('Initialization object handler brings segfaults thus run it separately'); + } + + /** + * Converts an object through the engine's boolean-conversion path (cast_object) + * + * A helper on purpose: written inline, the conversion result is a compile-time constant + * for static analysis (object-to-bool is always true there), while the installed cast + * handler decides it at runtime — the declared bool return type erases the narrowing + */ + private static function convertToBooleanViaEngine(object $instance): bool + { + $value = $instance; + settype($value, 'boolean'); + + return $value; + } + #[RunInSeparateProcess] public function testInstallReadPropertyHandler(): void {