Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
If I set my site to display no system error messages to end users, under some circumstances, they can still be shown due to the fact that the setting itself typically comes from the database (though it is possible to set it in settings.php) and there is a window in between setting up the drupal error handler and reading this setting from either database or cache where error messages default to being shown to end users. This seems at best inconsistent, and at worst a potential security issue, depending on what is revealed by the error message (I'd hope nothing too serious would be in those messages, but you never know). Note that this was first noticed on a D7 site, but the same happens in D8 also.
A real world example of how this can cause problems is if your error_level is set in the database rather than in settings.php, and your site uses memcache - a not too unusual setup:
- _drupal_bootstrap_configuration sets the error handler to drupal's DSM handler. From this point until variables are initialized, any error will be shown to end users, due to the fact that the default in error_displayable is to show everything to all
- _drupal_bootstrap_page_cache attempts to initialize variables, but ...
- cache needs to be set up first, so cache is initialized. If this fails (memcache server is down), the error will be shown to all end users. A cache fail is not usually a catasrophic fail, so the site appears to be up, but error messages are displayed to end users, which is not what was intended if configuration has ERROR_REPORTING_HIDE
- In fact any error up to the point that the variable is loaded from either the cache or database will be displayed to users - that's a pretty big window
If you then assume that a site using memcache might also use a caching proxy, then things get fairly nasty, as the messages in $_SESSION for anonymous users will cause your caching proxy to not cache their requests (due to the session cookie).
Here's a simple way to simulate this kind of error:
1: Set errors to hidden: https://skitch.com/e-alanevans/er335/logging-and-errors-testd8err.localhost
2: Trigger an error some point before variables are loaded (and imagine that this could be something like a memcache server being down)
diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc
index 64872be..521d09e 100644
--- a/core/includes/bootstrap.inc
+++ b/core/includes/bootstrap.inc
@@ -2416,6 +2416,7 @@ function _drupal_bootstrap_variables() {
require_once DRUPAL_ROOT . '/core/includes/lock.inc';
// Load variables from the database, but do not overwrite variables set in settings.php.
+ trigger_error('TEST VARIABLES', E_USER_ERROR); // <--------
$conf = variable_initialize(isset($conf) ? $conf : array());
// Load bootstrap modules.
require_once DRUPAL_ROOT . '/core/includes/module.inc';3: load a page https://skitch.com/e-alanevans/er35h/welcome-to-testd8err.localhost-test...
Proposed resolution
I believe the solution is simply to set the default on displaying errors to hide them. It really depends though on whether drupal installed out of the box is considered a development setup, or a production setup. I think the safer assumption is the latter (because you can always enable error messages if you want them). At the moment, if you want to hide these error messages, you have to either do it in settings.php, which may not be practical, and may not be sufficient in extreme edge cases where an error is triggered before settings.php is available (I have no examples for this). I'll add a patch for this shortly. I think the approach needs some discussion though.
As a partial workaround you can always making sure your error_level is set in settings.php - that significantly reduces the window where errors can be displayed to end users. There is very little in the code in between setting up the error handler and loading settings from settings.php that could trigger errors (I'm sure it could be forced if you try really hard, but under normal circumstances shouldn't happen).
Remaining tasks
Determine whether the proposed solution is desired.
Review + revise.
User interface changes
If the change is accepted, error messages are assumed by default to be hidden until explicitly changed to show. Error logging should be unaffected, just the display in drupal_set_message()s.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 1671318-14.patch | 2.25 KB | rpayanm |
| #11 | interdiff.txt | 1.76 KB | reszli |
| #11 | 1671318-11.patch | 2.35 KB | reszli |
Comments
Comment #1
Alan Evans CreditAttribution: Alan Evans commentedPossible solution attached.
Comment #2
Alan Evans CreditAttribution: Alan Evans commentedD7 patch would look like something like this ...
Comment #3
ksenzeeThe patches in #1 and #2 look good to me but fail testing (the exception handler tests assume that exception text shows by default). Explicitly setting the variable in the exception handler tests fixes the problem, and makes more sense anyway. Attaching patches for 8.x and 7.x.
Comment #5
ksenzeeOops, missed a failing test.
Comment #6
jams1988 CreditAttribution: jams1988 commented#5: 1671318-5.hide-errors.patch queued for re-testing.
Comment #7.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #8
pwolanin CreditAttribution: pwolanin commentedComment #9
reszliRerolled for D8.
Comment #11
reszliFixed error introduced by the old patch in another file.
Comment #12
reszliComment #13
jhedstromComment #14
rpayanmComment #15
jhedstromFrom what I can tell, the actual fix in #5 got lost from #11 on, and all that remains is a test that is passing without the change in error handling.
Comment #16
pwieck CreditAttribution: pwieck commentedComment #20
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedLinking to an issue that I thought this issue might solve, but looking at the patch here it's actually something different.
Comment #22
dkre CreditAttribution: dkre commentedI ran into this bug with Swiftmailer library creating the error (a notice) which Drupal then displayed on 8.2.4.
The error_level being reported within function _drupal_get_error_level() is 'verbose' while my settings are 'none' (or ERROR_REPORTING_HIDE / 'hide') within admin/config/development/logging. I'm not sure that's helpful but I couldn't make sense of how this is being defined.
For anyone looking for a workaround the only option is to hack core - but it does prevent ugly backtraced error messages being displayed on your production site.
Add the lines with + at the start:
/Core/Includes/errors.inc
Line 230:
This will only display these errors to the administrator account.
Comment #34
quietone CreditAttribution: quietone at PreviousNext commentedChecking in on the issue that has not had discussion for 7 years.
Is this still relevant to Drupal 10?
Since we need more information to move forward with this issue, I am setting the status to Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!