Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid fatal error handling in php 8.3 #20147

Open
matrozov opened this issue Apr 11, 2024 · 11 comments
Open

Invalid fatal error handling in php 8.3 #20147

matrozov opened this issue Apr 11, 2024 · 11 comments
Labels

Comments

@matrozov
Copy link

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
Copy link
Member

bizley commented Apr 11, 2024

Could you provide wider stack trace?

@matrozov
Copy link
Author

Could you provide wider stack trace?

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

@bizley
Copy link
Member

bizley commented Apr 11, 2024

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

@matrozov
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

rob006 commented Apr 11, 2024

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

@matrozov
Copy link
Author

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
Copy link
Contributor

rob006 commented Apr 11, 2024

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
Copy link
Author

matrozov commented Apr 11, 2024

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
Copy link
Author

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
Copy link
Author

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 (

@samdark samdark added the php8 label Apr 13, 2024
@samdark samdark added this to the 2.0.50 milestone Apr 13, 2024
@samdark samdark removed this from the 2.0.50 milestone May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants