User tests: Successful: Unsuccessful:
This PR implements the hide of the SQL Table Prefix from error messages if the webseite is not in debugging mode.
In debugging mode we get the full error message.
#__
This implements the good idea by @ryandemmer from here: #8129 Thanks.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
@phproberto what do you think about $this->getErrorNum
(returns the error num) and $this->getErrorMsg
which returns the cleaned up error message?
protected function getErrorNum()
{
return (int) mysql_errno($this->connection);
}
protected function getErrorMsg()
{
$errorMessage = (string) mysql_error($this->connection);
// Replace the Databaseprefix with `#__` if we are not in Debug
if (!$this->debug)
{
$errorMessage = str_replace($this->tablePrefix, '#__', $errorMessage);
}
return $errorMessage . ' SQL=' . $this->query;
}
// Get the error number and message before we execute any more queries.
$errorNum = $this->getErrorNum();
$errorMsg = $this->getErrorMsg();
Just done @phproberto Thanks for your tip can you review?
Category | ⇒ | Libraries MS SQL Postgresql SQL |
Easy | No | ⇒ | Yes |
Could you post easy ways to get different error messages?
Found. Just modified some queries in the model.
Getting now the Error fine (test on msqli).
Just a remark:
When debug is off no prefix is displayed at all = fine.
When debug is on, it only partly works:
If a table does not exist in the query, prefix is displayed only for the table concerned, example when debug is on, the prefix is displayed, but only in
exception 'RuntimeException' with message 'Table 'master.jos_contento' doesn't exist SQL=
in the remaining SQL part the prefix is absent
[...] FROM #__contento AS a LEFT JOIN #__categories [...]
If the message does not concern a non-existing table, then the prefix is always absent.
I have tested this item successfully on ffb8ef8
Hi, tested and the patch works OK for me.
Jan
I have tested this item successfully on ffb8ef8
Works as described. Good call, hiding that bit of information.
I have tested this item unsuccessfully on ffb8ef8
Only half of the Test was Successfully:
In Normale Mode the all prefixes been replaced with #__
in debug mode I would expect that I see all prefixes again - that was not the case:
expact Table 'db_name.tableprefix_bannerse' doesn't exist SQL=UPDATE tableprefix_bannerse SET clicks = (clicks + 1) WHERE id = 1
got Table 'db_name.tableprefix_bannerse' doesn't exist SQL= UPDATE #__bannerse SET clicks = (clicks + 1) WHERE id = 1
Testet on Joomla! 3.4.5 changed line 43 in components/com_banners/models/banner.php from ->update('#__banners')
to ->update('#__bannerse')
@infograf768 @Bakual @sudo-web @coolman01, @PhocaCz, @pos-it please retest.
Now it should work if you enable the Debug the raw SQL should shown
I have tested this item successfully on ffb8ef8
Tested on two code places again.
Now it works as expectet!
One Additional Remark....
It hast not directly to do with this patch but in some scenarios the error output included the full name of the database.
Table 'db_name.#__bannerse' doesn't exist...
I would say that is a kind of security issue.
Should I open a new Issue for that?
hmm you can't do anything with that information. For sure we can filter it out but I'm not sure if it makes sense.
All actions Joomla does goes to that database if you know the databse name or not. The prefix is that generic you need to access the tables as that should be unique. Maybe @Bakual can give a option here on filter it out or not ;)
This PR has received new commits.
CC: @coolman01, @PhocaCz, @pos-it, @sudo-web
@sudo-web i have just fixed Travis and alterd your test reusult.
This PR has received new commits.
CC: @coolman01, @PhocaCz, @pos-it, @sudo-web
Fixed it up again
@zero-24
Although I do not really think we need to get the prefix in debug mode, I managed to get it by changing the method to:
protected function getErrorMessage()
{
$errorMessage = (string) mysqli_error($this->connection);
// Replace the Databaseprefix with `#__` if we are not in Debug
if (!$this->debug)
{
$errorMessage = str_replace($this->tablePrefix, '#__', $errorMessage);
}
else
{
$this->sql = str_replace('#__', $this->tablePrefix, $this->sql);
}
return $errorMessage . ' SQL=' . $this->sql;
}
My last post above was done before I saw the changes in the PR. Will test again the PR as is.
tested PR and all works ok now in debug mode
Milestone |
Added: |
||
Status | Pending | ⇒ | Ready to Commit |
Milestone |
Added: |
Thanks setting RTC for 3.4.6
If You think we should filter the database out let me know. Than i can open a new PR that implement that :)
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-10-25 10:30:37 |
Closed_By | ⇒ | rdeutz |
Labels |
Removed:
?
|
If You think we should filter the database out let me know.
We don't need that. An attacker doesn't need to know that anyway as Joomla will query that database by default anyway. So attackers just don't care as they want to query the default database anyway.
Milestone |
Milestone |
Added: |
Milestone |
Added: |
Milestone |
I think is better that you have reusable functions like
showErrorMessage()
. It will avoid duplicated code and allow us to centralise error messages control there.