Improve directory validation in admin checks

dregad · dregad · commit aa3244ae82fc · 2011-09-12T13:40:12.000+02:00
This commit brings the following improvements in check_paths_inc.php:

 - In addition to checking that a directory is valid, we now also
   verify that it is readable, this way admin will know if the error
   is caused e.g. by a symlink, or due to access rights
 - Print actual path instead of a text description in install check.
   The config option name is already displayed as part of the first
   line of the check's output, so repeating the information does not
   add any additional value.
 - Escape data printed in messages (path) with htmlspecialchars() as
   recommended by dhx
 - Added comments to clearly identify purpose of each check block
diff --git a/admin/check/check_paths_inc.php b/admin/check/check_paths_inc.php
@@ -49,6 +49,7 @@
 	$t_path_config_names[] = 'absolute_path_default_upload_folder';
 }
 
+# Build paths for all configs
 $t_paths = array();
 foreach( $t_path_config_names as $t_path_config_name ) {
 	$t_new_path = array();
@@ -57,19 +58,39 @@
 	$t_paths[$t_path_config_name] = $t_new_path;
 }
 
+# Trailing directory separator
 foreach( $t_paths as $t_path_config_name => $t_path ) {
 	check_print_test_row(
 		$t_path_config_name . ' configuration option has a trailing directory separator',
 		substr( $t_path['config_value'], -1, 1 ) == DIRECTORY_SEPARATOR,
-		array( false => 'You must provide a trailing directory separator (' . DIRECTORY_SEPARATOR . ') to the end of the ' . $t_path_config_name . ' configuration value.' )
+		array( false =>
+			"You must provide a trailing directory separator (" . DIRECTORY_SEPARATOR .
+			") to the end of '" . htmlspecialchars( $t_path['config_value'] ) . "'."
+		) )
 	);
 }
 
+# Is a directory
 foreach( $t_paths as $t_path_config_name => $t_path ) {
 	check_print_test_row(
 		$t_path_config_name . ' configuration option points to a valid directory',
 		is_dir( $t_path['config_value'] ),
-		array( false => 'The path specified by the ' . $t_path_config_name . ' configuration option does not point to a valid and accessible directory.' )
+		array( false =>
+			"The path '" . htmlspecialchars( $t_path['config_value'] ) .
+			"' is not a valid directory."
+		)
+	);
+}
+
+# Is readable
+foreach( $t_paths as $t_path_config_name => $t_path ) {
+	check_print_test_row(
+		$t_path_config_name . ' configuration option points to an accessible directory',
+		is_readable( $t_path['config_value'] ),
+		array( false =>
+			"The path '" . htmlspecialchars( $t_path['config_value'] ) .
+			"' is not accessible."
+		)
 	);
 }
 
@@ -80,7 +101,9 @@
 	check_print_test_row(
 		$t_path_config_name . ' configuration option points to a writable directory',
 		is_writable( $t_path['config_value'] ),
-		array( false => "The path specified by the $t_path_config_name configuration option ('" . $t_path['config_value'] . "') must be writable." )
+		array( false =>
+			"The path '" . htmlspecialchars( $t_path['config_value'] ) . "' must be writable."
+		)
 	);
 }
 
