Skip to content

Conversation

@mbabker
Copy link
Contributor

@mbabker mbabker commented Sep 4, 2017

Partial Pull Request for Issue #16584

Summary of Changes

PHP 7.2 deprecates the track_errors INI configuration and the scoped $php_errormsg variable in favor of using the error_get_last() function for retrieving error data, and at PHP 7.0 using error_clear_last() to clear any errors (to my knowledge we aren't doing anything in core requiring the latter at the moment).

This PR phases out this deprecated functionality for the HTTP and Language packages.

Testing Instructions

The packages should still function normally, including error handling (easiest way to test error handling is enable debug mode and language debug and break one of the language files with a parsing error).

$php_errormsg = null;

$trackErrors = ini_get('track_errors');
ini_set('track_errors', true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do not you remove these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want it to track errors regardless of PHP config while in debug mode. So this ensures the setting is enabled then reset to what it was previously at the end of the process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nevermind. This is the deprecated thing. 🤦‍♂️

Need more coffee.

*/
protected function parse($filename)
{
// Capture hidden PHP errors from the parsing.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this comment is no longer applicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is still applicable. When debug mode is enabled we are still setting error reporting to maximum basically so that parse errors are exposed, then resetting error reporting to the configured value.

{
if (!$php_errormsg)
$lastError = error_get_last();
$message = sprintf('Could not connect to resource: %s', $uri, $err, $errno);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move the initialization of $message below to the else statement? The same on the second file too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't an else, unless you're saying to make it an if/else statement...

@csthomas
Copy link
Contributor

csthomas commented Jan 8, 2018

I suggest to use error_clear_last.

For test I removed the first double quote character from com_content language file.

Before patch:

JROOT/language/en-GB/en-GB.com_content.ini : error(s) in line(s) 6

After patch:

    JROOT/language/en-GB/en-GB.com_content.ini : error(s) in line(s) 6
    PHPJROOT/language/en-GB/en-GB.mod_menu.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_tags_popular.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_articles_category.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_articles_archive.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_articles_popular.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_syndicate.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_login.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_breadcrumbs.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_custom.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_languages.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/language/en-GB/en-GB.mod_search.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6
    PHPJROOT/administrator/language/en-GB/en-GB.plg_system_debug.iniPHP parser errors :syntax error, unexpected BOOL_TRUE in /home/tomash/public_html/a4/language/en-GB/en-GB.com_content.ini on line 6 

@mbabker
Copy link
Contributor Author

mbabker commented Jan 8, 2018

I suggest to use error_clear_last.

How reliable is the polyfill for error_clear_last()?. error_clear_last() was only introduced in PHP 7.0 so we need to polyfill it until 4.0.

@csthomas
Copy link
Contributor

csthomas commented Jan 8, 2018

Or you can remember the previous error message before line error_reporting(E_ALL) and compare it to the last one at the bottom. I assumed that error messages are unique:)

@csthomas
Copy link
Contributor

csthomas commented Jan 8, 2018

This is my test.

$ php -a
Interactive mode enabled

php > echo $e;
PHP Notice:  Undefined variable: e in php shell code on line 1
php > $handler = function() { return false;};
php > set_error_handler($handler);
php > @trigger_error('');
php > restore_error_handler();
php > print_r(error_get_last());
Array
(
    [type] => 1024
    [message] => 
    [file] => php shell code
    [line] => 1
)
php > echo phpversion();
7.1.12-3+ubuntu16.04.1+deb.sury.org+1

@mbabker
Copy link
Contributor Author

mbabker commented Jul 21, 2018

Eh, maybe later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants