Skip to content

Commit 807c53b

Browse files
author
Andriy Utkin
committed
Make config file saving safe against write failures
In case of disk space depletion, writing a new version of config fails, leaving the config file empty. This patch improves safety of updates to config.php by saving the new version of the config into a randomly-named temporary file in the same directory, and then renaming it to config.php, which renaming is atomic in POSIX-compliant filesystems and shouldn't involve disk space allocation. If writing the new config version is impossible, the current config remains unchanged. This patch drops the use of file locking as unnecessary. File locking merely established order in which the concurrent independent processes attempted file writing. In the end, the process which was last to update the file "wins" - their changes persist and the changes of previous writers are dropped as there's no conflict detection or change merging mechanism anyway. With the current change, there is still some resulting ordering, and the last writer still wins in the same way. Readers don't need file locking as well, as opening config.php for reading always provides a certain consistent version of the file. Signed-off-by: Andriy Utkin <dev@autkin.net>
1 parent ef97361 commit 807c53b

File tree

1 file changed

+29
-25
lines changed

1 file changed

+29
-25
lines changed

lib/private/Config.php

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,6 @@ private function readData() {
224224
continue;
225225
}
226226

227-
// Try to acquire a file lock
228-
if (!flock($filePointer, LOCK_SH)) {
229-
throw new \Exception(sprintf('Could not acquire a shared lock on the config file %s', $file));
230-
}
231-
232227
unset($CONFIG);
233228
include $file;
234229
if (!defined('PHPUNIT_RUN') && headers_sent()) {
@@ -244,7 +239,6 @@ private function readData() {
244239
}
245240

246241
// Close the file pointer and release the lock
247-
flock($filePointer, LOCK_UN);
248242
fclose($filePointer);
249243
}
250244

@@ -256,8 +250,7 @@ private function readData() {
256250
*
257251
* Saves the config to the config file.
258252
*
259-
* @throws HintException If the config file cannot be written to
260-
* @throws \Exception If no file lock can be acquired
253+
* @throws HintException If the config file cannot be written to, etc
261254
*/
262255
private function writeData() {
263256
$this->checkReadOnly();
@@ -268,30 +261,41 @@ private function writeData() {
268261
$content .= var_export($this->cache, true);
269262
$content .= ";\n";
270263

271-
touch($this->configFilePath);
272-
$filePointer = fopen($this->configFilePath, 'r+');
273-
274-
// Prevent others not to read the config
275-
chmod($this->configFilePath, 0640);
264+
// tmpfile must be in the same filesystem for the rename() to be atomic
265+
$tmpfile = tempnam(dirname($this->configFilePath), 'config.php.tmp.');
266+
// dirname check is for PHP's fallback quirk
267+
if (!$tmpfile || dirname($tmpfile) != dirname($this->configFilePath)) {
268+
if ($tmpfile) {
269+
unlink($tmpfile);
270+
}
271+
throw new HintException(
272+
"Can't create temporary file in config directory!",
273+
'This can usually be fixed by giving the webserver write access to the config directory.');
274+
}
276275

277-
// File does not exist, this can happen when doing a fresh install
276+
chmod($tmpfile, 0640);
277+
$filePointer = fopen($tmpfile, 'w');
278278
if (!is_resource($filePointer)) {
279279
throw new HintException(
280-
"Can't write into config directory!",
281-
'This can usually be fixed by giving the webserver write access to the config directory.');
280+
"Failed to open temporary file in config directory for writing",
281+
'Please report this to Nextcloud developers.');
282282
}
283283

284-
// Try to acquire a file lock
285-
if (!flock($filePointer, LOCK_EX)) {
286-
throw new \Exception(sprintf('Could not acquire an exclusive lock on the config file %s', $this->configFilePath));
284+
$write_ok = fwrite($filePointer, $content);
285+
$close_ok = fclose($filePointer);
286+
if (!$write_ok || !$close_ok) {
287+
unlink($tmpfile);
288+
throw new HintException(
289+
"Failed to save temporary file in config directory",
290+
'Please report this to Nextcloud developers.');
287291
}
288292

289-
// Write the config and release the lock
290-
ftruncate($filePointer, 0);
291-
fwrite($filePointer, $content);
292-
fflush($filePointer);
293-
flock($filePointer, LOCK_UN);
294-
fclose($filePointer);
293+
if (!rename($tmpfile, $this->configFilePath)) {
294+
unlink($tmpfile);
295+
throw new HintException(
296+
"Failed to replace the config file with the new copy",
297+
'Please report this to Nextcloud developers.');
298+
}
295299

296300
if (function_exists('opcache_invalidate')) {
297301
@opcache_invalidate($this->configFilePath, true);

0 commit comments

Comments
 (0)