0

So I have this code that I'm executing when someone presses the "register" button.

I've read over the thing 10 times and can't find what I'm doing wrong.

The problem is: When you click register it doesn't insert the details into the database (even though it says it registered the user). I even looked through my db.php where I store the DB details.

Code for register page:

    <?php
    if(isset($_POST['register'])){
                            $username = $_POST['username'];
                            $password = $_POST['password'];
                            $email = $_POST['email'];
                            $checkUsername = $odb->prepare("SELECT COUNT(*) FROM users WHERE username = :username");
                            $checkUsername->execute(array(':username' => $username));
                            $countUsername = $checkUsername -> fetchColumn(0);
                            $checkEmail = $odb->prepare("SELECT COUNT(*) FROM users WHERE email = :email");
                            $checkEmail->execute(array(':email' => $email));
                            $countEmail = $checkEmail -> fetchColumn(0);
                            if(!($countEmail == 0))
                            {
                                 echo '<p>This e-mail has been taken.</p>';
                             }
                             elseif(!($countUsername == 0))
                             {
                                  echo '<p>Error - this username has been taken.</p>';
                                }
                             else
                            {
                                try
                                {
                                    $insertUser = $odb -> prepare('INSERT INTO users (username,password,email) VALUES(:username, :password, :email)');
                                    $insertUser -> execute(array(':username' => $username, ':password' => $password, ':email' => $email));
                                    echo 'Sucessfully registered.';
                                }
                                catch (PDOException $e)
                                {
                                    echo 'Error: ' .$e->getMessage();
                                }
                            }
                }
?>

When I use if(isset($_POST['register'])){ it doesn't echo any message or alter the database.

When I use if(isset($_GET['register'])){ it echos user registered but nothing is added to the database.

Here's my DB.php :

    <?php
define('DB_HOST', 'localhost');
define('DB_NAME', 'some_db');
define('DB_USERNAME', 'some_dbuser');
define('DB_PASSWORD', 'password');
$odb = new PDO('mysql:host=' . DB_HOST . ';dbname=' . DB_NAME, DB_USERNAME, DB_PASSWORD);
$odb->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
?>

I hope someone knows what's going on, thanks!

chris85
  • 23,846
  • 7
  • 34
  • 51
Raghav J.
  • 53
  • 8

2 Answers2

0

I think you have a problem with the PDO data check this out:

http://php.net/manual/en/pdo.prepare.php

I am a MYSQL guy but I get the same error when not using mysql_real_escape_string and this error looks to be same but with PDO.

user2420647
  • 183
  • 11
  • PDO usage is correct, variable assignment was incorrect. You should familiarize yourself with `PDO` or `mysqli`. `Mysql_` functions are outdated/insecure. – chris85 Oct 04 '15 at 16:39
  • I already use mysqli and I know mysql is obsolete. But even in mysqli you have real_escape_string. You should keep on topic. AND also I said in the first part that the data sent into PDO is proberbly the error. A statement even then this is correct since the variable assignment is incorrect. End discussion on my point – user2420647 Oct 04 '15 at 16:45
  • You shouldn't escape you should use prepared statements. I'm on topic as well. I solved this umm.. +/- 10 minutes before your "answer". As to escaping in `mysqli`... `Thus, prepared statements are simply a more convenient and less error-prone approach to this element of database security.` From mysqli manual. Topic ended. – chris85 Oct 04 '15 at 16:57
  • Not even interested in trashtalking you. Good day to you. – user2420647 Oct 04 '15 at 17:10
0

Your form is processing as a GET request, not POST. The default form method is GET so either change/add method="post" or use $_GET. Using POST is a better option when sending user private user data.

Option A:

<form method="POST">

Option B (changing variable assignment):

if(isset($_GET['register'])){
    $username = $_GET['username'];
    $password = $_GET['password'];
    $email = $_GET['email'];
    $checkUsername = $odb->prepare("SELECT COUNT(*) FROM users WHERE username = :username");
    $checkUsername->execute(array(':username' => $username));
    $countUsername = $checkUsername -> fetchColumn(0);
    $checkEmail = $odb->prepare("SELECT COUNT(*) FROM users WHERE email = :email");
    $checkEmail->execute(array(':email' => $email));
    $countEmail = $checkEmail -> fetchColumn(0);
    if(!($countEmail == 0)) {
         echo '<p>This e-mail has been taken.</p>';
    } elseif(!($countUsername == 0)) {
          echo '<p>Error - this username has been taken.</p>';
    } else {
        try {
            $insertUser = $odb -> prepare('INSERT INTO users (username,password,email) VALUES(:username, :password, :email)');
            $insertUser -> execute(array(':username' => $username, ':password' => $password, ':email' => $email));
            echo 'Sucessfully registered.';
        } catch (PDOException $e) {
            echo 'Error: ' .$e->getMessage();
        }
    }
}

Also as previously noted passwords shouldn't be stored in plain text. MD5 and SHA1 are better than doing that but they aren't the best methods any more. Take a look at these posts:

http://php.net/manual/en/faq.passwords.php#faq.passwords.fasthash
https://security.stackexchange.com/questions/211/how-to-securely-hash-passwords

Community
  • 1
  • 1
chris85
  • 23,846
  • 7
  • 34
  • 51