There is a couple of strange things in your code.
First you insert the person and then check if they should be able to register or not. You should swap this and only allow the values to be inserted if it is ok.
You use the word or instead of || in the if-statement. It is ok to use it, but it have not the same precedence. Using || will avoid a lot of problems.
When you check if a variable is empty you check for an empty string. It is better to do a full check with a few commands. [man]isset[/man] checks if there is a variable or not with that name. [man]trim[/man] removes spaces in the beginning and end of the string. [man]empty[/man] checks if the string is empty or not. Look at the manual at least for the command empty since there are a few ways a variable may be empty.
You should make sure that the queries is safe for database injection by using [man]mysql_real_escape_string[/man] in every variable you get from a source you don't control, and you should do this in every query. I use [man]sprintf[/man] here to make it easier to handle the variables.
I know that was kind of much. I have changed in your code to let you get the idea.
<?php
// First we take care of the variables, there is no need to even open a database connection if they are not ok
// One way to keep track if everything is ok is to use a variable to check that. I use a variable called $ok.
$ok = true; // We set it to true to start with, then change it if something not as it should be.
if (isset($_POST['firstname'])) // First we check if the variable exists
{
$firstname = trim($_POST['firstname']; // Removes spaces in the beginning and end
if (empty($firstname)) // Check if the variable is empty or not.
{
$ok = false; // Set the ok variable to false if everything is not ok.
}
}
if (isset($_POST['lastname']) || !$ok) // Add the check if it is ok to the rest of the variables. Everything else is the same
{
$firstname = trim($_POST['firstname'];
if (empty($firstname))
{
$ok = false;
}
}
// And the same with the other variables. When you have learned a bit more PHP you should use a function for this and call it for each variable instead of writing all in the code.
// When all variables is checked we want to know if we should add it in the database or not.
if ($ok)
{
$con = mysql_connect("xxx","xxx","xxx");
if (!$con)
{
die('Could not connect: ' . mysql_error());
}
mysql_select_db("regist",$con);
$sql= sprintf ("INSERT INTO person (name, lastname, username, password, email, gender, notes)
VALUES ('%s', '%s', '%s', '%s', '%s', '%s', '%s')",
mysql_real_escape_string($firstname),
mysql_real_escape_string($lastname),
mysql_real_escape_string($username),
mysql_real_escape_string($password),
mysql_real_escape_string($email),
mysql_real_escape_string($gender),
mysql_real_escape_string($notes));
if (!mysql_query($sql,$con))
die ('Error: '. mysql_error());
mysql_close($con)
echo "You have registered successfully";
}
else
{
echo "Not all required fields are filled.";
}
?>
