Config file is checked multiple times with different checks

[LukasReschke]

From owncloud-archive/documentation#731 (comment):

---

Oh fun, oh fun.

What happens here is that our awesome code-base has the "is config writable check" in at least three different places and the logic is all different - yay.

1. A check in lib/private/config.php whether the file config/config.php exists (

   core/lib/private/config.php

   Lines 172 to 178 in c36bac3

   	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>.');
   	}

   )
2. A check in lib/base.php whether the file config/config.php is writable or the read-only mode is set (

   core/lib/base.php

   Lines 198 to 216 in 891474b

   	// Check if config is writable
   	$configFileWritable = is_writable($configFilePath);
   	if (!$configFileWritable && !OC_Helper::isReadOnlyConfigEnabled()
   	|| !$configFileWritable && \OCP\Util::needUpgrade()) {
   	if (self::$CLI) {
   	echo $l->t('Cannot write into "config" directory!')."\n";
   	echo $l->t('This can usually be fixed by giving the webserver write access to the config directory')."\n";
   	echo "\n";
   	echo $l->t('See %s', array(\OC_Helper::linkToDocs('admin-dir_permissions')))."\n";
   	exit;
   	} else {
   	OC_Template::printErrorPage(
   	$l->t('Cannot write into "config" directory!'),
   	$l->t('This can usually be fixed by '
   	. '%sgiving the webserver write access to the config directory%s.',
   	array('<a href="'.\OC_Helper::linkToDocs('admin-dir_permissions').'" target="_blank">', '</a>'))
   	);
   	}
   	}

   )
3. A check in lib/private/util.php which checks whether the config directory is writable. The even more funny thing here is that this code path is only called for new sessions, thus all existing sessions will work happily but as soon as you start a new one things will explode.(

   core/lib/private/util.php

   Lines 509 to 516 in aec79b0

   	if (!is_writable(OC::$configDir) or !is_readable(OC::$configDir)) {
   	$errors[] = array(
   	'error' => $l->t('Cannot write into "config" directory'),
   	'hint' => $l->t('This can usually be fixed by '
   	. '%sgiving the webserver write access to the config directory%s.',
   	array('<a href="' . \OC_Helper::linkToDocs('admin-dir_permissions') . '" target="_blank">', '</a>'))
   	);
   	}

   )

For the sake of having an usable documentation I'd say that we adjust the documentation for now and in the future unify these insane checks somehow in core - but that is unrelated… No need to have them at three different places and also no need to be different everywhere. Especially the third check is really some magic voodoo which abuses the session storage totally ;-)

---

Related to #11843

[MorrisJobke]

see also #13736

[PVince81]

Did we make any progress here, even small ?