[SOLVED] Problems looping through variables

jej1216 · December 14, 2007

I have an HTML form where a folder on the server is selected and submitted to a PHP page which displays the files in the folder selected. Those two pieces work. The php page then passes a checkbox and a file name that is to be deleted to a second php page. It is this second PHP page that I am having trouble with.

The code:

<?php
//File Delete - use Unlink
$file = $_POST['filename'];
$checked = $_POST['checkthis'];
$myFile = $file;
$fh = fopen($myFile, 'w') or die("can't open file");
$myFile = $file;

// I know I need a while or foreach loop here ......................

unlink($myFile);
echo "Deleted: ".$myFile. " because the checkbox said ".$checked;
fclose($fh);
?>

I have tried a while loop and a foreach loop, but I get PHP errors. Leaving out the while or foreach code deletes the last file listed in the folder regardless of whether it had the checkbox checked or not. So all that proves is that the filename variable is being passed but it only acts upon the last file in the list.

I also tried this:

$myFile = $file;
$fh = fopen($myFile, 'w') or die("can't open file");

if(isset($_POST[$checked']))
  {
       echo "Deleted: ".$MyFile."<br/>";
       unlink($MyFile);
  }
fclose($fh);

but that does not work either.

I'm new to PHP and new to arrays, so any help would be appreciated. I don't want anyone to write code for me, just to give me direction of where to look for the code problems.

TIA,

jej1216

lemmin · December 14, 2007

It probably doesn't have access to delete it because you are opening it right before you delete it. Try removing the fopen and fclose statements and just delete the file. Also, in your second part, variables are case-sensitive so you need to use $myFile, not $MyFile.

This is all you should need:

if(isset($_POST['checkthis']))
{
   unlink($_POST['filename']);
   echo "Deleted: ".$_POST['filename']."<br>";
}

If it still doesn't work, make sure the path is correct. Print out $_POST['filename'] and see if it corresponds to the correct location of the file.

roopurt18 · December 14, 2007

I also notice that you say you intend to delete multiple items at a time, but you are using your variables as if they contain a single value.

$file = $_POST['filename'];
$checked = $_POST['checkthis'];

What does the HTML for your form look like that submits to that page?

jej1216 · December 17, 2007

Here is the HTML form code:

<html>
<head><h3>Forms Delete - Folder Selection</h3></head>
<HR>
<body>
<form name="dellist" enctype="multipart/form-data" action="ehi_delete_list.php" method="POST">
<select name="folder">
<option value="">Choose:</option>
<option value="LSH">LSH</option>
<option value="LTACH">LTACH</option>
<option value="MVRRH">MVRRH</option>
<option value="NCLTCH">NCLTCH</option>
<option value="NCRH">NCRH</option>
<option value="NIACH">NIACH</option>
<option value="OUTP">OUTP</option>
<option value="REHAB">REHAB</option>
<option value="SNF">SNF</option>
<option value="STRH">STRH</option>
<option value="SUBSPEC">SUBSPEC</option>
<option value="UVSH">UVSH</option>
</select>
<br>
<p><input type="submit" value="Form List for This Folder" /></p>
</form>

I also commented out the fopen and fclose code as well as making my case on $myFile match (oops). Still no luck. The echo shows the correct folder, but lists only the last file in the folder even though it is not checked in the previous php page.

It seems as the $checked variable is not being considered - it doesn't seem to matter if it's checked or not. Also, it's only getting one value for $myFile.

Thanks,

roopurt18 · December 17, 2007

What about the HTML for the form where the user selects the files to delete?

Also, on the form you posted, you can leave off the enctype attribute if you want, since you're not uploading any files with it.

jej1216 · December 17, 2007

Here's the code for the first php page, which is the form that passes the checked and file variables:

<html>
<head><h3>Select form(s) to delete:</h3></head>
<HR>
<body>
<form name="delchecked" enctype="multipart/form-data" action="ehi_delete_these.php" method="POST">

<?php
$dir = '/usr/local/php_classes/forms/FORMS/' . $_POST['folder'] . '/';
echo "Folder is: ".$_POST['folder'];
echo "<br/>";
function shortimg($str){
  list($short)=explode(" ",$str,2);
  return $short;
}

if (is_dir($dir)) {
  if ($dh = opendir($dir)) {
    while (($file = readdir($dh)) !== false) {
      if($file !== "." && $file !== ".." )
        //Get the short version of the file name
        //and store them in 2 arrays
        $short=shortimg($file);
        $aryRef[]=$short;
        $files[$short]=$file;
      }
      //Force PHP to order them using a numeric algorithm
      sort($aryRef,SORT_NUMERIC);
      //Parse the reference array
      foreach ($aryRef as $key) {
        //Here, $key is the same as what was $short previously
        $file=$files[$key];
        //Do not need to escape variables in strings if the strings delimiter are "
      if($file !== "." && $file !== ".." ) {
       echo "<input type='checkbox' name='checkthis'/>";
       echo "<input type='text' size=100 name='filename' value='".$dir.$file."'/><br>";
      }
      }
      closedir($dh);
  }
}
?>

<p><input type="submit" value="Delete Checked Files" /></p>
</form>
<HR>
</body>

</html>

Here is the code from the php page that needs to take the values from the above page and delete the selected files:

<html>
<head><h3>The files listed below has been deleted:</h3></head>
<HR>
<body>

<?php
$file = $_POST['filename'];

if (!isset($_POST['checkthis'])) {
$checked = "";
}else{
$checked = $_POST['checkthis'];
}

echo "Checked value = ".$checked." ";
echo "File selected = ".$file." ";

$myFile = $file;
$fh = fopen($myFile, 'w') or die("can't open file");

// I know I need a while or foreach loop here ......................
if(isset($_POST['checkthis']))
  {
    unlink($myFile);
    echo "Deleted: ".$myFile. " because the checkbox said ".$checked;
  }

fclose($fh);

?>

<HR>
</body>
</html>

I realized a flaw in part of the 2nd php page code

if (!isset($_POST['checkthis'])) {
$checked = "";
}else{
$checked = $_POST['checkthis'];
}

Originally I had left out the 'else {' piece. Now, echoing the $checked value shows the blank value if a checkbox is unchecked and a value of 'on' when it is checked.

I also found this: If I leave no checkbox checked in the entire list of files in the first php page (code is above), nothing gets deleted but the $myFile echo always shows the last file in the list. If I check any checkbox on the first php page, however, The second php page shows that the checkbox is 'on' for the last file in the list, and deleted that file.

I think the code problem is here:

if(isset($_POST['checkthis']))
  {
    unlink($myFile);
    echo "Deleted: ".$myFile. " because the checkbox said ".$checked;
  }

Thanks,

roopurt18 · December 17, 2007

      while (($file = readdir($dh)) !== false) {
        if($file !== "." && $file !== ".." )
          //Get the short version of the file name
          //and store them in 2 arrays
          $short=shortimg($file);
          $aryRef[]=$short;
          $files[$short]=$file;
        }

Did you intentionally leave off the curly brackets on the if statement? That closing curly bracket goes with your while loop. You should always include curly brackets around the body of your if statements, even if they're optional; trust me on that one.

I've only glanced at your code, but if I understand it correctly you are grabbing all the files in the directory and generating:

  <input type='checkbox' name='checkthis' />
  <input type='text' name='filename' value='the_path_and_name' /><br>

The HTML this form generates will be similar to:

  <input type='checkbox' name='checkthis' />
  <input type='text' name='filename' value='/the/path/file1' /><br>
  <input type='checkbox' name='checkthis' />
  <input type='text' name='filename' value='/the/path/file2' /><br>

There are two things very, very wrong with this:

You are giving away the file structure of your web host - BAD!
You have multiple inputs with the same name.

If you can convince me that you understand what I've said so far, I can assist you with a good solution. But you have to convince me.

jej1216 · December 17, 2007

OK, here's the deal. Prior to this, I have only had to create PHP forms that would post to a MySQL database. It took me awhile, but I have successfully done that. This task is quite frankly eating my lunch. I did not intend to leave out the curly bracket after my if statement - I have added it in. You are correct - the intent is to list files in a folder and have the user check the box next to each file to be deleted. Point taken on showing the file structure on the server - I should have cleaned up my copy/paste prior to posting.

As for multiple inputs with the same name, I understand what you are saying - I think. My form that passes the checkbox and filename variables is overwriting each line so that I end up with the last checked box and the last file in the list. My actual delete page then is processing this single value for $checked and the single value for $file. Right?

So the delete php page is doing what it needs to do - the code I need to work on is in the form page that passes the checkbox and file values, right?

roopurt18 · December 17, 2007

You seem to have a good idea of what's going on.

My actual delete page then is processing this single value for $checked and the single value for $file. Right?

Exactly.

I should have cleaned up my copy/paste prior to posting.

Tsk. Tsk. It's a good thing you left it there otherwise you may have let it go through your final code. We all do goofy things from time to time while coding; it is always best to paste your code directly as it is so that people may point out things you're not even asking about.

Anyways, to help with the problem at hand, let's start with what we know. Also, since web applications are open to the public (even if password protected), we always want to give as little as possible about our application away to the user; always design your forms so that they send along just enough information to accomplish their task.

Let's start with the process:

User selects directory -> User selects files -> delete files

Which translates to (substitute your own file names)

list_dirs.php -> list_files.php -> delete_files.php

This is exactly what you have (except the script names), so far so good.

The root directories that you're listing to the user lives in:

/usr/local/php_classes/forms/FORMS/

There is no reason for the end-user to ever know this directory exists.

list_dirs.php exists and is working, so we skip that.

Looking at list_files.php, our form is incorrect; all of the inputs have the same names, also we are using some incorrect input types. Think about what the user needs to do on this page: check off files from a listing. Thus our form should look like:

Files:
  - [ ] file1
  - [ ] file2
  - [*] file3
  - [ ] file4
  - [*] file5

The brackets indicate a check box and the asterisks indicate that it is selected. There is no reason to place the file names in inputs with type="textbox"; indeed we do not want to invite the user to change file names (although a more experienced user can do so no matter what you do).

This would indicate the HTML source we should be generating for our form should maybe look like:

<p>
  Files:
  <ul>
    <li>
      <input type="checkbox" name="checkthis" />
      file1
    </li>
    <li>
      <input type="checkbox" name="checkthis" />
      file2
    </li>
    ...
  </ul>
</p>

We have two problems now:

We still have duplicate name attributes for our input fields.
How will we get the file names?

We can fix both problems with a neat trick: You can always coerce $_POST to be in a format that is desirable for your application. By appending square brackets to the name attribute of the inputs, you will turn $_POST['checkthis'] into an array. Not only can we turn $_POST['checkthis'] into an array, but we can define the keys! For our purposes, setting the keys equal to the file names will be most convenient. This changes our form to:

<p>
  Files:
  <ul>
    <li>
      <input type="checkbox" name="checkthis[file1]" />
      file1
    </li>
    <li>
      <input type="checkbox" name="checkthis[file2]" />
      file2
    </li>
    ...
  </ul>
</p>

Before we get back into the code, our final page that deletes the files will need to know which directory was originally selected in the drop down, so we will add that to the form as a hidden parameter. Thus our entire form should look like:

<form action="delete_files.php" method="post">
  <input type="hidden" name="directory" value="the_selected_path" />
<p>
  Files:
  <ul>
    <li>
      <input type="checkbox" name="checkthis[file1]" />
      file1
    </li>
    <li>
      <input type="checkbox" name="checkthis[file2]" />
      file2
    </li>
    ...
  </ul>
</p>
</form>

Here is pseudo code to generate that form:

list_files.php

<?php
  $base_dir = '/usr/local/php_classes/forms/FORMS/';
  $directory = $_POST['folder'];
  if(!is_dir($base_dir . $directory)){
    // User has selected a directory that DOES NOT EXIST - aka they're
    // cheating.
  }
?>
<form action="delete_files.php" method="post">
  <input type="hidden" name="directory" value="<?php echo $directory; ?>" />
  <p>
    Directory: <?php echo $directory; ?>
  </p>
<p>
  Files:
    <ul>
<?php
  // PHP PSEUDO CODE
  FOREACH $file IN $base_dir . $directory
    SKIP . AND ..
    echo "
      <li>
        <input type=\"checkbox\" name=\"checkthis[{$file}]\" value=\"1\" />
        {$file}
      </li>
    ";
  ENDFOREACH
?>
    </ul>
  </p>
  <p>
    <input type="submit" value="Delete Selected" />
  </p>
</form>

Moving along to delete_files.php, $_POST will have three keys: one for the submit button, one for the hidden field, and one for all of the checkboxes. The checkboxes key will be an array itself where each key is named after a file that was selected.

Try this as a simple delete_files.php:

<?php
  echo '<pre style="text-align: left;">' . print_r($_POST, true) . '</pre>';
?>

From there, it is trivial to write a loop that deletes the files that were selected:

delete_files.php

<?php
  $base_dir = '/usr/local/php_classes/forms/FORMS/';
  $directory = $_POST['directory'];

  foreach($_POST['checkthis'] as $file => $empty){
    if(file_exists($base_dir . $directory . '/' . $file)){
      unlink($base_dir . $directory . '/' . $file);
    }
  }
?>

There is one thing that I've noticed your scripts are missing (mine are as well), and that is proper checks to make sure the user isn't feeding you bad data. Remember that the user can try to be sneaky and traverse up your directory structure. So filter the user's incoming data, remove '.' and '..' strings from the paths they say they want to list, and any time you list or delete stuff make sure it's something the user originally had access to.

jej1216 · December 17, 2007

roopurt18,

Thanks!

This should give me what I need.

Sign In

[SOLVED] Problems looping through variables

Recommended Posts

jej1216

Link to comment

Share on other sites

lemmin

Link to comment

Share on other sites

roopurt18

Link to comment

Share on other sites

jej1216

Link to comment

Share on other sites

roopurt18

Link to comment

Share on other sites

jej1216

Link to comment

Share on other sites

roopurt18

Link to comment

Share on other sites

jej1216

Link to comment

Share on other sites

roopurt18

Link to comment

Share on other sites

jej1216

Link to comment

Share on other sites

Join the conversation

Browse

Activity

Important Information