[Pkg-owncloud-commits] [owncloud] 184/258: Add flock to config
David Prévot
taffit at moszumanska.debian.org
Sat Oct 11 17:22:34 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository owncloud.
commit bc5c608e4e290b4766c1a722d4c9b12cafc019ce
Author: Lukas Reschke <lukas at owncloud.com>
Date: Thu Sep 25 18:43:04 2014 +0200
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
---
lib/private/config.php | 134 +++++++++++++++++++++++++-----------------
lib/private/legacy/config.php | 38 +++---------
tests/lib/config.php | 130 ++++++++++++++++++++++++++++------------
3 files changed, 182 insertions(+), 120 deletions(-)
diff --git a/lib/private/config.php b/lib/private/config.php
index 82a1c46..7bf3863 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 at 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 899c195..2d34888 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 at 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.
*/
/**
@@ -40,11 +17,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;
}
@@ -90,7 +69,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 c67a66c..7d6c7ea 100644
--- a/tests/lib/config.php
+++ b/tests/lib/config.php
@@ -7,79 +7,135 @@
*/
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);
+
+ $content = file_get_contents($this->configFile);
- $expected = "<?php\n\$CONFIG = array (\n 'foo' => 'moo',\n 'bar' => 'red',\n);\n";
+ $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() {
- $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);
}
+
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list