[RESOLVED] delete code problem

robkir

This code ought to work (how many times have you said that to yourself?). I'm going blind trying to find out what I've done wrong. The user clicks a check box and the module removes the entry from the database...a beautiful concept if only it worked!

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title></title>
</head><body>

<?php

session_start();
include('../DBconMOD.php');

if(isset($_POST['cmdDelete']))
{
		if(isset($_POST['team_roster']))
		{
			while(list($key, $val) = each($_POST['shirtNo']))
			{
			$sql = "DELETE FROM team_roster WHERE playerID=".$val;
			mysql_query($sql);
			}	
		}
}

?>
<p class="headline_red_reg"> DELETE ENTRY or ENTRIES</p>
<p class = "text_reg">Check the box(es) of the players you wish to drop from the roster. <font color="red"><strong> WARNING</strong></font> <font color="= black"> </font>
When you either hit a return or click on the Delete button at the bottom of this page, the entries with checkmarks will be deleted and cannot be recovered (you will
have to rekey them)<br />

<form id="form1" name="form1" method="post" action="roster_delete.php">
<table border="1">
  <tr>
          <td width="5%"><strong>Del?</strong>&nbsp;</td>
          <td width="5%"><strong>Shirt Number</strong></td>
          <td width="25%"><strong>Last Name</strong></td>
          <td width="25%"><strong>First Name</strong></td>


  </tr>

<?php 

$sql="SELECT playerID, shirtNo, playerL, playerF
					   FROM team_roster
					   ORDER BY shirtNo ASC
					  "; 
$row = mysql_query($sql);
while($result = mysql_fetch_array($row))

{
?>

  <tr>
      <td><input type="checkbox" name="deleteMe[]" value="<?=$result['playerID'];?>" /></td>
      <td><?=$result['shirtNo'];?></td>
      <td><?=$result['playerL'];?></td>
	  <td><?=$result['playerF'];?></td>

  </tr>

<?php 
}
?>		
    <tr>
    	<td><input type="submit" name="cmdDelete" id="cmdDelete" value="Delete" /></td>
    </tr>
 </table>
 </form>
</body></html>	

<?php
mysql_close($link);
?>

bradgrafelman

For one, I don't see any form element named "team_roster", so this if() statement will never be true:

if(isset($_POST['team_roster']))

Second, you're trying to loop over a POST'ed element named "shirtNo", but once again there is no such form element named "shirtNo" (perhaps you meant "deleteMe" instead?).

Finally, your code is vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query. Instead, it must first be sanitized with a function such as [man]mysql_real_escape_string/man (or switch to a more updated library such as [man]MySQLi[/man] or [man]PDO[/man] and use prepared statements). Note in your case that since you're expecting $val to be an integer, you could instead cast the data to an int (or use [man]intval/man) to sanitize the data.

halojoy

There is no form input field team_roster
and what is to be deleted is stored in array $_POST['deleteMe']

Maybe there is some other issue
but begin with try this

<?php

session_start();
include('../DBconMOD.php');

if(isset($_POST['cmdDelete']))
{
    while(list($key, $val) = each($_POST['deleteMe']))
    {
        $sql = "DELETE FROM team_roster WHERE playerID=".$val;
        mysql_query($sql);
    }    

}

?>

robkir

bradgrafelman;10939823 wrote:
Finally, your code is vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query. Instead, it must first be sanitized with a function such as [man]mysql_real_escape_string/man (or switch to a more updated library such as [man]MySQLi[/man] or [man]PDO[/man] and use prepared statements). Note in your case that since you're expecting $val to be an integer, you could instead cast the data to an int (or use [man]intval/man) to sanitize the data.

I cast $val, but how would somebody manage an SQL injection attack on a checkbox?

robkir

halojoy;10939824 wrote:
There is no form input field team_roster
and what is to be deleted is stored in array $_POST['deleteMe']

That was it. Now that you clearly stated it, the answer was obvious (well, maybe not completely obvious to those of us still climbing up the mountain of understanding. It can get pretty confusing at times).

Thanks for credited code.

bradgrafelman

robkir;10939834 wrote:
I cast $val, but how would somebody manage an SQL injection attack on a checkbox?

Well if they're trying to do a SQL injection, they sure aren't going to use your form as-is. It's easy enough to POST whatever data you want to a web script, and there is even an addon for FireFox that allows you to modify the HTML of a page and see it re-rendered in real time.

robkir

bradgrafelman;10939841 wrote:
Well if they're trying to do a SQL injection, they sure aren't going to use your form as-is. It's easy enough to POST whatever data you want to a web script, and there is even an addon for FireFox that allows you to modify the HTML of a page and see it re-rendered in real time.

Thanks for the advice. There are so many innovative ways to stub one's toe (or should I say "toes"?).

robkir