User tests: Successful: Unsuccessful:
Hi Folks,
Currently the Joomla JLog class will default to using JLogLoggerMessagequeue class if a custom logger hasn't been added.
This means that by default something like: JLog::add("Some logging information here", JLog::INFO);
will output to the message queue on the front end. This is done regardless of the error_reporting level defined in the Joomla main config. So for example even with error_reporting set to none in the Joomla Global Configuration->Server settings error logging done with JLog where a component hasn't defined an alternative logger will print to the message queue.
This PR adds a check to the JLogLoggerMessagequeue to ensure that the error reporting level is not none before continuing to log the error. This will help prevent logging statements appearing on the front end of a site as part of the message queue.
Ensure your error reporting in your Joomla config is set to none. Then add something like:
JLog::add("Should this be here", JLog::INFO);
to the top of your templates index.php file and reload the page.
Before this patch is applied the following will be displayed on the page refresh.
Now apply the patch and still with error reporting set to none refresh the page. The log message should now not be present on the site.
I have only tested for error_reporting set to none. Should this tests for other error_reporting levels? If so how should we map the JLog levels to the Joomla config levels?
Cheers,
Eric.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
@mbabker at least in staging at present the JLogLoggerMessagequeue class is sending ALL the JLog error levels to the message queue.
case JLog::EMERGENCY:
case JLog::ALERT:
case JLog::CRITICAL:
case JLog::ERROR:
JFactory::getApplication()->enqueueMessage($entry->message, 'error');
break;
case JLog::WARNING:
JFactory::getApplication()->enqueueMessage($entry->message, 'warning');
break;
case JLog::NOTICE:
JFactory::getApplication()->enqueueMessage($entry->message, 'notice');
break;
case JLog::INFO:
JFactory::getApplication()->enqueueMessage($entry->message, 'message');
break;
That was just taken out of the JLogLoggerMessagequeue.
The test I did was using JLog::INFO and that was being displayed in the message queue. The PR won't change adding items to the messagequeue. It will just change adding items to the messagequeue via JLog.
Eric
At https://github.com/joomla/joomla-cms/blob/3.4.4/libraries/cms.php#L58 the message queue is only set up for log items of category "jerror". Whether this is right or not can be debated, but essentially anything that gets logged in that category will also be pushed to the message queue (consider it a poor man's shortcut for logging an error and displaying a message to the user in one call). By design, this JLog call will hit the message queue while the log statements in JDatabaseDriverMysqli::execute() will not forward to the message queue because it is the incorrect category. Since JLog::add()
does not have a default category, I'm not following what issue you're trying to address since your given examples should not be matching the message queue logger's criteria.
As for the error_reporting
configuration value, its only use is to set the PHP error level reporting (see https://github.com/joomla/joomla-cms/blob/3.4.4/includes/framework.php#L51-L90 for that). That setting does not correspond to what error levels are logged or what messages are enqueued and displayed by the application.
Hey Michael, I understand what you are saying and according to the docs what you are saying is how it should be. (https://docs.joomla.org/Using_JLog).
But.... The issue is that without specifying a category when calling JLog::add the entry is being added to the messagequeue regardless of priority.
I've attached three screenshots where I've used:
So this PR is trying to go about this the wrong. Maybe what I should have said, and where I should be looking is that calling JLog::add WITHOUT a category is still displaying the message to the user, despite the docs saying that this shouldn't happen.ie. the docs say that the message should ONLY be displayed to the user when jerror is adding to the category.
I've tested this on both a current J3.4.3 install and against the staging branch from git. For both I have added JLog::add to the template's index.php file without a category and the message is displayed to the user.
Can you maybe just do me a quick favour and just double check if the same thing happens to you? ie. if you add JLog::add("This message shouldn't show", JLog::ERROR) to your templates index.php file do you get the same thing?
Eric.
Category | ⇒ | Libraries |
Easy | No | ⇒ | Yes |
It does that but your approach only places a bandaid on the solution and doesn't actually fix anything long term. When you set no category, this condition is met and the message is queued for entry to the logs. So your bug isn't specific to the message queue logger but a bug in the base JLog object which would affect all log types (the only reason you see it with the message queue logger is because of our adding the message queue logger to the bootstrap).
Hi Michael
Agreed. I was looking in the wrong place and you are absolutely right it is just a band aid solution. I didn't dig deep enough.
So we are both on the same page and this shouldn't be happening and i will review the JLog class for a better fix.
Does that sound like a better idea?
Eric.
Now that I traced it out I see where you're hitting the issue and it's indeed a bug.
Thanks i didn't dig deep enough when i opened this.
When i get to work i will close this and open a more descriptive issue through the issue tracker.
Eric.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-10-14 23:51:38 |
Closed_By | ⇒ | digitalgarage |
I don't think this is right at all. The error reporting level set in the
global config doesn't control the items that get queued for messages.
Also, IIRC, the default configuration is to use that logger only for
"jerror" category messages.
On Tuesday, October 13, 2015, Eric notifications@github.com wrote: