tags:

views:

375

answers:

3

I'm writing a PHP app that has a 'control panel' that writes a prefs file with certain variables. On every POST, if the file doesn't exist, it is created. If it does exist, it is unlinked and a new file is touched with the same filename and new variables. This file is then included on another page with displays content based on the variables inside it.

$file = "phpsettings.php";

if (!file_exists($file)) {
   touch($file);
  $handle = fopen ($file, 'r+'); 
$str = "<?php \$pref1 = \"$mypref\"; ?>";

} else {

unlink($file);
   touch($file);
   $handle = fopen ($file, 'r+'); 
   $str = "<?php \$pref1 = \"$mypref\"; ?>";

}

fwrite ($handle, $str); 
fclose ($handle);

Is this a safe way of writing preferences, provided this file will be overwritten many times per day? What is a good way of both alerting the user of this control panel if the file wasn't saved correctly, and in that case, what would be a good contingency plan to avoid breaking the page this prefs file is included on short of defining a default set of variables to fill if !(file_exists)?

A: 

Why not just use the truncation capabilities of fopen()? I believe instead of "r+", you'll need to pass "w+"... Then if the file exists, it will be truncated, if it doesn't you'll just create a new file. So the code becomes:

$file = "phpsettings.php";
$handle = fopen( $file, 'w+' );
$str = "<?php \$pref1 = \"$mypref\"; ?>";
fwrite ($handle, $str); 
fclose ($handle);
dicroce
A: 

It is always better to store your users state in a session and only persist that state when needed.

Peter D
i should have clarified -- this isn't prefs for individual users, it is prefs for how the page will look to anyone that comes to that page. they are "editors' preferences."
al
So each user can set a different look for the page?
Peter D
No, one authorized user sets a look for the page out of a set of predefined options. The world sees the page as that one user defines.
al
+2  A: 

If you store your settings in an array, you can serialize() them and write to a text file, rather than writing raw php to a php file and including it.

If you're not sanitising your input for those preferences, and say $mypref1 represents someone's name, there's nothing stopping them from filling this out in the form field:

\"; echo \"PWNED

and your resulting PHP will become

<?php \$pref1 = \"$mypref\"; echo \"PWNED\"; ?>

So firstly, storing your preferences in an array and using serialize() is much safer:

$prefs = array('mypref1' => 'somethingorother');
$handle = fopen ($file, 'w'); 
fwrite($handle, serialize($prefs));
fclose($h);

// example code demonstrating unserialization
$prefs2 = unserialize(file_get_contents($file));
var_dump($prefs == $prefs2); // should output "(bool) true"

In your question, you also mention that if the file does exist, it is unlinked. You can simply truncate it to zero length by passing "w" as the second argument to fopen - you don't need to manually delete it. This should set the mtime anyway, negating the need for the call to touch().

If the values being written to the file are preferences, surely each preference could have a default, unless there are hundreds? array_merge will allow you to overwrite on a per-key basis, so if you do something like this:

// array of defaults
$prefs = array(
    'mypref1' => 'pants',
    'mypref2' => 'socks',
);
if (file_exists($file)) {
    // if this fails, an E_NOTICE is raised. are you checking your server error
    // logs regularly?
    if ($userprefs = unserialize(file_get_contents($file))) {
        $prefs = array_merge($prefs, $userprefs);
    }
}

If the issue is that there are heaps, and you don't want to have to initialise them all, you could have a get_preference method which just wraps an isset call to the prefs array.

function get_preference($name, &$prefs) {
    if (isset($pref[$name]))
        return $pref[$name];
    return null;
}
var_dump(get_preference('mypref1', $prefs));

Beyond all of the questions this raises though, the reality is that with your app, in the unlikely event that something does go wrong with the fopen, it should be regarded as a serious failure anyway, and the handful of users you're likely to have making use of this feature are going to be contacting you pretty darn quick if something goes wrong.

Shabbyrobe
Is this solution still necessary if (a) the options are radio buttons, not text fields-- yes they're still manipulatable, but (b) I trust the small group of people that will be using this form?
al
Not exactly, but it can be a good habit forming exercise if nothing else, and a good bit of practise at using techniques to mitigate injection issues.
Shabbyrobe