Updated: Comment #0

Problem/Motivation

At the moment whether or not your Drupal site is in maintenance mode is stored in the config system in the system.maintenance object. This means that in order to keep your site in maintenance mode during a config import then you must import a file with enabled set to 1. Then once you've finished your import and made sure everything is ok you'll go to the form and take the site out of maintenance mode - which means we've created a unnecessary difference between the snapshot and active.

The converse is even more problematic. If you put your site into maintenance mode and then import config where the value is 0 a some point during the import your site will be taken out of maintenance mode! I think this makes this issue critical.

Proposed resolution

Move system.maintenance.enabled to the state system.

Remaining tasks

Review code

User interface changes

None

API changes

-  if (Drupal::config('system.maintenance')->get('enabled')) {
+  if (Drupal::state()->get('system.maintenance_mode')) {
CommentFileSizeAuthor
#5 2084339-2.patch13.22 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

I'm not entirely convinced this is critical, though it would make maintenance mode behave in a buggy fashion for sure. @alexpott suggested that maintenance mode behaving unreliably could have security implications. In any case it shouldn't be too hard to patch.

mtift’s picture

mtift’s picture

Assigned: Unassigned » mtift

I'll take this one

alexpott’s picture

Assigned: mtift » Unassigned
Status: Active » Needs review
FileSize
13.22 KB

Here's a patch

alexpott’s picture

@mtift - really sorry I'd already written the patch and didn't notice your comment :(

catch’s picture

Status: Needs review » Reviewed & tested by the community
  // @todo This is borderline state, not config. Or a global system
-    //   "maintenance lock".

Yes, yes it is.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome, this makes total sense.

Tested manually and no problems.

Committed and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.