1

My register.php all works fine and when you try to register it says it is all working correctly, but nothing appears in my database. Am i doing something wrong?

<?php
echo "<h1>Register:</h1>";  
$submit = $_POST['submit'];  
//form data  $fullname = strip_tags($_POST['fullname']);  
$username = strip_tags($_POST['username']);  
$password = strip_tags($_POST['password']);  
$repeatpassword = strip_tags($_POST['repeatpassword']);  
if ($submit)  {  
    // check for existance  
    if ($fullname && $username && $password && $repeatpassword)  {  
    if ($password == $repeatpassword)  {
        //check char length of username and fullname  
        if (strlen($username > 25) || strlen($fullname) > 25)  {  
            echo "Length of username or fullname is too long! Max 25 characters for each!";  
        }  else  {
            //check password length  
            if (strlen($password) > 25 || strlen($password) < 6)  {  
                echo "Password must be between 6 and 25 characters";  
            }  else  {  
                //register the user!  
                //encrypt password  
                $password = ($password);  
                $repeatpassword = ($repeatpassword);  
                //open database  
                $connect = mysql_connect("localhost","user","password") 
                    or die ("Couldn't Connect!");  
                mysql_select_db("user_phplogin"); 
                //select database  
                $queryreg = mysql_query("  
                    INSERT INTO `user_phplogin`.`users` (`id`, `fullname`, `username`, `password`) VALUES  (NULL,'$fullname','$username','$password')  "); 
                die("You have been registered! <a href='login.html'> Return to login page</a>");  
                echo "Success!!";  
            }  
        }  
    }  else 
        echo "Your passwords do not match!";  
}  else  
    echo "Please fill in <b>all</b> fields!";  
}
?>
Mark Baker
  • 209,507
  • 32
  • 346
  • 385
Ben J Shapiro
  • 40
  • 1
  • 8
  • can you format your code, that's too hard to read. – piddl0r Dec 07 '12 at 14:39
  • 3
    **warning** your code is vulnerable to sql injection attacks! – Daniel A. White Dec 07 '12 at 14:40
  • 1
    You aren't checking if your `mysql_query` succeeds or not. – N.B. Dec 07 '12 at 14:40
  • 1
    Is your DB named 'user_phplogin' ? Try to put `mysql_select_db("user_phplogin") or die('not the right db'); ` – Sorin Trimbitas Dec 07 '12 at 14:42
  • 1
    **DO NOT** use `mysql_*` functions. They're deprecated and bring serious security implications to the table. Check out `mysqli` as a minimum. – BenM Dec 07 '12 at 14:44
  • 3
    be warned: The `mysql_xxx()` functions are considered obsolete and insecure. If you're writing new code, you should avoid using them. Use the `mysqli_xxx()` functions or the PDO library instead. See http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-function-in-php for more info – SDC Dec 07 '12 at 14:44
  • if (strlen($username > 25) || strlen($fullname) > 25) or if (strlen($username) > 25 || strlen($fullname) > 25)? – Mark Baker Dec 07 '12 at 14:45
  • an echo immediately after a die? – Mark Baker Dec 07 '12 at 14:45
  • 5
    And I love the password encryption routine – Mark Baker Dec 07 '12 at 14:46
  • @N.B. how do i check if it succeeds or not? – Ben J Shapiro Dec 07 '12 at 14:52
  • @BenJShapiro - `mysql_query` returns something, consult the manual to see what. – N.B. Dec 07 '12 at 14:56
  • [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained and the [deprecation process](http://j.mp/Rj2iVR) has begun on it. See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://j.mp/PoWehJ). – tereško Dec 07 '12 at 21:54

2 Answers2

1

The first issue I'll point out is that you're using the obsolete mysql_xx() functions. You should switch to either the mysqli_xx() functions or the PDO library. That alone won't solve the problem, but it's an important thing to point out.

Next: Data sanitation. You're sanitising your data with strip_tags() function. This will prevent your users from posting HTML code, but is insufficient to make your queries safe from SQL injection attacks and other issues. You need to escape the data to make it safe for use in SQL strings. With your existing code, you need to use mysql_real_escape_string() to do this. If you follow my advice and switch to a different set of SQL functions, you'll use either mysqli_real_escape_string() or PDO::quote(). See the relevant PHP manual pages for more info on how to use these functions. This is very important to fix.

Third point: Error handling. See the relevant part of your code:

$queryreg = mysql_query("
                INSERT INTO `user_phplogin`.`users` (`id`, `fullname`, `username`, `password`) VALUES  (NULL,'$fullname','$username','$password')  ");
die("You have been registered! <a href='login.html'> Return to login page</a>");
echo "Success!!";

In this code, you are calling the query function, but then always reporting the "You have registered" message, regardless of what the query function returns. It is possible for the query function to fail and throw an error, which is almost certainly what is happening, but you're not doing anything to check what that error is.

PHP provides error handling functions to help you with this. You need to check that $queryreg is populated, and if not, report a message explaining the error. eg:

if(!$queryreg) {
    die("Something went horribly wrong. SQL error message: ".mysql_error());
}

The output of mysql_error() or (equivalent functions in mysqli or PDO libs) will be the exact error message that MySQL output when the query failed. This should be sufficient to help you debug the problem, so it's good for testing. However, you probably shouldn't be printing messages like this to the user when your code is in production, because (a) it's messy, and (b) it gives unnecessary help to hackers by telling them about your queries and your data structure. In general, it's better to log errors like this, so that you can read them, but report a more generic error message to the user.

I hope that helps you work out the problem.

SDC
  • 14,192
  • 2
  • 35
  • 48
  • thanks @SDC i tried this and got the following: Something went horribly wrong. SQL error message: INSERT command denied to user 'user'@'localhost' for table 'users' – Ben J Shapiro Dec 07 '12 at 15:05
  • okay, so that gives you a good clue as to the nature of the problem. The user that you're using in your mySQL connection doesn't have permissions to do an insert command on the users table. You'll need to go to your database config tool (phpMyAdmin, or whatever you're using to set up your DB) and fix that. It's a DB config error; not something you'll be able to fix in PHP. – SDC Dec 07 '12 at 15:09
  • The rest of my points remain relevant, though -- you still need to fix your security! – SDC Dec 07 '12 at 15:10
  • cant explain how much you have helped but all is working now, i changed the permisions of the user and it worked perfectly. thank you – Ben J Shapiro Dec 07 '12 at 15:12
0

Why you add id? in your table is "autoincrement" id column?

$queryreg = mysql_query("INSERT INTO `users` (`fullname`, `username`, `password`) VALUES  ('$fullname','$username','$password')  ");

and you don't executed

echo "Success";

because you use die() before and script will terminate there

Igor Mancos
  • 314
  • 6
  • 15