From 92b6c2a031b32e845d976d3d1df1c18da1b26ac9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 21:56:33 +0000 Subject: [PATCH] fix(core): make CastObjectHook fall through to default engine behaviour safely Two defects made the documented fall-through pattern - proceed() then getResult() - unusable for cast types the default handler cannot satisfy: - The engine hands cast_object an UNINITIALIZED retval slot, and zend_std_cast_object_tostring returns FAILURE for numeric casts without writing it (the engine caller is the one that warns and substitutes 1). getResult() dereferenced that slot regardless, running the by-ref ReferenceEntry machinery over garbage - which observably unsets local variables in the calling frames and emits spurious "Undefined variable" warnings. - handle() unconditionally reported Core::SUCCESS, so a FAILURE from the original handler could never propagate: whatever the user handler returned - including an accidental null - was silently installed as the cast result, and the engine caller's default behaviour (warning plus substitute value for numeric casts, engine Error for string casts) was unreachable. proceed() now records its status. getResult() refuses to read the slot unless the last proceed() succeeded and returns null instead, and handle() propagates FAILURE to the engine when the user handler fell through (returned null after a failed proceed). Together this makes the naive fall-through behave exactly like an uninstalled handler for every cast type; the regression test asserts the engine's own diagnostic and substitute value come back and that no corruption noise is emitted. Fixes #153 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JsbdqisRfGuujN3QD9n8En --- phpstan-baseline.neon | 2 +- src/ClassExtension/Hook/CastObjectHook.php | 50 +++++++++++++++++-- tests/Reflection/ReflectionClassTest.php | 56 ++++++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index c2124a4..46d5c66 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3303,7 +3303,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 a8be40d..5dfdd6b 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); @@ -79,10 +97,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; @@ -90,14 +129,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 0055b5d..8bec1bd 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 {