Problem/Motivation
If the database connection is interrupted momentarily, and a database exception is thrown, there is a set of circumstances where a redirect to the installer page could be stored into the page cache if it uses a non-database storage. This will persist and redirect all users for a specific page to the installer page until the cache is cleared.
Steps to reproduce
Difficult to reproduce but with a specific forced failure point the problematic code paths can be demonstrated. Albeit, this forced failure is a specific exception that is unlikely to occur and the real cause is slightly more involved and explained after the steps to reproduce quickly.
1. Install page_cache and configure it to cache for 24 hours
2. Edit user module src/Form/UserLoginForm.php and throw an instance of \Drupal\Core\Database\DatabaseNotFoundException
3. Access /user/login using a CLI tool that shows headers
4. Note that X-Drupal-Cache: MISS and you receive a redirect
5. Access /user/login again
6. Note that X-Drupal-Cache: HIT and you receive the same redirect
Main contributor to the issue
The subscriber at core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php will see a redirect to the installer and see that it is not a SecuredRedirectResponse and will subsequently convert it to a LocalRedirectResponse. Unfortunately, this is cacheable, whereas the original redirect was not.
The redirect to the installer is generated by the subscriber in core/lib/Drupal/Core/EventSubscriber/ExceptionDetectNeedsInstallSubscriber.php. This generates a regular RedirectResponse. The fix may be for it to return a LocalRedirectResponse with max-age set to 0, or a TrustedRedirectResponse with cacheability also disabled.
Specific on the database failure, and a minor contributor
Looking at code it is attempting to avoid a redirect to the installer, as you can see in core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php in the method shouldRedirectToInstaller. This works in general except for the final check for tableExists on the sessions table, which will occur in the case of the database connection being lost and a PDOException being raised (MySQL in my case). However, this tableExists does not work.
The MySQL schema driver at core/lib/Drupal/Core/Database/Driver/mysql/Schema.php implements tableExists in a way that will never throw an exception. It will always assume any failure to be the table not existing. This causes the lost connection to generate the original installer redirect.
This issue won't occur for anyone running PgSQL or SQLite as those don't capture exceptions from the tableExists check.
Proposed resolution
1. Return a LocalRedirectResponse with no cacheability from the exception listener so that the page_cache module on the stack can be told not to cache it. *OR* Implement a new response that does not implement cacheability but does implement SecuredRedirectResponse - currently there are no implementations that do not implement CacheableResponseInterface.
(TBF I have realised the max-age 0 is ignored by page_cache currently still... so the latter may be better option, or a new response policy to deny this specific redirect.)
2. (Will raise in a follow up) Improve exception handling in MySQL schema so that it can correctly throw if there are connectivity issues, allowing the shouldRedirectToInstaller to behave in a better way and avoid showing installer pages to visitors when a MySQL database connection is interrupted
Remaining tasks
1. Fix caching issue of the installer redirect
2.
https://www.drupal.org/project/drupal/issues/3267207 →
(Related and alternate fix that may be blocked on MySQL issues:
https://www.drupal.org/project/drupal/issues/2797141 →
)
3. Write test coverage
4. update for trigger_error
User interface changes
None
API changes
None, unless a new redirect response is added that implements SecuredRedirectResponse but not CacheableResponseInterface
Data model changes
None
Release notes snippet
N/A