yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

Invalid fatal error handling in php 8.3 #20147

Closed matrozov closed 2 months ago

matrozov commented 5 months ago

What steps will reproduce the problem?

Install imap extension for php. Open a connection to the imap server and call the imap_fetch_overview function with an invalid sequence param, for example:

$imap = imap_open(...
imap_fetch_overview($imap, '-');

What is the expected result?

We expect to see a fatal error generated by the imap module:

PHP Fatal error:  Uncaught yii\base\ErrorException: PHP Request Shutdown: Sequence out of range (errflg=2) in Unknown:0
Stack trace:
#0 [internal function]: yii\base\ErrorHandler->handleError()
#1 {main}
  thrown in Unknown on line 0

What do you get instead?

We see that instead of a fatal error in the imap module, we get the following error in the ErrorHandler module in yii2:

PHP Fatal error:  Uncaught yii\base\UnknownPropertyException: Setting unknown property: yii\console\ErrorHandler::_memoryReserve in /app/vendor/yiisoft/yii2/base/Component.php:209
Stack trace:
#0 /app/vendor/yiisoft/yii2/base/ErrorHandler.php(115): yii\base\Component->__set()
#1 /app/vendor/yiisoft/yii2/base/ErrorHandler.php(139): yii\base\ErrorHandler->unregister()
#2 [internal function]: yii\base\ErrorHandler->handleException()
#3 {main}
  thrown in /app/vendor/yiisoft/yii2/base/Component.php on line 209

Additional info

The problem is not reproducible in php 8.2 and apparently versions below.

Q A
Yii version 2.0.49.3
PHP version 8.3.4
Operating system Debian bullseye (php:8.3-fpm-bullseye)
bizley commented 5 months ago

Could you provide wider stack trace?

matrozov commented 5 months ago

Could you provide wider stack trace?

This is the full stack trace that I see in the console output in both cases.

bizley commented 5 months ago

Hm, I'm baffled. Console error handler extends base error handler and _memoryReserve property is set inside the base one...

matrozov commented 5 months ago

It looks like after executing handleFatalError and unset($this->_memoryReserve), a handleException occurs and calls the "unregister" method, which leads to a crash. But I haven't figured out yet why this happens in php 8.3 but not in 8.2.

In php 8.3, after a fatal error, the same exception is called as a normal handleException, and in php 8.2 this error leads to handleError, not fatal.

It looks like in php 8.3 the imap module throws a different kind of exception or this exception is handled differently by php. It’s strange that after a fatal error, something else generally works and the code continues to be executed. I have a simple var_dump after the line with imap_fetch_overview and I see its output on both versions of php. And it’s strange that in php 8.3, after a fatal error, I also see it.

matrozov commented 5 months ago

I tried experimenting with other fatal errors, such as the memory limit. But on both versions of PHP the result was the same and correct. Probably the problem is in a specific imap module, but I did not find any mention in the documentation of changing error handling in it. I installed the module itself using the standard method, so I don’t think the problem is that I have some special version of the imap module:

RUN apt install -y libc-client-dev libkrb5-dev \
     && docker-php-ext-configure imap --with-kerberos --with-imap-ssl \
     && docker-php-ext-install imap
rob006 commented 5 months ago

Does it help if you change unset($this->_memoryReserve) with $this->_memoryReserve = null?

matrozov commented 5 months ago

Does it help if you change unset($this->_memoryReserve) with $this->_memoryReserve = null?

Yes, this fixes the crash inside the ErrorHandler and I see the original exception from imap in the console. By adding a similar action for _workingDirectory, do the same.

rob006 commented 5 months ago

That's weird. I suspected that during handling imap error (error 1), there is another error in error handler (error 2) which triggers error handler again which results Setting unknown property error (error 3). But in that case after removing unset() you should get error 2 instead of error 1 :/

matrozov commented 5 months ago

Yes, it's strange. Because in handleFatalError and handleException I get the same error. I thought that they generally call each other - but this is not so. It seems that the imap module actually triggers the same error twice. At least that's how it looks for now.

matrozov commented 5 months ago

On php 8.3 I get a serial call:

handleFatalError
handleError
handleException

They all cause from outside, and not from each other. On php 8.2 only:

handleFatalError
handleError

It is logical that handleFatalError is always called, because it is a reaction to register_shutdown_function. At the same time, there is only clearing of variables, and not error handling (in both cases error_get_last() === null). But it’s not clear where handleException comes from in the case of PHP 8.3.

matrozov commented 5 months ago

I am wrong. handleException is the response to throw $exception in handleError. Those. it's essentially an indirect challenge to each other. But in php 8.2 this does not lead to calling the handleException function, but in 8.3 it does. I'm confused (

profluck commented 2 months ago

PHP Version 8.3.9, Yii2 Version 2.0.50

An error occurred when trying to unpack invalid data with the native PHP function "unserialize"

Fatal error: Uncaught yii\base\UnknownPropertyException: Setting unknown property: yii\web\ErrorHandler::_memoryReserve in /var/www/html/vendor/yiisoft/yii2/base/Component.php on line 217

yii\base\ErrorHandler->handleException( $exception = class yii\base\ErrorException { protected $message = 'unserialize(): Extra data starting at offset 7 of 222 bytes'; private string ${Exception}string = ''; protected $code = 2; protected string $file = '/var/www/html/components/redis/Session.php'; protected int $line = 197; private array ${Exception}trace = [0 => [...], 1 => [...], 2 => [...], 3 => [...], 4 => [...], 5 => [...], 6 => [...]]; private ?Throwable ${Exception}previous = NULL; protected int $severity = 2 } )

yii\base\ErrorHandler->unregister( ) at .../ErrorHandler.php:139 yii\base\Component->__set( $name = '_memoryReserve', $value = NULL ) at .../ErrorHandler.php:115

samdark commented 2 months ago

We should change to cleaning up memory reserve by setting it to ''. We already do it in Yii3.

My6UoT9 commented 2 months ago

According to both error msgs, the extended unset, set from Components tries to set the variable to null, but the variable is private. Maybe, due to dynamic properties, this worked in previous php versions, but it would had never freed the actually memory reserve - just add a new dynamic variable to the object?