0

I have a problem with register form.My form works properly but whenever i try to insert username that already exists it doesn't shows any error.

here is my php register file:

<?php
$servername = "localhost";
$username = "root";
$password = "";

try {
    $conn = new PDO("mysql:host=$servername;dbname=dblogin", $username, $password);

    // set the PDO error mode to exception
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    if (isset($_POST['submit'])) {
        $user_name = $_POST['user_name'];
        $user_email = $_POST['user_email'];
        $user_pass = $_POST['user_pass'];
        $hash = password_hash($user_pass, PASSWORD_DEFAULT);

        $stmt = $con->prepare("SELECT user_name FROM users WHERE user_name = :user_name");

        if($stmt->rowCount() > 0){
        echo "exists!";
        }

       else{  
        $insert = $conn->prepare("INSERT INTO users (user_name,user_email,user_pass) values(:user_name,:user_email,:user_pass)");

        $insert->bindparam(':user_name',$user_name);
        $insert->bindparam(':user_email',$user_email);
        $insert->bindparam(':user_pass',$hash);


        $insert->execute();    
      }
    }
 catch(PDOException $e)
    {
    echo "connection failed";
    }


?> 

Thanks for your support

  • you miss 1 } in your if statement if (isset($_POST['submit'])) { if($stmt->rowCount() > 0){ echo "exists!"; } } – Barclick Flores Velasquez Oct 19 '17 at 05:19
  • 1
    your code formatting is a bit off so its hard to see but it looks like a missing `}` for the `try` block so will result in a parse error. `ini_set('display_errors',1);` and `error_reporting(E_ALL);` will show you this – andrew Oct 19 '17 at 05:19
  • 2
    Also you have a typo on your first stmt: $con instead of $conn. – LKKP4ThX Oct 19 '17 at 05:24
  • An advice: Don't use `rowCount()`, as it is not quite reliable - see "Description" part of [PDOStatement::rowCount](http://php.net/manual/en/pdostatement.rowcount.php). Use [PDOStatement::fetch](http://php.net/manual/en/pdostatement.fetch.php) and check if result is FALSE. If FALSE, then no records were found. Otherwise it contains the fetched record. Or, as an alternative to `fetch()`, use `query()` as in "Example #2" in [PDOStatement::rowCount](http://php.net/manual/en/pdostatement.rowcount.php). –  Oct 19 '17 at 06:02
  • Another advice: Instead of using `setAttribute()` pass a driver options array as last argument to `$conn = new PDO(...);`. Why? Read the "Note" after "Example #4" in [Connections and Connection management](http://php.net/manual/en/pdo.connections.php). –  Oct 19 '17 at 06:16
  • And a last suggestion: use `bindValue()` instead of `bindParam()`. The best argumentation by example ever is in the answer of @lonesomeday in [What is the difference between bindParam and bindValue?](https://stackoverflow.com/questions/1179874/what-is-the-difference-between-bindparam-and-bindvalue) –  Oct 19 '17 at 06:59
  • `$stmt = $con->prepare("SELECT user_name FROM users WHERE user_name = :user_name");` what happened to `$stmt->execute();` before your if statement? It's throwing the error as $stmt isn't fetching anything.. – Option Oct 19 '17 at 08:29

4 Answers4

0

You are not executing the select statement. You need to bind params and execute the select statement, try this after the select statemnt.

$stmt->bindparam(':user_name',$user_name);
$stmt->execute();
Farsheel
  • 610
  • 6
  • 17
0
 public function usernameCheck($username)
{

    $sql = "SELECT * FROM $this->table where username = :username";
    $query = $this->pdo->prepare($sql);
    $query->bindValue(':username', $username);
    $query->execute();
    if ($query->rowCount() > 0) {
        return true;

    } else {
        return false;
    }

}

use this one in your project hope it will work... :)

Atul Singh
  • 11
  • 3
  • thanks for your response atul but where to insert this code. can you give full code please?does i have to place `insert` statement in `else` – Maunil parikh Oct 19 '17 at 06:02
  • hey you no need to do that "if and else" things just simple call my function in your code it will work(just need to give your table name for prepare statement just give your connection(that is $conn->prepare()))...do it by yourself..if you have any problem thn let me know.. – Atul Singh Oct 19 '17 at 07:26
0

missing } in if statement

if (isset($_POST['submit'])) {
        $user_name = $_POST['user_name'];
        $user_email = $_POST['user_email'];
        $user_pass = $_POST['user_pass'];
        $hash = password_hash($user_pass, PASSWORD_DEFAULT);

        $stmt = $con->prepare("SELECT user_name FROM users WHERE user_name = :user_name");

        if($stmt->rowCount() > 0){
        echo "exists!";
        }
}else{

}
  • 2
    and the statement is never executed as @farsheel Rahman points out – andrew Oct 19 '17 at 05:22
  • @andrew "but whenever i try to insert username that already exists it doesn't shows any error." try to understand this. it isn't hard by the way. – Barclick Flores Velasquez Oct 19 '17 at 05:26
  • 1
    I was just trying to be helpful by pointing out there is another error in the code so that you could improve your answer, in fact there are two other errors as `$con` is used instead of `$conn`. Why not read the comments? it isn't hard by the way – andrew Oct 19 '17 at 05:30
0

I notice 4 things (2 of which have been mentioned by others):

First and smallest is you have a spelling error ($con instead of $conn) - don't worry it happens to the best of us - in you first $stmt query which means your select-results becomes NULL instead of 0 - so you rowCount find that it is not over 0 and moves on without your error message

Second you forgot to bind and execute the parameters in your first $stmt query which gives the same result for your rowCount results

Third always clean your variables even when using prepared statements - at a bare minimum use

$conn->mysql_real_escape_string($variable);

and you can with advantage use

htmlspecialchars($variable);

And fourth since you are not doing anything with the database (other than looking) you could simplify your code by simply writing:

$stmt = $conn->query("SELECT user_name FROM users WHERE user_name = '$user_name' LIMIT 1")->fetch();

as I said - no need to bind or execute in the first query

and as a general rule - don't use rowCount - ever - if you have to know the number of results (and in 99% of cases you don't) use count(); but if you as here just want to know if anything at all was found instead use:

if ( $stmt ) {
    echo "exists!";
} else {
    // insert new user as you did
}

Edit:

Also - as a side note - there are a few things you should consider when you initially create your connection...

Ex:

// Set variables
$servername = "localhost";
$username = "***";
$password = "***";
$database = "***";
$charset = 'utf8'; // It is always a good idea to also set the character-set

// Always create the connection before you create the new PDO
$dsn = "mysql:host=$servername;dbname=$database;charset=$charset"; 

// Set default handlings as you create the new PDO instead of after
$opt = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // And add default fetch_mode
    PDO::ATTR_EMULATE_PREPARES   => false, // And ALWAYS set emulate_prepares to false
];

// And now you are ready to create your new PDO
$conn = new PDO($dsn, $username, $password, $opt);

Just a suggestion... happy trails

APM
  • 97
  • 7
  • Sorry, I don't want to be obtrusive ;-) I just want to say, that the `query()` statement would be perfect in the absence of any `where` clause parameter passed through the user input. But, if at least one is present, like the one for `user_name`, then Maunil should still use `prepare()` and `execute()` instead of `query()`, in order to avoid the MySQL injection. –  Oct 19 '17 at 06:29
  • true if he was using the result later like showing the result on a webpage or other... but he is only looking in the database and using the result count to write exists if found any... so there is no chance of mysql injection anywhere – APM Oct 19 '17 at 06:33
  • Actually no. In preparing an sql statement it's not about how the result is used, but about to avoid giving the app user the chance to inject unproperly escaped strings for the parameters. The preparing process assures that whatever is passed as value for the parameter is read by the engine as an unobtrusive, normal string. –  Oct 19 '17 at 06:41
  • he should of course first clean his variables (just noticed he didn't)... ups... I just assumed... to OP - ALWAYS clean your variables before using them (even if you user prepared statements) at a bare minimum use mysql_real_escape_string($variable); – APM Oct 19 '17 at 06:42
  • Yes, in this situation the use of `query` is legit. With a note: when used together with `prepare()`, the `mysql_real_escape_string()` leads to double escaped user input, e.g. to false results. So, if `prepare()` is used, no `mysql_real_escape_string()` should be used. –  Oct 19 '17 at 06:49
  • like I said aenderei you are correct sir... if they are not cleaned he shouldn't use query in a single line :) – APM Oct 19 '17 at 06:49