[#14215] - [com_fields] p3.7b3] Invalid SQL query leaks mysql raw error
- Closed
- 23 Feb 2017
- Medium
- Build: staging
- # 14215
Steps to reproduce the issue
reopening of #13882
Configure a SQL field with a crap sql query
Like:
Expected result
That no structure of my mysql tables is leaked to end users no matter what error SQL comes up with
Actual result
Leaking of parts of the SQL query, which may contain field names, and the raw mysql error message
System information (as much as possible)
3.7.b3 with #14214 PR applied
joomla-cms-bot
- | Labels |
Added:
?
|
||
My issue here is: The error message should NOT leak information about the structure of the mysql database, its field/column names....
It should just say that the SQL provided by the Admin is invalid and could not be run by mysql.
Exceptions are supposed to have developer related information. That's why I'm saying don't touch it.
The problem is, as with everything in Joomla, there's no real handling of the error condition and the raw data is just thrown back. Even worse the field is trying to catch all Exceptions versus only Exceptions for which it knows how to handle/recover from.
So:
- Does this catch really need to enqueue a message to the session? (IMO no)
- Does this catch really need to catch Exception and every subclass of it? (IMO no)
- Someone do a pull request addressing point 2 so only database related Exceptions are caught and if it is decided that point 1 needs to stand to use a different message other than the Exception message.
Error handling really isn't that hard...
As a dumb frontend user, I should not be handed developer exceptions. I should be given dumb error messages that tell me to contact the super admin because there was a problem running the sql.
As a dumb frontend user, I should not be presented with details of that SQL query, including the actual query, mysql column names or anything else that the query contains.
:facepalm:
Phil, WE AGREE! I'm saying someone change the enqueued message from the Exception message to something user friendly. DO NOT CHANGE THE EXCEPTION MESSAGES AT ALL.
Or, go ahead and change all the exception messages in the database layer so that no developer could ever actually debug their database issues because "dumb user might accidentally see this message somewhere". Your call.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-02-23 19:09:53 |
| Closed_By | ⇒ | zero-24 |
The Exception message is fine. Nobody change that.
Your issue is the Exception is caught and the message displayed as is. There's no converting it to a user friendly or filtered message happening, because the field is just catching everything that gets thrown. Why does it even need to enqueue a message to display to the user here!?