From f3eefee5727627ffde1af16fe66e5ff8f9183aac Mon Sep 17 00:00:00 2001 From: Wilmer Arambula <42547589+terabytesoftw@users.noreply.github.com> Date: Wed, 25 Jun 2025 08:07:46 -0400 Subject: [PATCH] Fix #20348: `ErrorHandler::convertExceptionToError()` Passing `E_USER_ERROR` to `trigger_error()` is deprecated since PHP `8.4` --- framework/CHANGELOG.md | 1 + framework/UPGRADE.md | 35 +++++++++++++ framework/base/ErrorHandler.php | 5 ++ framework/mail/BaseMessage.php | 11 ++-- framework/widgets/ActiveField.php | 12 +++-- tests/framework/mail/BaseMessageTest.php | 56 ++++++++++++++++++++ tests/framework/widgets/ActiveFieldTest.php | 58 ++++++++++++++++++++- 7 files changed, 169 insertions(+), 9 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index c2f252742f..4dbd18e722 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -26,6 +26,7 @@ Yii Framework 2 Change Log - Enh #20413: Add PHPStan/Psalm annotations for `Action`, `ActionEvent`, `Application`, `DynamicModel` and `InlineAction` (max-s-lab) - Enh #20416: Add `PHPStan`/`PSalm` annotation for `owner` property in `Behavior` class (terabytesoftw) - Bug #20423: `strcmp()` Passing `null` to parameter `2` ($string2) of type string is deprecated (terabytesoftw) +- Bug #20348: `ErrorHandler::convertExceptionToError()` Passing `E_USER_ERROR` to `trigger_error()` is deprecated since PHP `8.4` (terabytesoftw) 2.0.52 February 13, 2025 diff --git a/framework/UPGRADE.md b/framework/UPGRADE.md index 9021360885..dc51988ec0 100644 --- a/framework/UPGRADE.md +++ b/framework/UPGRADE.md @@ -51,6 +51,41 @@ if you want to upgrade from version A to version C and there is version B between A and C, you need to follow the instructions for both A and B. +Upgrade from Yii 2.0.53 +----------------------- + +* `ErrorHandler::convertExceptionToError()` has been deprecated and will be removed in version 22.0. + + This method was deprecated due to `PHP 8.4` deprecating the use of `E_USER_ERROR` with `trigger_error()`. + The framework now handles exceptions in `__toString()` methods more appropriately based on the PHP version. + + **Before (deprecated):** + ```php + public function __toString() { + try { + return $this->render(); + } catch (\Throwable $e) { + ErrorHandler::convertExceptionToError($e); + return ''; + } + } + ``` + + **After (recommended):** + ```php + public function __toString() { + try { + return $this->render(); + } catch (\Throwable $e) { + if (PHP_VERSION_ID < 70400) { + trigger_error(ErrorHandler::convertExceptionToString($e), E_USER_ERROR); + return ''; + } + throw $e; + } + } + ``` + Upgrade from Yii 2.0.52 ----------------------- * There was a bug when loading fixtures into PostgreSQL database, the table sequences were not reset. If you used a work-around or if you depended on this behavior, you are advised to review your code. diff --git a/framework/base/ErrorHandler.php b/framework/base/ErrorHandler.php index c31cd058bc..74eb02c686 100644 --- a/framework/base/ErrorHandler.php +++ b/framework/base/ErrorHandler.php @@ -378,6 +378,11 @@ abstract class ErrorHandler extends Component * to PHP errors because exceptions cannot be thrown inside of them. * @param \Throwable $exception the exception to convert to a PHP error. * @return never + * + * @deprecated since 2.0.53. Use conditional exception throwing in `__toString()` methods instead. + * For PHP < 7.4: use `trigger_error()` directly with `convertExceptionToString()` method. + * For PHP >= 7.4: throw the exception directly as `__toString()` supports exceptions. + * This method will be removed in 2.2.0. */ public static function convertExceptionToError($exception) { diff --git a/framework/mail/BaseMessage.php b/framework/mail/BaseMessage.php index 801b7468a8..5ea187242d 100644 --- a/framework/mail/BaseMessage.php +++ b/framework/mail/BaseMessage.php @@ -59,9 +59,14 @@ abstract class BaseMessage extends BaseObject implements MessageInterface // use trigger_error to bypass this limitation try { return $this->toString(); - } catch (\Exception $e) { - ErrorHandler::convertExceptionToError($e); - return ''; + } catch (\Throwable $e) { + if (PHP_VERSION_ID < 70400) { + trigger_error(ErrorHandler::convertExceptionToString($e), E_USER_ERROR); + + return ''; + } + + throw $e; } } } diff --git a/framework/widgets/ActiveField.php b/framework/widgets/ActiveField.php index a1b20cb801..1761b8566e 100644 --- a/framework/widgets/ActiveField.php +++ b/framework/widgets/ActiveField.php @@ -174,12 +174,14 @@ class ActiveField extends Component // use trigger_error to bypass this limitation try { return $this->render(); - } catch (\Exception $e) { - ErrorHandler::convertExceptionToError($e); - return ''; } catch (\Throwable $e) { - ErrorHandler::convertExceptionToError($e); - return ''; + if (PHP_VERSION_ID < 70400) { + trigger_error(ErrorHandler::convertExceptionToString($e), E_USER_ERROR); + + return ''; + } + + throw $e; } } diff --git a/tests/framework/mail/BaseMessageTest.php b/tests/framework/mail/BaseMessageTest.php index 32bf2dcd31..dada78e84e 100644 --- a/tests/framework/mail/BaseMessageTest.php +++ b/tests/framework/mail/BaseMessageTest.php @@ -60,6 +60,54 @@ class BaseMessageTest extends TestCase $message = $mailer->compose(); $this->assertEquals($message->toString(), '' . $message); } + + public function testExceptionToString() + { + if (PHP_VERSION_ID < 70400) { + $this->markTestSkipped('This test is for PHP 7.4+ only'); + } + + $message = new TestMessageWithException(); + + $this->expectException(\Exception::class); + $this->expectExceptionMessage('Test exception in toString.'); + + (string) $message; + } + + public function testExceptionToStringLegacy() + { + if (PHP_VERSION_ID >= 70400) { + $this->markTestSkipped('This test is for PHP < 7.4 only'); + } + + $message = new TestMessageWithException(); + + $errorTriggered = false; + $errorMessage = ''; + + set_error_handler( + function ($severity, $message, $file, $line) use (&$errorTriggered, &$errorMessage) { + if ($severity === E_USER_ERROR) { + $errorTriggered = true; + $errorMessage = $message; + + return true; + } + + return false; + }, + E_USER_ERROR, + ); + + $result = (string) $message; + + restore_error_handler(); + + $this->assertTrue($errorTriggered, 'E_USER_ERROR should have been triggered'); + $this->assertStringContainsString('Test exception in toString.', $errorMessage); + $this->assertSame('', $result, 'Result should be an empty string'); + } } /** @@ -178,3 +226,11 @@ class TestMessage extends BaseMessage return get_class($this); } } + +class TestMessageWithException extends TestMessage +{ + public function toString() + { + throw new \Exception('Test exception in toString.'); + } +} diff --git a/tests/framework/widgets/ActiveFieldTest.php b/tests/framework/widgets/ActiveFieldTest.php index b38cfa29c8..0307856895 100644 --- a/tests/framework/widgets/ActiveFieldTest.php +++ b/tests/framework/widgets/ActiveFieldTest.php @@ -25,7 +25,7 @@ use yii\widgets\MaskedInput; class ActiveFieldTest extends \yiiunit\TestCase { use ArraySubsetAsserts; - + /** * @var ActiveFieldExtend */ @@ -700,6 +700,54 @@ HTML; $this->assertStringContainsString('placeholder="pholder_both_direct"', (string) $widget); } + public function testExceptionToString() + { + if (PHP_VERSION_ID < 70400) { + $this->markTestSkipped('This test is for PHP 7.4+ only'); + } + + $field = new TestActiveFieldWithException(); + + $this->expectException(\Exception::class); + $this->expectExceptionMessage('Test exception in toString.'); + + (string) $field; + } + + public function testExceptionToStringLegacy() + { + if (PHP_VERSION_ID >= 70400) { + $this->markTestSkipped('This test is for PHP < 7.4 only'); + } + + $field = new TestActiveFieldWithException(); + + $errorTriggered = false; + $errorMessage = ''; + + set_error_handler( + function ($severity, $message, $file, $line) use (&$errorTriggered, &$errorMessage) { + if ($severity === E_USER_ERROR) { + $errorTriggered = true; + $errorMessage = $message; + + return true; + } + + return false; + }, + E_USER_ERROR, + ); + + $result = (string) $field; + + restore_error_handler(); + + $this->assertTrue($errorTriggered, 'E_USER_ERROR should have been triggered'); + $this->assertStringContainsString('Test exception in toString.', $errorMessage); + $this->assertSame('', $result, 'Result should be an empty string'); + } + /** * Helper methods. */ @@ -811,3 +859,11 @@ class TestMaskedInput extends MaskedInput } } +class TestActiveFieldWithException extends ActiveField +{ + public function render($content = null) + { + throw new \Exception('Test exception in toString.'); + } +} +