Jump to content

A Parse Error That I Can't Seem To Solve


White_Lily

Recommended Posts

Hi I am trying to adjust my registration form because another web developer in the company said that all my empty() checking was making the form checking slow, and to put them into a foreach array with the same message for each failed field check.

 

I tried to do this however i have come accross a parse error that makes no sense to me.

 

Here is my code:

 

<?php

if($_POST["submitReg"]){
$validateArray = array("name" => "text", "email" => "email");

foreach($validateArray as $key => $value)
{
if($value == "text"){
$$key = htmlspecialchars(mysql_real_escape_string($_POST[$key]));
}else{
$$key = mysql_real_escape_string($_POST[$key]);
}

if(empty($$key))
{
$msg3 = "Please fill in all fields of this form.";
}
}

$regName = htmlspecialchars(mysql_real_escape_string($_POST["name"]));
$regUsername = htmlspecialchars(mysql_real_escape_string($_POST["username"]));
$regPassword = htmlspecialchars(mysql_real_escape_string($_POST["password"]));
$regREpassword = htmlspecialchars(mysql_real_escape_string($_POST["rePassword"]));
$email = mysql_real_escape_string($_POST["email"]);

if(preg_match("^[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$", $_POST["email"]) === 0){
$msg3 .= "Please enter a valid email.";
}else{
if($regPassword != $regREpassword || $regREpassword != $regPassword){
$msg3 .= "The password fields didn't match.";
}else{
$check = select("*", "members", "username = '$regUsername'");
$assoc = mysql_fetch_assoc($check);

if($regUsername == $assoc["username"]){
$msg3 .= "That username has been taken, pick another.";
}else{
$regPassword = sha1($regPassword);
$id = uniqid();

$register = insert("members", "name, email, username, password, user_level, id, ban", "'$regName', '$email', '$regUsername', '$regPassword', 1, '$id', 0");

if($register){

$newsTitle = "New member registered!";

$cont = $regUsername." has just joined Fusion Forums!<br>";
$cont.= "Check out his/her profile:<br><br>";
$cont.= "View Profile";

$newsCont = $cont;

$newMem = insert("news", "news_title, news_content, username", "'$newsTitle', '$newsCont', '$regUsername'");

if($newMem){
$to = $email;
$subject = "Fusion Forums - Account Confirmation";
$message = "Hello! You have recently registered to Fusion Forum's.<br><br>";
$message.= "This is a confirmation email, below you will find your account details along with a Unique ID.<br><br>";
$message.= "In order to activate your account you must first enter the ID into the text field that follows the link at the end of this email. Your details are as follows:<br><br>";
$message.= "<table>";
$message.= "<tr>";
$message.= "<td><strong>Name:</strong></td>";
$message.= "<td></td>";
$message.= "<td>".$regName."</td>";
$message.= "</tr>";
$message.= "<tr>";
$message.= "<td><strong>Email:</strong></td>";
$message.= "<td></td>";
$message.= "<td>".$email."</td>";
$message.= "</tr>";
$message.= "<tr>";
$message.= "<td><strong>Username:</strong></td>";
$message.= "<td></td>";
$message.= "<td>".$regUsername."</td>";
$message.= "<tr>";
$message.= "<tr>";
$message.= "<td><strong>Unique ID:</strong></td>";
$message.= "<td></td>";
$message.= "<td>".$id."</td>";
$message.= "<tr>";
$message.= "</table><br><br>";
$message.= "Please follow this link in order to activate your account (opens in your default browser):<br>";
$message.= "<a href='http://www.janedealsart.co.uk/activate.php?id=".$id."'>Activate Account</a>";
$from = "noreply@janedealsart.co.uk";
$headers = 'MIME-Version: 1.0' . "\r\n";
$headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
$headers.= "From: ".$from;
mail($to, $subject, $message, $headers);
$done = "You have successfully registered to Fusion Fourm's.<br>";
$done.= "We have sent you an email with a confirmation code on it,";
$done.= " when you go to confirm your account you will need this code in order to be able to access the forum's.";

$msg2 .= $done;

}else{
$msg3 .= "Sorry, we could not register your account details, if this persists contact the webmaster. ";
}
}else{
$msg3 .= "Sorry, we could not register your account details, if this persists contact the webmaster. ";
}
}
}
}

if($msg2){
echo '<div class="success">Success: '.$msg2.'</div>';
}else{
if($msg3){
echo '<div class="error">Error: '.$msg3.'</div>';
}

echo '<form action="" method="POST">';
echo '<label>Full Name:</label>';
echo '<input type="text" class="field" name="name" />';
echo '<div class="clear"></div>';
echo '<label>Email:</label>';
echo '<input type="text" class="field" name="email" />';
echo '<div class="clear"></div>';
echo '<label>Username:</label>';
echo '<input type="text" class="field" name="username" />';
echo '<div class="clear"></div>';
echo '<label>Password:</label>';
echo '<input type="password" class="field" name="password" />';
echo '<div class="clear"></div>';
echo '<label>Again:</label>';
echo '<input type="password" class="field" name="rePassword" />';
echo '<div class="clear"></div>';
echo '<input type="submit" class="button" name="submitReg" value="Register" />';
echo '<div class="clear"></div>';
echo '</form>';

}

?>

 

Here is the error:

 

Parse error: syntax error, unexpected $end in /home/sites/janedealsart.co.uk/public_html/inc/regCheck.php on line 135

 

I am slightly confused as i did a search for the variable $end and it does not exist within the code...

 

However this is line 135:

 

?>

 

Any ideas as to why this is would be appreciated.

 

(I'm not posting here to find out how vulnerable this form is to some types of injection, i just want to sort this error out.)

Edited by White_Lily
Link to comment
Share on other sites

Looks like to me there is an error in your nesting. You have double end braces. I nest differently then you do but I changed it over and you have a double end there in red and missing one below at bottom. I will add my nested code in also.

 

}else{
$msg3 .= "Sorry, we could not register your account details, if this persists contact the webmaster. ";
[color=#ff0000]}
}[/color]else{
$msg3 .= "Sorry, we could not register your account details, if this persists contact the webmaster. ";
}
}
}
}

 

<?php


if($_POST["submitReg"])
{
$validateArray = array("name" => "text", "email" => "email");


foreach($validateArray as $key => $value)
{
 if($value == "text")
 {
  $key = htmlspecialchars(mysql_real_escape_string($_POST[$key]));
 }
 else
 {
  $key = mysql_real_escape_string($_POST[$key]);
 }


 if(empty($key))
 {
  $msg3 = "Please fill in all fields of this form.";
 }
}


$regName = htmlspecialchars(mysql_real_escape_string($_POST["name"]));
$regUsername = htmlspecialchars(mysql_real_escape_string($_POST["username"]));
$regPassword = htmlspecialchars(mysql_real_escape_string($_POST["password"]));
$regREpassword = htmlspecialchars(mysql_real_escape_string($_POST["rePassword"]));
$email = mysql_real_escape_string($_POST["email"]);


if(preg_match("^[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$", $_POST["email"]) === 0)
{
 $msg3 .= "Please enter a valid email.";
}
else
{
if($regPassword != $regREpassword || $regREpassword != $regPassword)
{
 $msg3 .= "The password fields didn't match.";
}
else
{
 $check = select("*", "members", "username = '$regUsername'");
 $assoc = mysql_fetch_assoc($check);


 if($regUsername == $assoc["username"])
 {
  $msg3 .= "That username has been taken, pick another.";
 }
 else
 {
  $regPassword = sha1($regPassword);
  $id = uniqid();


  $register = insert("members", "name, email, username, password, user_level, id, ban", "'$regName', '$email', '$regUsername', '$regPassword', 1, '$id', 0");


  if($register)
  {
   $newsTitle = "New member registered!";
   $cont = $regUsername." has just joined Fusion Forums!<br>";
   $cont.= "Check out his/her profile:<br><br>";
   $cont.= "View Profile";
   $newsCont = $cont;
   $newMem = insert("news", "news_title, news_content, username", "'$newsTitle', '$newsCont', '$regUsername'");


   if($newMem)
   {
 $to = $email;
 $subject = "Fusion Forums - Account Confirmation";
 $message = "Hello! You have recently registered to Fusion Forum's.<br><br>";
 $message.= "This is a confirmation email, below you will find your account details along with a Unique ID.<br><br>";
 $message.= "In order to activate your account you must first enter the ID into the text field that follows the link at the end of this email. Your details are as follows:<br><br>";
 $message.= "<table>";
 $message.= "<tr>";
 $message.= "<td><strong>Name:</strong></td>";
 $message.= "<td></td>";
 $message.= "<td>".$regName."</td>";
 $message.= "</tr>";
 $message.= "<tr>";
 $message.= "<td><strong>Email:</strong></td>";
 $message.= "<td></td>";
 $message.= "<td>".$email."</td>";
 $message.= "</tr>";
 $message.= "<tr>";
 $message.= "<td><strong>Username:</strong></td>";
 $message.= "<td></td>";
 $message.= "<td>".$regUsername."</td>";
 $message.= "<tr>";
 $message.= "<tr>";
 $message.= "<td><strong>Unique ID:</strong></td>";
 $message.= "<td></td>";
 $message.= "<td>".$id."</td>";
 $message.= "<tr>";
 $message.= "</table><br><br>";
 $message.= "Please follow this link in order to activate your account (opens in your default browser):<br>";
 $message.= "<a href='http://www.janedealsart.co.uk/activate.php?id=".$id."'>Activate Account</a>";
 $from = "noreply@janedealsart.co.uk";
 $headers = 'MIME-Version: 1.0' . "\r\n";
 $headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
 $headers.= "From: ".$from;
 mail($to, $subject, $message, $headers);
 $done = "You have successfully registered to Fusion Fourm's.<br>";
 $done.= "We have sent you an email with a confirmation code on it,";
 $done.= " when you go to confirm your account you will need this code in order to be able to access the forum's.";

 $msg2 .= $done;


   }
   else
   {
 $msg3 .= "Sorry, we could not register your account details, if this persists contact the webmaster. ";
   }
   else
   {
 $msg3 .= "Sorry, we could not register your account details, if this persists contact the webmaster. ";
   }
  }
 }
}


if($msg2)
{
 echo '<div class="success">Success: '.$msg2.'</div>';
}
else
{
 if($msg3)
 {
  echo '<div class="error">Error: '.$msg3.'</div>';
 }
}

echo '<form action="" method="POST">';
echo '<label>Full Name:</label>';
echo '<input type="text" class="field" name="name" />';
echo '<div class="clear"></div>';
echo '<label>Email:</label>';
echo '<input type="text" class="field" name="email" />';
echo '<div class="clear"></div>';
echo '<label>Username:</label>';
echo '<input type="text" class="field" name="username" />';
echo '<div class="clear"></div>';
echo '<label>Password:</label>';
echo '<input type="password" class="field" name="password" />';
echo '<div class="clear"></div>';
echo '<label>Again:</label>';
echo '<input type="password" class="field" name="rePassword" />';
echo '<div class="clear"></div>';
echo '<input type="submit" class="button" name="submitReg" value="Register" />';
echo '<div class="clear"></div>';
echo '</form>';


}


?>

Link to comment
Share on other sites

@White_Lilly:

 

The new forum software doesn't do a good job of keeping the formatting for code when posting. So, I don't know if what you posted is representative if the formatting you actually had. But, as snowdog posted you would do yourself a great favor by adding formatting (especially indenting) to provide a visual representation of the logical flow of your code. However, I would also suggest that if you have a very deep nest of logic that you should probably take a second look to see if you can reduce how deep that nesting goes.

 

I do see an error in the code snowdog posted where another ending brace was removed that shouldn't have. That might not have happened with simpler structure. But there are other problems as well:

 

You are running all of the input through htmlspecialchars() and mysql_real_escape_string() BEFORE you validate the data. You should validate the data first THEN transform the data as needed. Using the method you have, valid data could fail the validation. Plus, if I am reading the coed right yu are running the POST values through that same logic twice - once in the foreach loop and again right after that.

 

Also, I would advise not using htmlspecialchars() on the data you save to the database. You should make that change as you echo the content to the page. If you were to ever use the data for something other than directly outputting onto an HTML page the data would be messed up.

 

if($regPassword != $regREpassword || $regREpassword != $regPassword)

Huh?

 

Regarding the nested logic, here is an example of something that can be simplified. Instead of this

 if(preg_match("^[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$", $_POST["email"]) === 0)
{
 $msg3 .= "Please enter a valid email.";
}
else
{
 if($regPassword != $regREpassword || $regREpassword != $regPassword)
 {
	 $msg3 .= "The password fields didn't match.";
 }
 else
 {

 

You could have

 if(preg_match("^[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$", $_POST["email"]) === 0)
{
 $msg3 .= "Please enter a valid email.";
}
elseif($regPassword != $regREpassword || $regREpassword != $regPassword)
{
 $msg3 .= "The password fields didn't match.";
}
else
{

 

But, I don't like logic where the script stops on the first error. It should check for ALL errors then provide the user a list of what failed.

Edited by Psycho
Link to comment
Share on other sites

Here's a rewrite in what I think is a more logical workflow that isn't as complicated:

<?php

//Set error message for form
$error_msg = '';

//Set variable for populating form field values
//Used to make fields sticky in case of errors
$nameHTML = '';
$usernameHTML = '';
$emailHTML = '';

if($_POST["submitReg"])
{
   //Preprocess the inputs
   $regName       = trim($_POST["name"]);
   $regUsername   = trim($_POST["username"]);
   $regPassword   = trim($_POST["password"]);
   $regREpassword = trim($_POST["rePassword"]);
   $regEmail      = trim($_POST["email"]);

   //Run field validations
   $errors = array(); //Array to hold errors

   //Validate name
   if($regName == '')
   {
       $errors[] = "Name is required.";
   }
   else
   {
       $regUsernameSQL = mysql_real_escape_string($regUsername);
       $check = select("COUNT(*)", "members", "username = '$regUsernameSQL'");
       if(mysql_result($check, 0) !== 0)
       {
           $errors[] = "That username has been taken, pick another.";
       }
   }
   //Validate email
   if($regEmail == '')
   {
       $errors[] = "Email is required.";
   }
   elseif(!preg_match("^[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$", $regEmail))
   {
       $errors[] = "Email is not valid.";
   }
   //Validate passwords
   if($regPassword == '' || $regREpassword == '')
   {
       $errors[] = "Password  and confirmation are required.";
   }
   elseif($regPassword != $regREpassword)
   {
       $errors[] = "The password fields didn't match.";
   }

   //If validations passed, attempt to create records
   if(!count($errors))
   {
       //Validations passed
       $regNameSQL = mysql_real_escape_string($regName);
       $emailSQL   = mysql_real_escape_string($email);
       $regPasswordSQL = sha1($regPassword);
       $id = uniqid();
       $register = insert("members",
                          "name, email, username, password, user_level, id, ban",
                          "'$regNameSQL', '$emailSQL', '$regUsernameSQL', '$regPasswordSQL', 1, '$id', 0");
       if(!$register)
       {
           $errors[] = "Sorry, we could not register your account details, if this persists contact the webmaster.";
       }
   }

   //If member was created, attempt to send email
   if(!count($errors))
   {
       $to = $email;
       $subject = "Fusion Forums - Account Confirmation";
       $message = "Hello! You have recently registered to Fusion Forum's.<br><br>";
       $message.= "This is a confirmation email, below you will find your account details along with a Unique ID.<br><br>";
       $message.= "In order to activate your account you must first enter the ID into the text field that follows the link at the end of this email.             details are as follows:<br><br>";
       $message.= "<table>\n";
       $message.= "<tr><td><strong>Name:</strong></td><td>{$regName}</td></tr>\n";
       $message.= "<tr><td><strong>Email:</strong></td><td>{$email}</td></tr>\n";
       $message.= "<tr><td><strong>Username:</strong></td><td>{$regUsername}</td><tr>\n";
       $message.= "<tr><td><strong>Unique ID:</strong></td><td>{$id}</td><tr>\n";
       $message.= "</table><br><br>";
       $message.= "Please follow this link in order to activate your account (opens in your default browser):<br>";
       $message.= "<a href='http://www.janedealsart.co.uk/activate.php?id=".$id."'>Activate Account</a>";
       $from = "noreply@janedealsart.co.uk";
       $headers = 'MIME-Version: 1.0' . "\r\n";
       $headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
       $headers.= "From: {$from}";

       if(!mail($to, $subject, $message, $headers))
       {
           $errors[] = "There was a problem sending your confirmation email.";
       }
   }

   //If user registration passed, add forum message
   if(!count($errors))
   {
       $newsTitle = "New member registered!";
       $cont =  "{$regUsername} has just joined Fusion Forums!<br>";
       $cont.= "Check out his/her profile:<br><br>";
       $cont.= "View Profile";
       $newsCont = $cont;
       $newMem = insert("news", "news_title, news_content, username", "'$newsTitle', '$newsCont', '$regUsername'");

       //If this fails the user registration still completed. No need to provide error to user.
   }

   //If there were errors, create error message for form
   if(count($errors))
   {
       $error_msg  = "<div class='error'>";
       $error_msg .= "The following error(s) occured:\n";
       $error_msg .= "<ul>\n";
       foreach($errors as $error)
       {
           $error_msg .= "<li>$error</li>\n";
       }
       $error_msg .= "</ul>\n";
       $error_msg .= "</div>";

       //Set the form field values to be repopulated
       $nameHTML = htmlentities($name);
       $usernameHTML = htmlentities($username);
       $emailHTML = htmlentities($email);
   }
   else
   {
       //Include page with register success message
       include('register_success.php');
       //$done  = "You have successfully registered to Fusion Fourm's.<br>";
       //$done .= "We have sent you an email with a confirmation code on it,";
       //$done .= " when you go to confirm your account you will need this code in order to be able to access the forum's.";
       exit();
   }
}

?>
<html>
<head><title>Registration Form</title></head>
<body>
<?php echo $error_msg; ?>
<form action="" method="POST">
<label>Full Name:</label>
<input type="text" class="field" name="name" value="<?php echo $nameHTML; ?>" />
<div class="clear"></div>
<label>Email:</label>
<input type="text" class="field" name="email" value="<?php echo $emailHTML; ?>" />
<div class="clear"></div>
<label>Username:</label>
<input type="text" class="field" name="username" value="<?php echo $usernameHTML; ?>" />
<div class="clear"></div>
<label>Password:</label>
<input type="password" class="field" name="password" />
<div class="clear"></div>
<label>Again:</label>
<input type="password" class="field" name="rePassword" />
<div class="clear"></div>
<input type="submit" class="button" name="submitReg" value="Register" />
<div class="clear"></div>
</form>
</BODY>
</HTML>

Edited by Psycho
Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.