Merge pull request #10934 from owncloud/datadir-write-setup

Don't complain about non-writable datadirs before we're installed

2 files changed, 40 insertions, 26 deletions

diff --git a/lib/private/setup.php b/lib/private/setup.php
index ccf996460d0..b1b3388f81b 100644
--- a/lib/private/setup.php
+++ b/lib/private/setup.php
@@ -37,18 +37,28 @@ class OC_Setup {
 			$dbtype = 'sqlite';
 		}
 
+		$username = htmlspecialchars_decode($options['adminlogin']);
+		$password = htmlspecialchars_decode($options['adminpass']);
+		$datadir = htmlspecialchars_decode($options['directory']);
+
 		$class = self::$dbSetupClasses[$dbtype];
+		/** @var \OC\Setup\AbstractDatabase $dbSetup */
 		$dbSetup = new $class(self::getTrans(), 'db_structure.xml');
 		$error = array_merge($error, $dbSetup->validate($options));
 
+		// validate the data directory
+		if (
+			(!is_dir($datadir) and !mkdir($datadir)) or
+			!is_writable($datadir)
+		) {
+			$error[] = $l->t("Can't create or write into the data directory %s", array($datadir));
+		}
+
 		if(count($error) != 0) {
 			return $error;
 		}
 
 		//no errors, good
-		$username = htmlspecialchars_decode($options['adminlogin']);
-		$password = htmlspecialchars_decode($options['adminpass']);
-		$datadir = htmlspecialchars_decode($options['directory']);
 		if(    isset($options['trusted_domains'])
 		    && is_array($options['trusted_domains'])) {
 			$trustedDomains = $options['trusted_domains'];
diff --git a/lib/private/util.php b/lib/private/util.php
index e5137de0fad..46a61716333 100755
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -409,14 +409,15 @@ class OC_Util {
 	/**
 	 * check if the current server configuration is suitable for ownCloud
 	 *
+	 * @param \OCP\IConfig $config
 	 * @return array arrays with error messages and hints
 	 */
-	public static function checkServer() {
+	public static function checkServer($config) {
 		$l = \OC::$server->getL10N('lib');
 		$errors = array();
-		$CONFIG_DATADIRECTORY = OC_Config::getValue('datadirectory', OC::$SERVERROOT . '/data');
+		$CONFIG_DATADIRECTORY = $config->getSystemValue('datadirectory', OC::$SERVERROOT . '/data');
 
-		if (!self::needUpgrade() && OC_Config::getValue('installed', false)) {
+		if (!self::needUpgrade($config) && $config->getSystemValue('installed', false)) {
 			// this check needs to be done every time
 			$errors = self::checkDataDirectoryValidity($CONFIG_DATADIRECTORY);
 		}
@@ -456,7 +457,7 @@ class OC_Util {
 		}
 
 		// Check if there is a writable install folder.
-		if (OC_Config::getValue('appstoreenabled', true)) {
+		if ($config->getSystemValue('appstoreenabled', true)) {
 			if (OC_App::getInstallPath() === null
 				|| !is_writable(OC_App::getInstallPath())
 				|| !is_readable(OC_App::getInstallPath())
@@ -471,25 +472,27 @@ class OC_Util {
 			}
 		}
 		// Create root dir.
-		if (!is_dir($CONFIG_DATADIRECTORY)) {
-			$success = @mkdir($CONFIG_DATADIRECTORY);
-			if ($success) {
-				$errors = array_merge($errors, self::checkDataDirectoryPermissions($CONFIG_DATADIRECTORY));
-			} else {
+		if ($config->getSystemValue('installed', false)) {
+			if (!is_dir($CONFIG_DATADIRECTORY)) {
+				$success = @mkdir($CONFIG_DATADIRECTORY);
+				if ($success) {
+					$errors = array_merge($errors, self::checkDataDirectoryPermissions($CONFIG_DATADIRECTORY));
+				} else {
+					$errors[] = array(
+						'error' => $l->t('Cannot create "data" directory (%s)', array($CONFIG_DATADIRECTORY)),
+						'hint' => $l->t('This can usually be fixed by '
+							. '<a href="%s" target="_blank">giving the webserver write access to the root directory</a>.',
+							array(OC_Helper::linkToDocs('admin-dir_permissions')))
+					);
+				}
+			} else if (!is_writable($CONFIG_DATADIRECTORY) or !is_readable($CONFIG_DATADIRECTORY)) {
 				$errors[] = array(
-					'error' => $l->t('Cannot create "data" directory (%s)', array($CONFIG_DATADIRECTORY)),
-					'hint' => $l->t('This can usually be fixed by '
-						. '<a href="%s" target="_blank">giving the webserver write access to the root directory</a>.',
-						array(OC_Helper::linkToDocs('admin-dir_permissions')))
+					'error' => 'Data directory (' . $CONFIG_DATADIRECTORY . ') not writable by ownCloud',
+					'hint' => $permissionsHint
 				);
+			} else {
+				$errors = array_merge($errors, self::checkDataDirectoryPermissions($CONFIG_DATADIRECTORY));
 			}
-		} else if (!is_writable($CONFIG_DATADIRECTORY) or !is_readable($CONFIG_DATADIRECTORY)) {
-			$errors[] = array(
-				'error' => 'Data directory (' . $CONFIG_DATADIRECTORY . ') not writable by ownCloud',
-				'hint' => $permissionsHint
-			);
-		} else {
-			$errors = array_merge($errors, self::checkDataDirectoryPermissions($CONFIG_DATADIRECTORY));
 		}
 
 		if (!OC_Util::isSetLocaleWorking()) {
@@ -1310,11 +1313,12 @@ class OC_Util {
 	 * either when the core version is higher or any app requires
 	 * an upgrade.
 	 *
+	 * @param \OCP\IConfig $config
 	 * @return bool whether the core or any app needs an upgrade
 	 */
-	public static function needUpgrade() {
-		if (OC_Config::getValue('installed', false)) {
-			$installedVersion = OC_Config::getValue('version', '0.0.0');
+	public static function needUpgrade($config) {
+		if ($config->getSystemValue('installed', false)) {
+			$installedVersion = $config->getSystemValue('version', '0.0.0');
 			$currentVersion = implode('.', OC_Util::getVersion());
 			if (version_compare($currentVersion, $installedVersion, '>')) {
 				return true;