why does my php script create two entries in my mysql db for every form submitted?

Question

here's my script:

$name=$_POST['name'];
$email=$_POST['email'];
$pass=$_POST['pass2'];
$year=$_POST['year'];
$month=$_POST['month'];
$day=$_POST['day'];
$skype=$_POST['skype'];
$yahoo=$_POST['yahoo'];

if ($name == "" || $email == "" || $pass == "") {
echo "Please fill in all the fields";
}

$username = "username";
$password = "password";
$host = "localhost";

mysql_connect($host,$username,$password) or die("No connect");

mysql_select_db("dbname") or die("Unable to select database");

$query = "INSERT INTO members VALUES

('','$name','$email','$pass','$year','$month','$day','$skype','$yahoo')";
mysql_query($query) or die(mysql_error());

$ok = "registered";

if (mysql_query($query)) {
echo($ok);
}
else {
}

?>

Answer 1

You are calling mysql_query($query) twice - once after you declare it, and again inside the if block. You should do something like $result = mysql_query($query) or die....

Then check if($result) { ... }

Answer 2

Some observations:

1. The previous answerer was correct, you execute your SQL query twice, which is why you get two rows.

2. You're not checking your input data, and that's an open invitation to injection attacks (SQL and HTML). Also, you may overflow your database fields if you don't check lengths on your inputs. I've written a function in the code below that does all that. All you need to do is, when calling the function, set the arguments to be correct for your field.

For example, if the name column is a varchar(50) field in your database, you'd use:

$name = prepText( $_POST['name'], 50 );

If it's a varchar(225) field, you'd use:

$name = prepText( $_POST['name'], 225 );

3. I have used a foreach loop to check whether the fields in the form are filled in. It will require all users to provide some info in each form field. If you only want name, email and password to be checked, use this foreach block instead:

foreach( $_POST as $key => $value) {
if($key == "name" || $key == "password" || $key == "email" ) {
if( trim($value) == "" ) {
$valuesOK = false;
break;
}
}
}

4. It appears you may be using month, day and year as MySQL column names. If so, you should not, as those words have special meanings in MySQL. Instead, I have changed those column names to myyear, mymonth and myday, which removes their special MySQL meaning.

5. I've placed this code block inline in an HTML page to demonstrate that you don't need to construct an entire HTML page in your confirmation variable, you can simply echo out the string you want to act as confirmation.




function prepText($somestring, $length) {
//function cleans up $somestring for database insert
//trims line to match specific length to prevent DB field overflow
$somestring = substr( $somestring, 0, $length );
$somestring = mysql_escape_string( $somestring );
$somestring = htmlspecialchars( $somestring );
$somestring = trim( $somestring );
return $somestring;
}

//set boolean to check for good fields
$valuesOK = true;

//loop through all form fields;
//if any are blank, set boolean to false
foreach($_POST as $item) {
if( trim( $item ) == "" ) {
$valuesOK = false;
break;
}
}

//if boolean is true, process insert
if( $valuesOK ) {
//clean up form inputs
$name = prepText( $_POST['name'], 50 );
$email = prepText( $_POST['email'], 50 );
$pass = prepText( $_POST['pass2'], 50 );
$year = prepText( $_POST['year'], 4);
$month = prepText( $_POST['month'], 2);
$day = prepText( $_POST['day'], 2);
$skype = prepText( $_POST['skype'], 50);
$yahoo = prepText( $_POST['yahoo'], 50);

//connect to db, select database
$username = "username";
$password = "password";
$host = "localhost";

$link = mysql_connect( $host, $username, $pass ) or die( "Cannot connect to database server" );
mysql_select_db("dbname") or die("Unable to select database");

//create insert query and execute it
$query = "INSERT INTO members ( name, email, pass, myyear, mymonth, myday, skype, yahoo ) VALUES ( '$name', '$email', '$pass', $year, $month, $day, '$skype', '$yahoo' )":
mysql_query($query) or die(mysql_error());
echo "

Answer 3

Looks like you still need to execute your SQL. Like this: $sql = "SQL HERE"; $query = mysql_query($sql) or die("Cannot query the database.
" . mysql_error()); I would also make sure it's only posting when submitted. If you have two seperate files, this is not an issue. If your using the same file, add a hidden form field named 'submitted' and give it a value of 'yes', then surround your database code with: if($_POST['submitted'] == 'yes'){ DB code here. }