Add flock to config

This adds a file lock to the config in hope that this prevents race conditions as reported in https://github.com/owncloud/core/issues/11070

Testplan:

- [ ] Delete config.php and make it read-only => Error is thrown that it is not writeable
- [ ] Installation still works
- [ ] Changing config settings works (i.e. using the SMTP config switches in the administration menu)
- [ ] Your PC didn't blow up
- [ ] Installing the news app and the "Disable AppCode checker" app did not destroy your installation

Only skip the main config

Otherwise read only additional configs might not be processed

Test on tmpdir

3 files changed, 181 insertions, 123 deletions

diff --git a/lib/private/config.php b/lib/private/config.php
index 82a1c46c9d5..7bf3863e2a6 100644
--- a/lib/private/config.php
+++ b/lib/private/config.php
@@ -1,37 +1,12 @@
 <?php
 /**
- * ownCloud
- *
  * @author Frank Karlitschek
  * @author Jakob Sack
  * @copyright 2012 Frank Karlitschek frank@owncloud.org
  *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
- *
- * You should have received a copy of the GNU Affero General Public
- * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-/*
- *
- * An example of config.php
- *
- * <?php
- * $CONFIG = array(
- *     "database" => "mysql",
- *     "firstrun" => false,
- *     "pi" => 3.14
- * );
- * ?>
- *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
  */
 
 namespace OC;
@@ -41,26 +16,45 @@ namespace OC;
  * configuration file of ownCloud.
  */
 class Config {
-	// associative array key => value
+	/** @var array Associative array ($key => $value) */
 	protected $cache = array();
-
+	/** @var string */
 	protected $configDir;
-	protected $configFilename;
-
+	/** @var string */
+	protected $configFilePath;
+	/** @var string */
+	protected $configFileName;
+	/** @var bool */
 	protected $debugMode;
 
 	/**
-	 * @param string $configDir path to the config dir, needs to end with '/'
+	 * @param string $configDir Path to the config dir, needs to end with '/'
+	 * @param string $fileName (Optional) Name of the config file. Defaults to config.php
 	 */
-	public function __construct($configDir) {
+	public function __construct($configDir, $fileName = 'config.php') {
 		$this->configDir = $configDir;
-		$this->configFilename = $this->configDir.'config.php';
+		$this->configFilePath = $this->configDir.$fileName;
+		$this->configFileName = $fileName;
 		$this->readData();
-		$this->setDebugMode(defined('DEBUG') && DEBUG);
+		$this->debugMode = (defined('DEBUG') && DEBUG);
+	}
+
+	/**
+	 * Enables or disables the debug mode
+	 * @param bool $state True to enable, false to disable
+	 */
+	public function setDebugMode($state) {
+		$this->debugMode = $state;
+		$this->writeData();
+		$this->cache;
 	}
 
-	public function setDebugMode($enable) {
-		$this->debugMode = $enable;
+	/**
+	 * Returns whether the debug mode is enabled or disabled
+	 * @return bool True when enabled, false otherwise
+	 */
+	public function isDebugMode() {
+		return $this->debugMode;
 	}
 
 	/**
@@ -128,27 +122,43 @@ class Config {
 	 * Loads the config file
 	 *
 	 * Reads the config file and saves it to the cache
+	 *
+	 * @throws \Exception If no lock could be acquired or the config file has not been found
 	 */
 	private function readData() {
-		// Default config
-		$configFiles = array($this->configFilename);
-		// Add all files in the config dir ending with config.php
-		$extra = glob($this->configDir.'*.config.php');
+		// Default config should always get loaded
+		$configFiles = array($this->configFilePath);
+
+		// Add all files in the config dir ending with the same file name
+		$extra = glob($this->configDir.'*.'.$this->configFileName);
 		if (is_array($extra)) {
 			natsort($extra);
 			$configFiles = array_merge($configFiles, $extra);
 		}
+
 		// Include file and merge config
 		foreach ($configFiles as $file) {
-			if (!file_exists($file)) {
+			if(!@touch($file) && $file === $this->configFilePath) {
+				// Writing to the main config might not be possible, e.g. if the wrong
+				// permissions are set (likely on a new installation)
 				continue;
 			}
+			$filePointer = fopen($file, 'r');
+
+			// Try to acquire a file lock
+			if(!flock($filePointer, LOCK_SH)) {
+				throw new \Exception(sprintf('Could not acquire a shared lock on the config file %s', $file));
+			}
+
 			unset($CONFIG);
-			// ignore errors on include, this can happen when doing a fresh install
-			@include $file;
-			if (isset($CONFIG) && is_array($CONFIG)) {
+			include $file;
+			if(isset($CONFIG) && is_array($CONFIG)) {
 				$this->cache = array_merge($this->cache, $CONFIG);
 			}
+
+			// Close the file pointer and release the lock
+			flock($filePointer, LOCK_UN);
+			fclose($filePointer);
 		}
 	}
 
@@ -157,6 +167,8 @@ class Config {
 	 *
 	 * Saves the config to the config file.
 	 *
+	 * @throws HintException If the config file cannot be written to
+	 * @throws \Exception If no file lock can be acquired
 	 */
 	private function writeData() {
 		// Create a php file ...
@@ -168,18 +180,34 @@ class Config {
 		$content .= var_export($this->cache, true);
 		$content .= ";\n";
 
-		// Write the file
-		$result = @file_put_contents($this->configFilename, $content);
-		if (!$result) {
-			$defaults = new \OC_Defaults;
+		touch ($this->configFilePath);
+		$filePointer = fopen($this->configFilePath, 'r+');
+
+		// Prevent others not to read the config
+		chmod($this->configFilePath, 0640);
+
+		// File does not exist, this can happen when doing a fresh install
+		if(!is_resource ($filePointer)) {
 			$url = \OC_Helper::linkToDocs('admin-dir_permissions');
 			throw new HintException(
 				"Can't write into config directory!",
 				'This can usually be fixed by '
-					.'<a href="' . $url . '" target="_blank">giving the webserver write access to the config directory</a>.');
+				.'<a href="' . $url . '" target="_blank">giving the webserver write access to the config directory</a>.');
 		}
-		// Prevent others not to read the config
-		@chmod($this->configFilename, 0640);
+
+		// Try to acquire a file lock
+		if(!flock($filePointer, LOCK_EX)) {
+			throw new \Exception(sprintf('Could not acquire an exclusive lock on the config file %s', $this->configFilePath));
+		}
+
+		// Write the config and release the lock
+		ftruncate ($filePointer, 0);
+		fwrite($filePointer, $content);
+		fflush($filePointer);
+		flock($filePointer, LOCK_UN);
+		fclose($filePointer);
+
+		// Clear the opcode cache
 		\OC_Util::clearOpcodeCache();
 	}
 }
diff --git a/lib/private/legacy/config.php b/lib/private/legacy/config.php
index 4d6fefc4e17..13ff0dbe040 100644
--- a/lib/private/legacy/config.php
+++ b/lib/private/legacy/config.php
@@ -6,32 +6,9 @@
  * @author Jakob Sack
  * @copyright 2012 Frank Karlitschek frank@owncloud.org
  *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
- *
- * You should have received a copy of the GNU Affero General Public
- * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-/*
- *
- * An example of config.php
- *
- * <?php
- * $CONFIG = array(
- *     "database" => "mysql",
- *     "firstrun" => false,
- *     "pi" => 3.14
- * );
- * ?>
- *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
  */
 
 /**
@@ -42,11 +19,13 @@
  */
 class OC_Config {
 
-	/**
-	 * @var \OC\Config
-	 */
+	/** @var \OC\Config */
 	public static $object;
 
+	/**
+	 * Returns the config instance
+	 * @return \OC\Config
+	 */
 	public static function getObject() {
 		return self::$object;
 	}
@@ -92,7 +71,6 @@ class OC_Config {
 	 * @param string $key key
 	 *
 	 * This function removes a key from the config.php.
-	 *
 	 */
 	public static function deleteKey($key) {
 		self::$object->deleteKey($key);
diff --git a/tests/lib/config.php b/tests/lib/config.php
index f739df3ce97..180f6b1649b 100644
--- a/tests/lib/config.php
+++ b/tests/lib/config.php
@@ -7,82 +7,134 @@
  */
 
 class Test_Config extends PHPUnit_Framework_TestCase {
-	const CONFIG_FILE = 'static://config.php';
-	const CONFIG_DIR = 'static://';
-	const TESTCONTENT = '<?php $CONFIG=array("foo"=>"bar");';
+	const TESTCONTENT = '<?php $CONFIG=array("foo"=>"bar", "beers" => array("Appenzeller", "Guinness", "Kölsch"), "alcohol_free" => false);';
 
-	/**
-	 * @var \OC\Config
-	 */
+	/** @var array */
+	private $initialConfig = array('foo' => 'bar', 'beers' => array('Appenzeller', 'Guinness', 'Kölsch'), 'alcohol_free' => false);
+	/** @var string */
+	private $configFile;
+	/** @var \OC\Config */
 	private $config;
+	/** @var string */
+	private $randomTmpDir;
 
 	function setUp() {
-		file_put_contents(self::CONFIG_FILE, self::TESTCONTENT);
-		$this->config = new OC\Config(self::CONFIG_DIR);
+		$this->randomTmpDir = \OC_Helper::tmpFolder();
+		$this->configFile = $this->randomTmpDir.'testconfig.php';
+		file_put_contents($this->configFile, self::TESTCONTENT);
+		$this->config = new OC\Config($this->randomTmpDir, 'testconfig.php');
 	}
 
-	public function testReadData() {
-		$config = new OC\Config('/non-existing');
-		$this->assertAttributeEquals(array(), 'cache', $config);
-
-		$this->assertAttributeEquals(array('foo' => 'bar'), 'cache', $this->config);
+	public function tearDown() {
+		unlink($this->configFile);
 	}
 
 	public function testGetKeys() {
-		$this->assertEquals(array('foo'), $this->config->getKeys());
+		$expectedConfig = array('foo', 'beers', 'alcohol_free');
+		$this->assertSame($expectedConfig, $this->config->getKeys());
 	}
 
 	public function testGetValue() {
-		$this->assertEquals('bar', $this->config->getValue('foo'));
-		$this->assertEquals(null, $this->config->getValue('bar'));
-		$this->assertEquals('moo', $this->config->getValue('bar', 'moo'));
+		$this->assertSame('bar', $this->config->getValue('foo'));
+		$this->assertSame(null, $this->config->getValue('bar'));
+		$this->assertSame('moo', $this->config->getValue('bar', 'moo'));
+		$this->assertSame(false, $this->config->getValue('alcohol_free', 'someBogusValue'));
+		$this->assertSame(array('Appenzeller', 'Guinness', 'Kölsch'), $this->config->getValue('beers', 'someBogusValue'));
+		$this->assertSame(array('Appenzeller', 'Guinness', 'Kölsch'), $this->config->getValue('beers'));
 	}
 
 	public function testSetValue() {
 		$this->config->setDebugMode(false);
 		$this->config->setValue('foo', 'moo');
-		$this->assertAttributeEquals(array('foo' => 'moo'), 'cache', $this->config);
-		$content = file_get_contents(self::CONFIG_FILE);
+		$expectedConfig = $this->initialConfig;
+		$expectedConfig['foo'] = 'moo';
+		$this->assertAttributeEquals($expectedConfig, 'cache', $this->config);
 
-		$expected = "<?php\n\$CONFIG = array (\n  'foo' => 'moo',\n);\n";
+		$content = file_get_contents($this->configFile);
+		$expected = "<?php\n\$CONFIG = array (\n  'foo' => 'moo',\n  'beers' => \n  array (\n    0 => 'Appenzeller',\n  " .
+			"  1 => 'Guinness',\n    2 => 'Kölsch',\n  ),\n  'alcohol_free' => false,\n);\n";
 		$this->assertEquals($expected, $content);
+
 		$this->config->setValue('bar', 'red');
-		$this->assertAttributeEquals(array('foo' => 'moo', 'bar' => 'red'), 'cache', $this->config);
-		$content = file_get_contents(self::CONFIG_FILE);
+		$this->config->setValue('apps', array('files', 'gallery'));
+		$expectedConfig['bar'] = 'red';
+		$expectedConfig['apps'] = array('files', 'gallery');
+		$this->assertAttributeEquals($expectedConfig, 'cache', $this->config);
 
-		$expected = "<?php\n\$CONFIG = array (\n  'foo' => 'moo',\n  'bar' => 'red',\n);\n";
+		$content = file_get_contents($this->configFile);
+
+		$expected = "<?php\n\$CONFIG = array (\n  'foo' => 'moo',\n  'beers' => \n  array (\n    0 => 'Appenzeller',\n  " .
+			"  1 => 'Guinness',\n    2 => 'Kölsch',\n  ),\n  'alcohol_free' => false,\n  'bar' => 'red',\n  'apps' => \n " .
+			" array (\n    0 => 'files',\n    1 => 'gallery',\n  ),\n);\n";
 		$this->assertEquals($expected, $content);
 	}
 
 	public function testDeleteKey() {
 		$this->config->setDebugMode(false);
 		$this->config->deleteKey('foo');
-		$this->assertAttributeEquals(array(), 'cache', $this->config);
-		$content = file_get_contents(self::CONFIG_FILE);
+		$expectedConfig = $this->initialConfig;
+		unset($expectedConfig['foo']);
+		$this->assertAttributeEquals($expectedConfig, 'cache', $this->config);
+		$content = file_get_contents($this->configFile);
 
-		$expected = "<?php\n\$CONFIG = array (\n);\n";
+		$expected = "<?php\n\$CONFIG = array (\n  'beers' => \n  array (\n    0 => 'Appenzeller',\n  " .
+			"  1 => 'Guinness',\n    2 => 'Kölsch',\n  ),\n  'alcohol_free' => false,\n);\n";
 		$this->assertEquals($expected, $content);
 	}
 
-	public function testSavingDebugMode() {
+	public function testSetDebugMode() {
 		$this->config->setDebugMode(true);
-		$this->config->deleteKey('foo'); // change something so we save to the config file
-		$this->assertAttributeEquals(array(), 'cache', $this->config);
+		$this->assertAttributeEquals($this->initialConfig, 'cache', $this->config);
 		$this->assertAttributeEquals(true, 'debugMode', $this->config);
-		$content = file_get_contents(self::CONFIG_FILE);
+		$content = file_get_contents($this->configFile);
+		$expected = "<?php\ndefine('DEBUG',true);\n\$CONFIG = array (\n  'foo' => 'bar',\n  'beers' => \n  array (\n    0 => 'Appenzeller',\n  " .
+			"  1 => 'Guinness',\n    2 => 'Kölsch',\n  ),\n  'alcohol_free' => false,\n);\n";
+		$this->assertEquals($expected, $content);
 
-		$expected = "<?php\ndefine('DEBUG',true);\n\$CONFIG = array (\n);\n";
+		$this->config->setDebugMode(false);
+		$this->assertAttributeEquals($this->initialConfig, 'cache', $this->config);
+		$this->assertAttributeEquals(false, 'debugMode', $this->config);
+		$content = file_get_contents($this->configFile);
+		$expected = "<?php\n\$CONFIG = array (\n  'foo' => 'bar',\n  'beers' => \n  array (\n    0 => 'Appenzeller',\n  " .
+			"  1 => 'Guinness',\n    2 => 'Kölsch',\n  ),\n  'alcohol_free' => false,\n);\n";
 		$this->assertEquals($expected, $content);
 	}
 
-	/**
-	 * @expectedException \OC\HintException
-	 */
-	public function testWriteData() {
-		if (\OC_Util::runningOnWindows()) {
-			throw new \OC\HintException('this is ireelevent for windows');
-		}
-		$config = new OC\Config('/non-writable');
-		$config->setValue('foo', 'bar');
+	public function testIsDebugMode() {
+		// Default
+		$this->assertFalse($this->config->isDebugMode());
+
+		// Manually set to false
+		$this->config->setDebugMode(false);
+		$this->assertFalse($this->config->isDebugMode());
+
+		// Manually set to true
+		$this->config->setDebugMode(true);
+		$this->assertTrue($this->config->isDebugMode());
+	}
+
+	public function testConfigMerge() {
+		// Create additional config
+		$additionalConfig = '<?php $CONFIG=array("php53"=>"totallyOutdated");';
+		$additionalConfigPath = $this->randomTmpDir.'additionalConfig.testconfig.php';
+		file_put_contents($additionalConfigPath, $additionalConfig);
+
+		// Reinstantiate the config to force a read-in of the additional configs
+		$this->config = new \OC\Config($this->randomTmpDir, 'testconfig.php');
+
+		// Ensure that the config value can be read and the config has not been modified
+		$this->assertSame('totallyOutdated', $this->config->getValue('php53', 'bogusValue'));
+		$this->assertEquals(self::TESTCONTENT, file_get_contents($this->configFile));
+
+		// Write a new value to the config
+		$this->config->setValue('CoolWebsites', array('demo.owncloud.org', 'owncloud.org', 'owncloud.com'));
+		$expected = "<?php\n\$CONFIG = array (\n  'foo' => 'bar',\n  'beers' => \n  array (\n    0 => 'Appenzeller',\n  " .
+			"  1 => 'Guinness',\n    2 => 'Kölsch',\n  ),\n  'alcohol_free' => false,\n  'php53' => 'totallyOutdated',\n  'CoolWebsites' => \n  array (\n  " .
+			"  0 => 'demo.owncloud.org',\n    1 => 'owncloud.org',\n    2 => 'owncloud.com',\n  ),\n);\n";
+		$this->assertEquals($expected, file_get_contents($this->configFile));
+
+		// Cleanup
+		unlink($additionalConfigPath);
 	}
+
 }