Updating Selected Records

I have got myself a bit confused whilst trying to create an 'update selected records' page. In memberlist.php, a set of records is shown, each with a checkbox. 

When the user clicks on 'Update Selected' it opens up membermultiupdate.php. I cannot get this page to recognise which id's have been passed and put them in an array so only the checked records are shown.

The page in question is at http://www.margate-fc.com/memberlist.php. When it opens membermultiupdate.php, there are currently 2 errors....

Warning: implode() [function.implode]: Invalid arguments passed in /homepages/46/d98455693/htdocs/membermultiupdate.php on line 11

which refers to

$update = implode ("','", mysql_real_escape_string ($_POST['checkbox']))

Warning: mysql_fetch_array(): supplied argument is not a valid MySQL result resource in /homepages/46/d98455693/htdocs/membermultiupdate.php on line 44

which is

while ($rowmembers = mysql_fetch_array($members))

I have also tried mysql_fetch_assoc there too.

I may be miles out or I may be missing something simple but I have tried for hours now to get my head around this and have not even got to the point where I can see if the actual update side works!

Any suggestions would be greatly received and I have put the relevant code from both pages below.

Thanks in advance for any help or suggestions

Steve

memberlist.php

The query to get all rows

$members = mysql_query(" SELECT * FROM users");

The submit form with checkboxes

<form name="form1" method="post" action="membermultiupdate.php">
<?php
while ($rowmembers = mysql_fetch_assoc($members))
{echo '  <tr>
<td><input type="hidden" name="id[]" value="'.$rowmembers['id'].'" />'.$rowmembers['id'].'</td> 
<td><input name="checkbox[]" type="checkbox" id="checkbox[]" value="'.$rowmembers['id'].'"></td></td> 
<td>'.$rowmembers['id'].'</td>
<td>'.$rowmembers['username'].'</td>
<td>'.$rowmembers['first_name'].'</td>
<td>'.$rowmembers['last_name'].'</td>
<td>'.$rowmembers['email'].'</td>
';
if ($rowmembers['active'] == 1)
{echo '<td> YES </td>';}
else
{echo ' <td>NO </td>';}
echo '
<td><a href="memberedit.php?member='.$rowmembers['id'].'"><img src="http://icons.iconarchive.com/icons/custom-icon-design/office/256/edit-icon.png" width="15" height="15" /></a> <img src="http://icons.iconarchive.com/icons/custom-icon-design/office/256/delete-icon.png" width="15" height="15" /></td>
</tr>';}
    	
?>
<tr> 
<td colspan="4" align="center"><input type="submit" name="submit" value="Update Selected"></td> 
</tr>
</form>

membmultiupdate.php

The code bit to try to show all checked rows

if (isset ($_POST['submit']))
{
  if (is_array ($_POST['checkbox']))
  {
  $update = implode ("','", mysql_real_escape_string ($_POST['id']));

  $members = mysql_query("SELECT
  *
  FROM
  users
  WHERE
  id IN (".$update.")
  ");
  }
}

The form to show all of the checked rows

<form name="form1" method="post" action="">
<?php
while ($rowmembers = mysql_fetch_array($members))
{echo '
<tr>


  <td width="200px" align="right" style="font-weight:bold">
	<label for="name">Username:</label>
  </td>
  <td width="200px" >
	<input type="text" name="name" value="'.$rowmembers['username'].'" /><br>
  </td>
</tr>
<tr>
  <td width="200px" align="right" style="font-weight:bold">
	<label for="name">First Name:</label>
  </td>
  <td width="200px" >
	<input type="text" name="first_name" value="'.$rowmembers['first_name'].'" /><br>
  </td>
</tr>
<tr>
  <td width="200px" align="right" style="font-weight:bold">
	<label for="name">Last Name:</label>
  </td>
  <td width="200px" >
	<input type="text" name="last_name" value="'.$rowmembers['last_name'].'" /><br>
  </td>
</tr>
<tr>
  <td width="200px" align="right" style="font-weight:bold">
	<label for="name">Email:</label>
  </td>
  <td width="200px" >
	<input type="text" name="email" value="'.$rowmembers['email'].'" /><br>
  </td>
</tr>'
    	
;}
    	
?>
<tr> 
<td colspan="4" align="center"><input type="submit" name="update" value="update"></td> 
</tr> 
</form>

The code to update the rows from the above form and the return to memberlist.php

if($update)
{ 
foreach($_POST['id'] as $id)
{ 


	$sql1="
	UPDATE 
	users  
	SET 
	username='".$_POST["name".$id]."'
	, first_name='".$_POST["first_name".$id]."'
	, last_name='".$_POST["last_name".$id]."'
	, email='".$_POST["email".$id]."'
	, ONOFF='".$_POST["ONOFF".$id]."' 
	WHERE id='".$id."'"; 
	$result1=mysql_query($sql1); 
} 
} 

if($result1){ 
header("location:memberlist.php"); 
} 

You are passing an array $_POST['checkbox']

You cannot perform mysql_real_escape_string() on an array. This line of code first tries to run mysql_real-escape_string() on the array and then do implode.

$update = implode ("','", mysql_real_escape_string ($_POST['checkbox']))

I'm guessing the mysql_real_escape_string() is returning false which is why implode() is producing the error you displayed above - can't perform implode() on the boolean false.

But, seriously - you named your checkbox inputs 'checkbox'?! Name them something descriptive such as 'member_id' fior crying out loud.

Also, you cannot duplicate element ids in an html page, so you can't use 'checkbox[]' as the id for the fields. Instead, just append the id to the id

'checkbox_{$rowmembers['id']}'

Taking out the mysql_real_escape_string() worked perfectly thanks.

The update itself is not working but that is a problem to look at when I get home tonight.

Thanks again

Steve

BTW in the 'real' code the checkboxes are actually named 'membid'. I only changed them briefly as I though it would make my posted code easier to understand!

Taking out the mysql_real_escape_string() worked perfectly thanks.

Well, you still want to sanitize the values before using in a query. Since these are 'id' values I assume they are integers. So, you shouldn't have been using mysql_real_escape_string() to begin with. That function is for "string" data. For integers you could use something like intval(). If the value cannot be interpreted as an integer, intval() will return 0. You could either run your query with any 0's that are produced - since there would be no matching records they wouldn't do anything. Or, you could remove the 0 elements using array_filter.

Also, don't create your queries directly in the mysql_query() function. Instead, create them as a string variable so you can echo them to the page during the debugging process.

$updateIDs = implode(', ', array_filter(array_map('intval', $_POST['checkbox'])));


$query = "SELECT *
          FROM users
          WHERE id IN ({$updateIDs})";

$members = mysql_query($query);

Also, don't create your queries directly in the mysql_query() function.

That's a bad habit I got into initially and keep forgetting to get out of!

Thanks to the suggestion you posted the page is half working! The code below brings the right records into the form on the update page, but I am having trouble getting the update to work.

if (isset ($_POST['submit']))
{
  if (is_array ($_POST['membid']))
  {
$updateIDs = implode(', ', array_filter(array_map('intval', $_POST['membid'])));

$query = "SELECT * FROM users WHERE id IN ({$updateIDs})";

$members = mysql_query($query);
  }
}

I have looked through a lot of tutorials and examples, some using foreach loops and some using counters as well as the phpfreaks tutorial but have not got anything to work. The only time anything I tried did anything to the records was when it deleted all fields in all rows!

I will start on the quest again after work but not sure which method would be the best way to go?

Thanks

Steve

OK, looking at your update script I see you are giving the input fields all the same name! They need to have individual names and/or be array indexes. Here is what I would do:

Create all the input field as array names and use the record id as the index for those arrays. something like:

echo "<input type='text' name='name[{$rowmembers['id']}]' value='{$rowmembers['username']}' /><br>\n";

Then in your script to process the changes, just do a foreach() on one of the fields and us the id to reference all the fields to update that record

foreach($)POST['name'] as $id => $name)
{
    $firstName = $_POST['first_name'][$id];
    $lastName = $_POST['first_name'][$id];
    $email = $_POST['first_name'][$id];

    //sanitize and validate input then run update query
}

Huge thanks. This is now working absolutely perfectly and has also introduced me to foreach loops!

The final code is below, admittedly without sanitization as it is 1.30am and I am off to bed and it is still on my learning list!

However, I have made an attempt at validating data and sanitizing it simply with mysql_escape_string and stripslashes (although I may be missing the point there) on the registration form and have put that code at the bottom of this post. Admittedly it is not the tidiest code and is heavily commented so I know what is what as I go along but at this moment it works and my only real concern is if mysql_escape_string and stripslashes give enough safeguards.

Thanks again

Steve

The Update Script

	
    foreach($_POST['username'] as $id => $username)
{
    $firstName = $_POST['first_name'][$id];
    $lastName = $_POST['last_name'][$id];
    $email = $_POST['email'][$id];

    //run update query

$sql1="UPDATE users SET username='".$username."', first_name='".$firstName."', last_name='".$lastName."', email='".$email."' WHERE id='".$id."'";
$result1=mysql_query($sql1);
}

Registration script with validation

if(isset($_POST['sent']) && $_POST['sent'] =="yes" )
{//The form was submitted so check data
	{//START INPUT CHECKS
		#USERNAME
			if (empty($_POST['name']))//Check for Username
			  {//Username is empty
				  $regmsg .= 'Username must be entered<br />';
				  $namevalid = 1;
			  }
			else
			  {//Username is posted
				//Get the Username
				$name = mysql_escape_string($_POST['name']);
				//Is the Username alphanumeric and between 6 & 10 characters?
				if (!preg_match("/^[a-z0-9_]{6,10}+\z/i",$name))
				  {//Username is not alphanumeric
					  $regmsg .= 'Usernames must be between 6 & 10 characters and can only contain letters, numbers and _.<br />';
					  $namevalid = 1;
				  }
				else
				  {//Username is alphanumeric so check if it already exists
					$usersearch = mysql_query('SELECT username FROM users WHERE username="'.$name.'"') or die(mysql_error());  
					$usermatch  = mysql_num_rows($usersearch);
					//Specify error message if Username is already in use 
					if($usermatch > 0)
					{
						$regmsg .= 'Username already in use';
						$namevalid = 1;
					}
				  }
			  }
			  
		#EMAIL
			if (empty($_POST['email']))//Check for Email
			  {//Email is empty
				  $regmsg .= 'Email must be entered<br />';
				  $emailvalid = 1;
			  }
			else
			  {//Email is posted
				// Get the email address
				$email = mysql_escape_string($_POST['email']); 
				//Is the email address in a correct format?
				if(!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $email))
				  {// email is not an email address
					  $regmsg .= 'The email you have entered is invalid.<br />';  
					  $emailvalid = 1;
				  }
				 else
				  {//Email is a valid address so check if it already exists
				  $emailsearch = mysql_query('SELECT email FROM users WHERE email="'.$email.'"') or die(mysql_error());  
				  $emailmatch  = mysql_num_rows($emailsearch);
				  //Specify error message if Email is already in use 
				  if($emailmatch > 0)
				  {
					  $regmsg .= 'An account has already been registered with that email address.<br />';
					  $emailvalid = 1;
				  }
			  }
			  }
			  
		#FIRST NAME
			if (empty($_POST['first_name']))//Check for First Name
			  {//First Name is empty
				  $regmsg .= 'First Name must be entered<br />';
				  $fnamevalid = 1;
			  }
			else
			  {//First Name is posted
				//Get the First Name
				$fname = stripslashes($_POST['first_name']);
				//Does the First Name only contain letters, spaces, - or '?
				if (!preg_match("/^[a-z\\' -]+\z/i",$fname) || substr_count($fname, "'") > 1 || substr_count($fname, "-") > 1)
				  {//First Name is not valid
					  $regmsg .= 'First Names can only contain letters, spaces, \' and _.<br />';
					  $fnamevalid = 1;
				  }

			  }
			  
			  
			  
		#LAST NAME
			if (empty($_POST['last_name']))//Check for Last Name
			  {//Last Name is empty
				  $regmsg .= 'Last Name must be entered<br />';
				  $lnamevalid = 1;
			  }
			else
			  {//Last Name is posted
				//Get the Last Name
				$lname = stripslashes($_POST['last_name']);
				//Does the Last Name only contain letters, spaces, - or '?
				if (!preg_match("/^[a-z\\' -]+\z/i",$lname) || substr_count($lname, "'") > 1 || substr_count($lname, "-") > 1)
				  {//Last Name is not valid
					  $regmsg .= 'Last Names can only contain letters, spaces, \' and _.<br />';
					  $lnamevalid = 1;
				  }

			  }
			  
		#PASSWORDS
			if (empty($_POST['password']) OR empty($_POST['password2']))//Check for Passwords

			  {//At least one Password field is empty
				  $regmsg .= 'Both password fields must be entered<br />';
				  $passvalid = 1;
			  }
			else
			  {//Both Passwords are posted
				  $password = mysql_escape_string($_POST['password']); 
				  $password2 = mysql_escape_string($_POST['password2']); 
			  //Is the first Password alphanumeric and does the second one match?
				  if (!preg_match("/^[a-z0-9]{6,10}+\z/i",$password) OR !preg_match("/^[a-z0-9]{6,10}+\z/i",$password2))
				  {
					  $regmsg .= 'Passwords must be alphanumeric and between 6 & 10 characters long.<br />';
					  $passvalid = 1;
				  
					  }
				  if($password <> $password2)
				  {
					  $regmsg .= 'Your Passwords do not match.<br />';
				   	  $passvalid = 1;
				  
				   }	
			  }
		  
			  
		}//END INPUT CHECKS


	if (($passvalid !=1) AND ($namevalid !=1) AND ($fnamevalid !=1) AND ($lnamevalid !=1) AND ($emailvalid !=1) AND ($passvalid !=1)) 
	{//Everything is valid so insert user

			mysql_query
			(
			"INSERT 
			INTO users 
			 (username, first_name, last_name, password, email, hash, register_date, userlevel) 
			VALUES
			 ( 
			'". mysql_escape_string($name) ."', 
                                etc.............

... only real concern is if mysql_escape_string and stripslashes give enough safeguards.

mysql_real_escape_string() is sufficient.  stripslashes is unnecessary unless your host has magic_quotes_gpc enable and you need to undo that prior to processing the data.  If that is the case, you should have a little script to do that as part of your script startup and which is run on every page (usually put in a file and included on ever page).

You'll want to save your mysql_real_escape_string until your ready to put the value into SQL.  Do all your validations and other checks prior to escaping it, as escaping it can introduce characters or conditions that may cause an otherwise valid string to fail validation.

Optionally, if your validation process will ensure that a string will cause no problems (eg, if you validate a string contains only 'a'-'z' or only '0'-'9') you could skip the mysql_real_escape_string call, but continuing to do it will not hurt either.

I am not sure if magic_quotes_gpc is enabled and looking through the hosts FAQ (1&1 shared hosting) there is no indication one way or the other. I will look more into that when I get home.

If I have read your post correctly, there is no need to sanitize at the point of collecting the data from the form and it is only necessary at the point of inserting it into the database IF no secure validation has been done on the data?

Having said that and if I am correct, it would not be a bad idea to sanitize the inserted data anyway just to get into the habit of it?

Thanks

Steve

You can programatically disable magic quotes at runtime if needed. The manual provides an example script that will check if magic quotes are enabled and, if so, will run stripslashes(). Even if your current host doesn't have magic quotes enabled if they were to do so in the future or if you moved to a different host that does, your code will still work.

Also, regarding validating/sanitizing your variables, mysql_real_escape_string() will protect your queries from SQL Injection (and should be a last step). But, it is not all that you need if you want to prevent errors. As the name implies, mysql_real_escape_string() is for sanitizing string data. If you have a DB field that requires integer values and a non-numeric value is passed, mysql_real_escape_string() will not prevent the query from failing. It is sometimes a grey line as to where validation ends and sanitizing begins.

Here are some suggestions on the steps I typically take:

- Always use trim() on user submitted values (unless there is a valid reason not to) before doing any validations on length. Otherwise, a space entered into a required field would pass validation.

- Be careful about using any other functions that will modify the user input prior to validations [striptags(), htmlspecialcharacters(), etc.]. Only run those after initial validation is done unless you have a valid reason for doing otherwise. In fact, I almost never run those type of functions on input before storing in the database. Instead I run it at the time I pull the values from the database when I display them on the page. This makes the data device independent. So, if I was to output the data to an XML feed or something other than an HTML page, converting characters to their HTML entities would not make sense.

- For any data that should be a numeric value (int, float, etc) use an appropriate PHP function to convert the value to that type of numeric value. Then validate that the value is appropriate.

- Always do PHP validations before DB validations. For example, if a user submits a form for data to update an existing record, one fo the values will be the primary key for the record - so you may want to validate that the record exists before running the update. But first, you should validate that all of the update data is valid first (required fields have input, amounts are numbers, etc.) before checking if the record exists. DB transactions are one of the more costly processes.

I am not sure if magic_quotes_gpc is enabled and looking through the hosts FAQ (1&1 shared hosting) there is no indication one way or the other. I will look more into that when I get home.

You can use phpinfo() to check.  Just make a page with the following:

<?php phpinfo(); ?>

The search the output for the 'magic_quotes_gpc' directive and see what the values are.

You can programatically disable magic quotes at runtime if needed.

While it's possible to disable it at runtime, it does not do any good.  By the time your script is executed, PHP will already have collected the input data and run the magic quotes process so changing the setting has no effect.