eliminate OC_Template::printErrorPage in database classes, fixes #12182 #12186
Conversation
|
Mhm. Any specific reason to throw an hint exception? - I'd either use the generic one or create a new one. |
| @@ -71,7 +72,7 @@ public function insertIfNotExist($table, $input) { | |||
| $entry .= 'Offending command was: ' . $query.'<br />'; | |||
| \OC_Log::write('core', $entry, \OC_Log::FATAL); | |||
| error_log('DB error: ' . $entry); | |||
There was a problem hiding this comment.
we can also kill error_log() - would be written to the log twice then
|
@LukasReschke because a HintExecption will end up with the same output (if not caught before). |
Yes - but do we really need to print information about database errors to the user? - This can lead to nasty bugs such as #11325 I'd prefer if we would just show the generic error page - getting more information is then always possible via the log. |
|
@LukasReschke mh, not, we can keep it, but we pass just "Database error" and "ask your admin" and pass the original exception to the hint one. So DB information will not be leaked to users, but apps / core can react upon the original exception with full details. |
we have to be careful in changing the exception type to be thrown because of the potential to change the behavior regarding the status code. |
|
Ok. Convinced then ;-) |
|
→ removed information leak |
|
The inspection completed: 1 new issues |
|
Sometimes I think this bot is just an invention of @DeepDiver1975 to make us developers suffer for untested code 😈 ;-) |
|
@owncloud-bot Retest this please. |
|
🚀 Test PASSed. 🚀 |
|
👍 |
1 similar comment
|
👍 |
eliminate OC_Template::printErrorPage in database classes, fixes #12182
as discussed. I replaced all occurences in lib/private/db.
@MorrisJobke @DeepDiver1975 @LukasReschke