The MySQL database connection part of the code emits error messages, which contain sensitive information about the server (eg. the file system path of the socket). Even if you disable display_errors, this is still displayed, though it should not. Here is a patch, which honours the display_errors setting and only emits the error message if the server admin's desire is to see error messages in the browser.
BTW this patch is against 4.4.x CVS, while this is also an issue in the HEAD version (but the install system changes are said to contain some remedy for nonexistent database connections). If it is desired, this can also be applied against HEAD.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | common.inc_1.patch | 1.34 KB | ax |
| Drupal-honour-display-errors-in-db-connect.patch | 903 bytes | gábor hojtsy |
Comments
Comment #1
ax commentedthere are more places where sensitive information is revealed via error messages. see SQL error for bad URL parameters for an example. thats why i think this issue should be handled more generally than only fixing the mysql connection part. that is, /all/ errors should only be displayed if display_errors in php.ini is set. patch (incl. some s#"#'#g) attached.
Comment #2
gábor hojtsyAx's patch is complementary to mine. The error handler is registered way after the database connection is attempted.
Comment #3
Kjartan commentedWhat are the odds of the install system making it for 4.5? I am not really up to speed on its development. If the install system is ready for 4.5 I don't see a reason to apply Goba's patch yet.
Ax: care to make a separate issue for your patch and explaining why you remove error_reporting()? With your patch when display_errors is enabled it will also show @function, which indicates that any error SHOULD be ignored.
Comment #4
Steven commentedWe now have our own toggle for displaying error messages or not, but of course it cannot be read with database access.
Still, I'm not entirely convinced about this patch. Installations with display_errors=off will offer admins no clue whatsoever as to what is wrong. Without these error messages, nothing gets logged at all (no watchdog possibility yet at that point), leaving admins no indication whatsoever of what went wrong.
Comment #5
gábor hojtsySo then we can have either a one line error message in the die (but definitely not the
mysql_error()output), or we can have a standarderror.htmlshipped with Drupal, which is a friendly error message to the user. Steven, you complain about admins not getting to know about the error, but mostly end users will see such an error message, and not the admins, and they should not see themysql_error()return value. Even if it does not contain sensitive information, it is far from friendly error handling.BTW on Weblabor.hu, we solved this problem with HTTP redirecting the visitor to http://weblabor.hu/hiba.html (which is our
error.htmlbasically) in case of this low level database error.Comment #6
Steven commentedFriendlier error messages and pages, okay. But I still think we should display the error string visually somehow, even to end-users. We cannot distinguish end-users from admins. I believe Drupal should offer enough meaningful information in every situation. Even if they can't understand what it means, at least they can copy-paste it so others can.
Otherwise we'll have to make a checklist of various things that can trigger a db error at that point, and paste it to anyone who asks about seeing such a vague error screen.
Comment #7
killes@www.drop.org commentedDoesn't apply anymore.
Comment #8
killes@www.drop.org commentedstill a patch
Comment #9
Jaza commentedIssue no longer relevant, since error_handler() no longer outputs error messages directly, it outputs them via drupal_set_message(). Closing issue.