The problem is that when I fill the form, it inserts two entries on sql, the first one which I entered and the second one is blank ! I really don't know why it adds the second one. I doubt on if($result)
in which I think that $result executes a second time, and that's why it is showing the blank row.
<?php
$connection = mysql_connect("localhost", "db", "pass");
mysql_select_db("dbname", $connection);
$name = mysql_real_escape_string(stripslashes($_POST['name']));
$email = mysql_real_escape_string(stripslashes($_POST['email']));
$password = mysql_real_escape_string(stripslashes($_POST['pass']));
$sql="INSERT INTO users VALUES('$name','$email','$password')";
$result = mysql_query($sql);
if($result) {
echo '{"success":1}';
} else {
echo '{"success":0,"error_message":"Sorry, your registration failed. Please go back and try again."}';
}
?>
If your database table contains more then 3 columns, E.G id,name,email,password then you should specify which columns you want to insert values into.
"INSERT INTO users (name, email, password) VALUES (:name, :email, :pass);"
Though doing this will not cause double row inserts so you will need to elaborate on how you are posting these values to this file. Are you by chance using jQuery to POST the values but not returning false or preventing the default action on the form? (Deduced from the JSON responce)
Also you have no protection or checks that its even POST, a bot could hit your code and cause many blank inserts as you are not checking anything just inserting blindly.
Also please take notice of the other answers about security if you intend to put your code into production.
Why are you using strip_slashes()
?
mysql_real_escape_string()
is enough. Although of course prepared statements using PDO are the modern, safer way to do things.
I also suggest you use:
if($result !== false)
instead of if($result)
to be sure that you are are getting a positive result.
Can you post your full results for query "SHOW CREATE TABLE [table_name]" so we can see your table structure?
As someone else suggested, you shouldn't save plain text passwords in your database.
function securePassword($plain_text_pw) {
$salt = 'jklolomfg99';
$salted_password = $salt . $password;
$hashed_pw = md5($hashed_pw);
return $hashed_pw;
}
function checkPassword($username,$plain_text_pw) {
$secure_pw = securePassword($plain_text_pw);
// query would look something like this:
// $sql = "SELECT `id` FROM `users` WHERE `name` = "' . $username . '"
// AND `secure_password` = "' . $secure_password . '"';
// if no results, then return false. If 1 result, return true. log user in.
}
Note: SHA1 may be more secure than MD5, but you probably are not going to be the victim of world class hackers.
Not sure why it's adding a blank row. Could the form be submitting twice?
There's quite a bit you can do here to improve the security.
You could add some validation, for example:
Also you shouldn't add the password straight into the DB. Storing passwords in their original form is very bad practice. You should first add a 'salt' which is a key known only to your application (i.e any random string 'dwewsd2r23345wfsdf') then hash the whole thing. This will make life a bit harder for any hackers who manage to access your DB. See here for more on password hashing
Finally, you should use PDO instead of the mysql functions for your database queries. If you use prepared statements that will add another layer of security. See here for more on PDO.