Need a little help with the update code

Hello everyone!..

Can someone please help me to clear a mistake which I seem not able to find!..I use the below script for updating user details, but when this script run I see no changes to the password which was what I intended to update. If I remove the check section(i.e. check if password is not empty or the two passwords match the code seem to work fine)…but this is not what I want I want to be able to know the password field was filled and that the two fields real match!..can someone please help!..:frowning:


<?php include('includes/dbConnect.inc.php');

		include('includes/corefuncs.inc.php');
		//remove backslashes
		nukeMagicQuotes();
		//initilise flag
		$done = false;
		//prepare an array of expected items
		$expected = array('password1', 'password2','userID');
		//create database connection
		$conn = dbConnect('admin');
		//get details of selected record
		if($_GET && !$_POST){
			if(isset($_GET['userID']) && is_numeric($_GET['userID'])){
				$userID = $_GET['userID'];
			}
			else{
				$userID = NULL;
			}
			if($userID){
				$sql = "SELECT * FROM users WHERE userID = $userID";
				$result = mysql_query($sql) or die(mysql_error());
				$row = mysql_fetch_assoc($result);
			}
			}
			//if form has been submitted, update record
			if(array_key_exists('update',$_POST)){
				//prepare expected items for insertion into database
				foreach($_POST as $key => $value){
					if(in_array($key, $expected)){
						${$key} = mysql_real_escape_string($value);
					}
				}
				//abandon if primary key is invalid
				if(!is_numeric($userID)){
					die('invalid requst');
				}
				//prepare sql query
				
				
				//added code
				$password1 = htmlspecialchars($_POST['password1']);
				$password2 = htmlspecialchars($_POST['password2']);
				
				//initialise error array to display errors
	$error = array();
	//check if all fields have been filled
		if( empty($password1) || empty($password2) ){
			$error[] = 'Please fill in all the details';
		}
				
	//check password length
		if(strlen($password1)<4 || preg_match('/\\s/', $password1)){
			$error[] = 'Password should be at least 4 characters; no spaces';
		}
	//check if password do match
		 if( $password1!=$password2){
		 	$error[] = 'Password do not match; re-enter same password';
		 }
	if no error, check for duplicate username
		if(!$error){
		$pass1 = md5('$password1');
						
				$pass1 = md5($_POST['password1']);
				
						
				$sql = "UPDATE users SET password = '$pass1' WHERE userID = $userID";
				//submit query
				$done = mysql_query($sql) or die(mysql_error());
				
				header ('Location: List_All_User.php');
			
			}
			}//end if(!error)
		
		//redirect page if $userID is invalid
		if(!isset($userID)){
			header('Location: List_All_User.php');
			exit;
		}
?>

And this is the form…



<?php if(empty($row)){?>
<p class="warning">Invalid request: record does not exist.</p>
<?php } else {

if (isset($error)) {
  echo '<ul>';
  foreach ($error as $item) {
    echo "<li>$item</li>";
	}
  echo '</ul>';
  }


?>
   
		<form id="update"  name="update" method="post" class="formfield" action="Update_User.php">
        <h1>Change user password</h1>
        <div >
<label class="fixedwidth">First Name:</label>
<?php echo htmlentities($row['fname']); ?>	</div>
    <div >
<label class="fixedwidth">Other Names:</label>
<?php echo htmlentities($row['othernames']); ?>	</div>
	 	<div >
<label class="fixedwidth">Username:</label>
<?php echo htmlentities($row['username']); ?>	</div>
    
   			 <div>
                    <label for="gender" class="fixedwidth">Gender:</label>
                    <?php echo $row['gender']; ?>                </div>
	 
	 <div >
<label for="userrole" class="fixedwidth">User Role:</label>
<?php echo htmlentities($row['userrole']); ?>	</div>
 <div >
<label class="fixedwidth">Password:</label>
<input name="password1" id="password1" type="password" class="fixedwidth" />
	</div>
<div>
<label class="fixedwidth">Re-type Password:</label>
<input name="password2" id="password2" type="password" class="fixedwidth"  />
	</div>	
	<div>
    <input name="userID" type="hidden" value="<?php echo htmlentities($row['userID']); ?>" />
    </div>
	<div class="buttonarea">
      <div align="center">
        <input name="update" id="update" type="submit" value="update user" />
      </div>
	</div>
</form>

<?php } ?>
          <p align="center"><img src="images/headingbg.gif" alt="div" height="18" /></p>
          </td> 
       
      </tr> 
    </table> 


‘U cannot test your own code’:slight_smile:

var_dump() your variables prior and after any conditional fork in your code.

Especially before doing things like;


if(!$error){}

Which is trying to test a variable which is not even set if everything passes.

Its generally seen as good practice to declare a variable, such as this array, before you start filling it up.


$error = array();

Get used to var_dump() ing these vars as you work, and really make sure you have understood the difference between != and !==

PHP: Comparison Operators - Manual

PHP comparison operators - a Tutorial

Also read this kind of extended “php truth table” to see the kind of holes you can be digging for yourself by testing for equals (==) rather than identical (===).

PHP Variable and Array Tests

I’m not saying dont use == I am saying that when you are scratching your head because your code does not fork correctly, var_dump() will shed light on the problem.

Keep doing that till you learn to do the var_dump() as you develop, instead of trying to debug.

Somethings need to be changed. First do not mess with the value of the password, do not encode it, filter, or validate its contents. So change:


//added code
$password1 = htmlspecialchars($_POST['password1']);
$password2 = htmlspecialchars($_POST['password2']);

To this:


//added code
$password1 = $_POST['password1'];
$password2 = $_POST['password2'];

Why should there be no spaces? Do not limit the characters a user can enter as a password. Also a password length of 4 is way to low, 6 to 8 is a better minimum however longer is better. Change the below:


//check password length
if(strlen($password1)<4 || preg_match('/\\s/', $password1)){
  $error[] = 'Password should be at least 4 characters; no spaces';
}

To this:


//check password length
if(strlen($password1) < 6){
  $error[] = 'Password should be at least 6 characters';
}

As for using md5 without a salt…Please do not do that. Add a salt and think about using SHA1 or greater like SHA256 or SHA512.