Skip to content

Commit 1013c3b

Browse files
authored
Merge pull request #34009 from andrey-utkin/safe-config-update
Make config file saving safe against write failures
2 parents c0c31bd + 807c53b commit 1013c3b

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();
@@ -272,30 +265,41 @@ private function writeData() {
272265
$content .= var_export($this->cache, true);
273266
$content .= ";\n";
274267

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

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

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

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

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

0 commit comments

Comments
 (0)