Followup of: #1958680: Drupal 8 does not install with safe_mode = On

It turns out that when safe_mode is used, Drupal installation will currently fail when visiting /core/install.php.
Fatal error: Uncaught exception 'Drupal\Core\Config\StorageException' with message 'Missing configuration file: system.logging' in /home/sddf226a7261cfa9/www/core/lib/Drupal/Core/Config/InstallStorage.php:54 Stack trace: #0 /home/sddf226a7261cfa9/www/core/lib/Drupal/Core/Config/FileStorage.php(73): Drupal\Core\Config\InstallStorage->getFilePath('system.logging') #1 /home/sddf226a7261cfa9/www/core/lib/Drupal/Core/Config/FileStorage.php(82): Drupal\Core\Config\FileStorage->exists('system.logging') #2 /home/sddf226a7261cfa9/www/core/lib/Drupal/Core/Config/Config.php(410): Drupal\Core\Config\FileStorage->read('system.logging') #3 /home/sddf226a7261cfa9/www/core/lib/Drupal/Core/Config/Config.php(204): Drupal\Core\Config\Config->load() #4 /home/sddf226a7261cfa9/www/core/includes/errors.inc(156): Drupal\Core\Config\Config->get('error_level') #5 /home/sddf226a7261cfa9/www/core/includes/bootstrap.inc(2271): error_displayable() #6 [internal function]: _drupal_exception_handler(Object(Drupal\Core\Config\StorageException)) #7 {main} in /home/sddf226a7261cfa9/www/core/lib/Drupal/Core/Config/InstallStorage.php on line 54

Proposed solution

It took me a while to figure out that the use of safe_mode was the actual issue here,
so I'd propose to check the requirement to have safe_mode Off very early in the installation process.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

Category: feature » task

Switching to a task, since there's no reason for this to be held up by thresholds.

Great investigation work!

patrickd’s picture

Status: Active » Needs review
FileSize
588 bytes

This patch adds a simple "if (ini_get('safe_mode'))" check directly after the PHP version requirement was checked

patrickd’s picture

To test the patch:

- Apply it locally (where you have safe_mode turned off by default)
=> Installation should work as always

- Either turn "safe_mode = On" in your php.ini file
- Or as simplytest.me is currently using safe_mode itself you can test it directly:
http://simplytest.me/project/drupal/8.x?patch[]=http://drupal.org/files/...
=> On the installation page a warning about the requirement should show up

penyaskito’s picture

Status: Needs review » Reviewed & tested by the community

Tested it locally and with simpletest.me and the user message is more user friendly than a WSOD. RTBCing.

frankcupid’s picture

#2: check_safe_mode-1959062-2.patch queued for re-testing.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Safe mode is deprecated in PHP 5.3 and completely removed in PHP 5.4. I'm OK with failing early in case there's a 5.3 installation with it still enabled, but we should add a comment to remove this check again once we require > 5.4.

patrickd’s picture

Status: Needs work » Needs review
FileSize
719 bytes

Okay, added a @todo

BUT it also seems like the same error mentioned in the issue summary happens when "open_basedir" is used. open_basedir is AFIAK not deprecated in PHP .. is there a consensus about the compatibility of drupal with the open_basedir option?

JakeWilund’s picture

I've applied the patch in #7 and receive the following error when attempting installation:

Additional uncaught exception thrown while handling exception.

Original

Drupal\Core\Config\StorageException: Missing configuration file: system.filter in Drupal\Core\Config\InstallStorage->getFilePath() (line 54 of /var/www/clients/client1/web2/web/core/lib/Drupal/Core/Config/InstallStorage.php).

Additional

Drupal\Core\Config\StorageException: Missing configuration file: system.filter in Drupal\Core\Config\InstallStorage->getFilePath() (line 54 of /var/www/clients/client1/web2/web/core/lib/Drupal/Core/Config/InstallStorage.php).

patrickd’s picture

@JakeWilund did you have safe_mode activated when you tested it? does the installation work without having the patch applied?

JakeWilund’s picture

I tried installation prior to applying the patch and received the same error. I then proceeded to set safe_mode in my php.ini, but then received an error stating that safe_mode has been deprecated in PHP 5.4 (which I didn't realize my server was running up until that point).

So then I attempted installation after applying the patch, and that's where I'm at now. Back to the original error that I posted above.

patrickd’s picture

If you get this error without having the patch applied and without having safe_mode enabled this has nothing to do with this issue.
Please search the queue for similar bug reports / ask on IRC for help / create a new bug report with your error and your server configuration.

patrickd’s picture

btw, @JakeWilund can you have a look into your php.info and tell me whether "open_basedir" is set? (See comment #7)

JakeWilund’s picture

open_basedir is commented out, so not set.

I posted in this issue because it was the closest I could find to the issue I am having. Unless you think that the open_basedir option has something to do with the error I am getting, I will open a new issue as you suggested.

patrickd’s picture

hmm, when it's not set then it probably hasn't something to do with this.. but your right, the errors are similar

brad.bulger’s picture

i've confirmed that having open_basedir set causes a similar exception:

Additional uncaught exception thrown while handling exception.

Original

Drupal\Core\Config\StorageException: Missing configuration file: system.theme.global in Drupal\Core\Config\InstallStorage->getFilePath() (line 54 of /var/www/d8/nix/drupal/core/lib/Drupal/Core/Config/InstallStorage.php).

Additional

Drupal\Core\Config\StorageException: Missing configuration file: system.site in Drupal\Core\Config\InstallStorage->getFilePath() (line 54 of /var/www/d8/nix/drupal/core/lib/Drupal/Core/Config/InstallStorage.php).

that's with open_basedir set to /var/www/

patrickd’s picture

Title: Check safe_mode = Off requirement early » Check safe_mode = Off / open_basedir requirement early
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Great job! thank you.

So we now have to enhance the patch of #7 and add the same check for open_basedir in.

patrickd’s picture

Issue tags: +Novice

tagging with novice

brad.bulger’s picture

Title: Check safe_mode = Off / open_basedir requirement early » Check safe_mode = Off requirement early
Status: Needs work » Needs review
Issue tags: -Needs issue summary update, -Novice

btw, testing that glob() has not returned FALSE in InstallStorage->getComponentNames() prevents that exception. that looks like a PHP bug - https://bugs.php.net/bug.php?id=47358

instead, the installation fails in "Verify requirements" (core/install.php?langcode=en&profile=standard) with the message

The service definition "request" does not exist.

it seemed worth looking into why it was failing, since as mentioned in #7 open_basedir is not deprecated, but that was all i found.

brad.bulger’s picture

Title: Check safe_mode = Off requirement early » Check safe_mode = Off / open_basedir requirement early
Issue tags: +Needs issue summary update, +Novice
FileSize
1.04 KB

this is an update of the patch to include a similar check of open_basedir, if in fact that's what you all want to do.

patrickd’s picture

Status: Needs review » Reviewed & tested by the community

I just talked about this with chx and he also thinks that we should not specifically support open_basedir because it's just not really secure and you can bypass it with most of the commonly used php extensions. So if we don't specifically support open_basedir and drupal does not work with it turned on - just drop it completely.

Patch looks good for me!

thanks!

John Pitcairn’s picture

Won't that be a problem for a lot of hosting environments, where open_basedir use is mandated by the hosting provider, or by corporate IT requirements? It may be hard to convince IT departments that their requirement of a non-deprecated PHP "safety" feature should be relaxed just to support Drupal 8.

patrickd’s picture

These hosting providers have to understand that this isn't a "safety" feature, because it can be bypassed stupidly simple (just google for open_basedir).

I'm also a little sad that there are currently no very good ways to secure php environments without using a virtual-machine or kernel-virualization technology. But that's just the way it is

Status: Reviewed & tested by the community » Needs work

The last submitted patch, check_safe_mode-1959062-19.patch, failed testing.

John Pitcairn’s picture

Sadly, the hosts and (especially) company IT crew tend to just mandate policy and that's it. Changing their minds can be impossible. In the case of hosting providers we can always switch, but for some of the more entrenched company-hosted environments we will most likely just be stuck with it.

It's going to be hard to tell whoever is in charge of the shiny new Drupal project that their IT department won't let Drupal 8 run because it requires an "insecure" (their words) server setup. We can't claim it's a deprecated feature, and whether it actually provides any meaningful security will be largely irrelevant in that discussion. Just sayin'.

patrickd’s picture

Status: Needs work » Needs review

#19: check_safe_mode-1959062-19.patch queued for re-testing.

brad.bulger’s picture

Status: Needs review » Reviewed & tested by the community

last requeue for testing passed

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

I can see the sense if bailing out with safe_mode being ON as that is deprecated in 5.3 and gone in 5.4... but I feel uncomfortable with the open_basedir change.

Putting back to needs review for more discussion. Also I would not be against this issue re-focussing to just the safe_mode check (as it was originally) and creating a new issue to discuss the open_basedir problem.

catch’s picture

I'd be fine with splitting the issue.

With open_basedir can we do something like read the current value of open_basedir, remove the current working directory, set it back with ini_set(), then check a hook_requirements() warning up?

patrickd’s picture

Title: Check safe_mode = Off / open_basedir requirement early » Check safe_mode = Off requirement early
Issue tags: -Novice
FileSize
719 bytes

Created a separate issue for open_basedir: #2019911: Add install requirement that open_basedir = Off since 8.x currently doesn't support it

Re-adding the safe_mode = Off patch of comment #7

Can we switch this back to RTBC then? (when patch stays green, what it most likely will)

brad.bulger’s picture

Status: Needs review » Reviewed & tested by the community

i'd think so. there and back again.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 00155e5 and pushed to 8.x. Thanks!

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