Use PHP7 and J3.5.1 (or latest staging)
add a syntax or other fatal error in any PHP file
If error reporting is ON then at least the FILE and LINE that produced the fatal error should be shown
The file and line that created the fatal error, are not shown
PHP7.x
J3.5.x (or latest staging)
The reason is that we do not let the PHP7 default error handler to produces it own output,
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 ?
Labels |
Added:
?
|
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.
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
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
[EDIT]
because PHP7 "reports" the error by throwing an exception, and Joomla exception handling code does not retrieve this info anywhere
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 #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.
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 !!)
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
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.
Ok i understand your point
but do we want the Joomla debug to be always ON while we are developing ?
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
in any case getFile() and getLine() need to be added somewhere
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
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
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
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).
I will, thanks
Category | ⇒ | Templates (admin) Templates (site) |
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-08-03 16:18:10 |
Closed_By | ⇒ | brianteeman |
Closed based on comments above and the presence of a PR for testing
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
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 ?: