I’m looking at index.php in the install dir, specifically at the section of lines starting at line #433 which looks like this:
// config.inc.php writable?
echo "<p>Checking if <span class='mono'>manager/includes/config.inc.php</span> is writable: ";
$isWriteable = is_writable("../manager/includes/config.inc.php");
if(!file_exists("../manager/includes/config.inc.php")) {
// make an attempt to create the file
@$hnd=fopen("../manager/includes/config.inc.php", 'w');
@fwrite($hnd,"<?php //MODx configuration file ?>");
@fclose($hnd);
$isWriteable = file_exists("../manager/includes/config.inc.php");
@unlink("../manager/includes/config.inc.php");
}
if(!$isWriteable) {
echo "<span class='notok'>Failed!</span></p><p><strong>For new Linux/Unix installs, please create a blank file named <span class='mono'>config.inc.php</span> in the <span class='mono'>manager/includes/</span> directory with file permissions set to 777.</strong></p>";
$errors += 1;
} else {
echo "<span class='ok'>OK!</span></p>";
}
That logic seems backwards to me. Shouldn’t it look something like this?:
// config.inc.php writable?
echo "<p>Checking if <span class='mono'>manager/includes/config.inc.php</span> is writable: ";
if(!file_exists("../manager/includes/config.inc.php")) {
// make an attempt to create the file
@$hnd=fopen("../manager/includes/config.inc.php", 'w');
@fwrite($hnd,"<?php //MODx configuration file ?>");
@fclose($hnd);
}
$isWriteable = is_writable("../manager/includes/config.inc.php");
if(!$isWriteable) {
echo "<span class='notok'>Failed!</span></p><p><strong>For new Linux/Unix installs, please create a blank file named <span class='mono'>config.inc.php</span> in the <span class='mono'>manager/includes/</span> directory with file permissions set to 777.</strong></p>";
$errors += 1;
} else {
echo "<span class='ok'>OK!</span></p>";
}
In other words, we check if the file is there, and if not we create it and leave it alone (why are we unlinking it to start with?) and only then check for permissions and such. BTW, I also think there should be better error handling in there to deal with permission problems and such but that’s a different issue.
Anyway, am I missing something in why the current code is structured the way it is?
MODX Staff
