diff --git a/lib/base.php b/lib/base.php index 7674c16c26b5b..be2457ba9e9e0 100644 --- a/lib/base.php +++ b/lib/base.php @@ -412,7 +412,20 @@ public static function initSession() { ini_set('session.cookie_path', $cookie_path); // Let the session name be changed in the initSession Hook - $sessionName = OC_Util::getInstanceId(); + try { + $sessionName = OC_Util::getInstanceId(); + } catch (\OC\HintException $e) { + $error = [ + 'error' => $e->getMessage(), + 'hint' => $e->getHint(), + ]; + + http_response_code(503); + OC_Util::addStyle('guest'); + OC_Template::printGuestPage('', 'error', ['errors' => [$error]]); + + exit; + } try { // Allow session apps to create a custom session object diff --git a/lib/private/Config.php b/lib/private/Config.php index f462bebaf5887..5cccdd5558337 100644 --- a/lib/private/Config.php +++ b/lib/private/Config.php @@ -237,7 +237,20 @@ private function writeData() { $content .= var_export($this->cache, true); $content .= ";\n"; - touch ($this->configFilePath); + // Check if config directory/file is writable and/or create the config file + if ( + (!is_file($this->configFilePath) && !is_writable(dirname($this->configFilePath))) + || !touch($this->configFilePath) + || !is_writable($this->configFilePath) + ) { + // TODO fix this via DI once it is very clear that this doesn't cause side effects due to initialization order + // currently this breaks app routes but also could have other side effects especially during setup and exception handling + $url = \OC::$server->getURLGenerator()->linkToDocs('admin-dir_permissions'); + throw new HintException( + "Can't write into config directory!", + 'This can usually be fixed by giving the webserver write access to the config directory. See ' . $url); + } + $filePointer = fopen($this->configFilePath, 'r+'); // Prevent others not to read the config diff --git a/tests/lib/ConfigTest.php b/tests/lib/ConfigTest.php index 2a4c962034079..92753b7f2836e 100644 --- a/tests/lib/ConfigTest.php +++ b/tests/lib/ConfigTest.php @@ -160,4 +160,63 @@ public function testConfigMerge() { unlink($additionalConfigPath); } + public function testExceptionOnConfigDirNotWritable() { + $notWritableDir = $this->randomTmpDir.'not_writable/'; + mkdir($notWritableDir, 0500); + if (is_writable($notWritableDir)) { + $this->markTestSkipped("Couldn't ensure that the test directory is not writable"); + } + + $config = new \OC\Config($notWritableDir, 'testconfig.php'); + + $this->expectException(\OC\HintException::class); + + $config->setValue('foobar', 'baz'); + } + + public function testExceptionOnConfigFileNotWritable() { + $configFile = $this->randomTmpDir.'not_writable_config.php'; + touch($configFile); + chmod($configFile, 0400); + if (is_writable($configFile)) { + $this->markTestSkipped("Couldn't ensure that the test file is not writable"); + } + + $config = new \OC\Config($this->randomTmpDir, 'not_writable_config.php'); + + $this->expectException(\OC\HintException::class); + + $config->setValue('foobar', 'baz'); + } + + public function testNoExceptionOnConfigDirNotWritableButConfigFileWritable() { + $notWritableDir = $this->randomTmpDir.'not_writable/'; + mkdir($notWritableDir, 0700); + + $configFile = $notWritableDir.'writable_config.php'; + touch($configFile); + chmod($configFile, 0600); + if (!is_writable($configFile)) { + $this->markTestSkipped("Couldn't ensure that the test file is writable"); + } + + chmod($notWritableDir, 0500); + if (is_writable($notWritableDir)) { + $this->markTestSkipped("Couldn't ensure that the test directory is not writable"); + } + + $config = new \OC\Config($notWritableDir, 'writable_config.php'); + $config->setValue('foobar', 'baz'); + + $this->assertTrue(true, 'No exception when writing to config file'); + + chmod($notWritableDir, 0700); + } + + public function testNoExceptionOnCreatingConfigFileInWritableConfigDir() { + $config = new \OC\Config($this->randomTmpDir, 'this_file_does_not_exist.config.php'); + $config->setValue('foobar', 'baz'); + + $this->assertTrue(true, 'No exception when creating config file'); + } }