Fatal PHP errors, like syntax errors, not an object, etc, are not shown properly when using PHP 7

[ggppdk]

Steps to reproduce the issue

Use PHP7 and J3.5.1 (or latest staging)

add a syntax or other fatal error in any PHP file

Expected result

If error reporting is ON then at least the FILE and LINE that produced the fatal error should be shown

Actual result

The file and line that created the fatal error, are not shown

• even if you enabled the Joomla Debug parameter, then you get the backtrace (function/file stack), but still even in this case, the File and Line (of the offending file) are not shown

System information (as much as possible)

PHP7.x
J3.5.x (or latest staging)

Additional comments

The reason is that we do not let the PHP7 default error handler to produces it own output,

• but instead Joomla catches the exception and prints the error with custom handler

so far so good, but then the Joomla error handler does not seem to call

...error->getFile()
...error->getLine()

in order to get the needed info and display it,
or am i missing something ?

[ggppdk]

Here are the lines that get the error code and error message and print it

Backend:
https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/error.php#L234
https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/hathor/error.php#L37

Frontend:
https://github.com/joomla/joomla-cms/blob/staging/templates/protostar/error.php#L166
https://github.com/joomla/joomla-cms/blob/staging/templates/beez3/error.php#L160

• by calling ...error->getCode() ...error->getMessage()

but you can see that they do not call:
...error->getFile()
...error->getLine()

which is needed for PHP 7 because the getMessage() does not include file and line information

Probably we need to add something like the following code, to the error.php of the templates ?:

if ( version_compare(phpversion(), '7.0.0', '>=') )
{
    // Add the file and line at which the error occured
    $error_file = $this->error->getFile();
    $error_line = $this->error->getLine();
    if ( $error_file )
    {
        echo '<pre>' .$error_file .':'. $error_line . '</pre>';
    }
}

[mbabker]

PHP 7 made no changes to how stack traces are processed, so if you make such changes you do NOT need to wrap it in a version comparison conditional. See https://groups.google.com/d/topic/joomla-dev-cms/bPsyhj55qcA/discussion for more context.

[ggppdk]

about searching i only tried to find the issue here, with a quick look i did not find,

about version compare, indeed version compare is not needed

in the link that reported the issue before,
https://groups.google.com/d/topic/joomla-dev-cms/bPsyhj55qcA/discussion

it is suggested to print the file and line only if debug is ON is wrong

• but printing this information should depend only on error_reporting variable

thus getFile and getLine should not be inside
if ($this->debug)
or inside
... renderBacktrace(), that prints the call stask and is used only when Joomla debug flag is ON

currently

• if you use error_reporting and PHP 5.x you can get the info about where the error occured
• if you use error_reporting and PHP 7.x you can NOT get the info about where the error occured

[EDIT]
because PHP7 "reports" the error by throwing an exception, and Joomla exception handling code does not retrieve this info anywhere

[mbabker]

All that can be done from the Joomla side is expand what data is displayed on the error page when it is given a Throwable. The fact that PHP core changed some language errors into catchable Throwable instances is why people seem to think that error handling/reporting has changed between PHP 7, but the internal behaviors of Throwable objects hasn't changed at all (the stack trace purposefully omits the line the object was thrown on, there was a lengthy discussion on the PHP Internals mailing list recently on this; and there is no method on Throwable that is a concatenation of getMessage(), getFile(), and getLine() so if you want to display all of that data you have to render it yourself).

The language changes most noticeably cause two behavioral differences:

1. Those issues will now be handled by any try/catch with a catch (Throwable $t) section or whatever callable you've registered by set_exception_handler($callable)
2. Behaviors that relied on PHP 5 style error handling (i.e. logging to PHP's error logs) aren't performed automatically now because those error objects are going into a userland error handler so it's up to the implementation to decide what to do (tests on Try to log errors that hit the global exception handler #10341 are welcome to get some logging from anything hitting the error handler set up)

thus getFile and getLine should not be inside if ($this->debug)

Yes they should. People freak out when file paths are displayed back to a user, even if they are sanitized beforehand. It shouldn't be a default behavior to spit this stuff out on the error page.

[ggppdk]

It shouldn't be a default behavior to spit this stuff out on the error page.

you are right, but i did not suggest such a thing in my last message

please look at the picture,
in my last messge, i suggested checking the error_reporting variable,
(my original code wrongly, did not not include check of error_reporting variable, my mistake !!)

[ggppdk]

Thus setting error_reporting to

• "development" and not getting the error to print makes no sense
• furthermore, if we place the getFile() getLine() code, to be printed only when Joomla debug Flag is ON, then what is error reporting variable used for ? or do you suggest moving it is usage inside ... code that checks if Joomla Debug is ON ?

i think these were and should remain indepedent

• developers and end users expect error_reporting parameter, to work regardless Joomla Debug Flag

[mbabker]

If a site has turned on error reporting (mistakenly or otherwise), if the checks are dependent only on the error reporting variable then the stack trace and error file/line are displayed. If it's left to only be displayed when debug mode is enabled, that's a more explicit action to display this extra information that's really only relevant when debugging the issue. It might seem bass ackwards, but given Joomla's user base isn't purely developers, it's better to rely on debug mode here versus error reporting, especially given how easily people claim that a file path is an information disclosure security issue.

[ggppdk]

Ok i understand your point

but do we want the Joomla debug to be always ON while we are developing ?

• since the Joomla DEBUG Consoles JS adds 0.5 - or up 3 seconds to large pages

even 0.5 (unneeded) seconds more on pages while work is a little annoying

anyway if such a behaviour, is introduced, then no problem for me

• i will just patch the error.php of the Joomla templates in my localhost dev environment

in any case getFile() and getLine() need to be added somewhere

• even if that is used when Joomla Debug is ON, so that users that need support have a way to get the full message

[ggppdk]

@mbabker

about this argument

If a site has turned on error reporting (mistakenly or otherwise) ....

Your argument has some logic as i acknowledged

---

But it makes little sense to print the file and line information for

• PHP notices
• PHP warnings

but hide these for fatal errors so that we make difficult for people to get support

---

OK, it is possible to enforce such a new behaviour for PHP warnings and notices, if it is decided that Joomla will override the PHP error handler with a custom one !

so what will happen ?

will Joomla override the PHP error handler to force removal of ... file / line in PHP notices and warnings

• and only show these if Joomla Debug parameter is ON ? (currently, we cannot ask people to turn ON Joomla debug, because file/line do not get printed in this case either, instead we must ask them to edit error.php file)

also will Joomla remove parameter error reporting ?
or it will use it only if Joomla DEBUG parameter is ON ?

I am only asking, this is not urgent, but this new behaviour of fatal errors will be more often in near future, as more users will have PHP7 on their web-sites

[mbabker]

At the end of the day usually what I suggest gets ignored anyway. But considering people groan and raise security issues about the fact error messages have SQL queries and file paths in them, I'd suggest NOT including those types of output on error pages without some sort of explicit action beyond enable/disable error reporting.

Get #10341 tested and you don't need to worry about this stuff showing up on screen. It'll write to the error logs.

As for replacing PHP's default error handler with a "custom" handler for Joomla, there are probably people who would say yes Joomla needs to do this to mask "sensitive info". But, also consider that the error handler is massively different than the callback function used for the exception handler (which prevents a fatal error about having an uncaught exception). So that's not just a simple "hey, an E_SOMETHING was thrown; trigger the error page" action as that function has to be smart enough to handle most error levels (exceptions are listed in the set_error_handler() documentation).

[ggppdk]

I will, thanks

[brianteeman]

Closed based on comments above and the presence of a PR for testing

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10732.}