Improve directory validation in admin checks

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
mantisbt · Sep 12, 2011 · aa3244a · aa3244a
1 parent a903240
commit aa3244a
Showing 1 changed file with 26 additions and 3 deletions.
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."
+		)
 	);
 }