-1

I'm making a username check for php, I need to check if the string is between 3 and 20 characters, I've tried this, but it doesn't work! heres the link to my code sample, I don't know how to work stackoverflow :(

if(isset($_POST['submit'])){
        $user = trim(mysql_real_escape_string($_POST['user']));
        $email = trim(mysql_real_escape_string($_POST['email']));
        $pass1 = trim(mysql_real_escape_string($_POST['pass1']));
        $pass2 = trim(mysql_real_escape_string($_POST['pass2']));
        if(!empty($user) && !empty($email) && !empty($pass1) && !empty($pass2)){
            if(ctype_alnum($user)){
                if(filter_var($email, FILTER_VALIDATE_EMAIL)){
                    if(strlen($user) < 3){
                        if(strlen($user) > 20){
                            $query1 = mysql_query("SELECT username FROM users WHERE username='$user'");
                            $query2 = mysql_query("SELECT email FROM users WHERE email='$email'");
                            $count1 = mysql_num_rows($query1);
                            $count2 = mysql_num_rows($query2);
                            if($count1 == 0 && $count2 == 0){
                                if($pass1 == $pass2){

                                } else {
                                    $output = '<div id="output"><header><h1>Error, passwords do not match!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                                }
                            } else {
                                $output = '<div id="output"><header><h1>Error, username & email are taken!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                            }
                            if($count1 == 1){
                                $output = '<div id="output"><header><h1>Error, username is taken!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                            }
                            if($count2 == 1){
                                $output = '<div id="output"><header><h1>Error, email is taken!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                            }
                        } else {
                            $output = '<div id="output"><header><h1> CC Error, username must be 3-20 characters!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                        }
                    } else {
                        $output = '<div id="output"><header><h1>xx Error, username must be 3-20 characters!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                    }
                } else {
                    $output = '<div id="output"><header><h1>Error, invalid email!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
                }
            } else {
                $output = '<div id="output"><header><h1>Error, username must be alphanumeric!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
            }
        } else {
            $output = '<div id="output"><header><h1>Error, missing fields!</h1><a href="#" onclick="document.getElementById(\'output\').style.display = \'none\';return false;"><i class="fa fa-times"></i></a></header></div>';
        }
    }
RiggsFolly
  • 83,545
  • 20
  • 96
  • 136
rnd
  • 55
  • 1
  • 4
  • 4
    Every time you use [the `mysql_`](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) database extension in new code **[a Kitten is strangled somewhere in the world](http://2.bp.blogspot.com/-zCT6jizimfI/UjJ5UTb_BeI/AAAAAAAACgg/AS6XCd6aNdg/s1600/luna_getting_strangled.jpg)** it is deprecated and has been for years and is gone for ever in PHP7. If you are just learning PHP, spend your energies learning the `PDO` or `mysqli` database extensions and prepared statements. [Start here](http://php.net/manual/en/book.pdo.php) – RiggsFolly Mar 23 '17 at 00:05
  • Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Mar 23 '17 at 00:05
  • 1
    `if(strlen($user) < 3){ if(strlen($user) > 20){` you're using it wrong. Use that in one condition. Plus, make sure that the POST arrays aren't failing you; we don't know what the form looks like. – Funk Forty Niner Mar 23 '17 at 00:05
  • and if you plan on going live with this; don't. If you want to keep your db/site intact, then give up on this code entirely. – Funk Forty Niner Mar 23 '17 at 00:06
  • you're also using way too much code/queries than needed. You are also abusing the usage of conditional statements. – Funk Forty Niner Mar 23 '17 at 00:09
  • And now some hints on how to use SO: don't say "it doesn't work". Provide the specific error that happens or unexpected behaviour, and what you expected. Make your example short, nobody likes wading through lots of code. Yes, this does mean you have to do some work yourself... –  Mar 23 '17 at 00:52

2 Answers2

0

If just a single check on $user:

if (strlen($user) >= 3 && strlen($user) <= 20) {
    ...
}
0

Standard MYSQL is deprecated and should no longer be used. If you ever upgrade your version of PHP on your server you will be in some trouble.

You should look into MYSQLI prepared statements or PDO. Prepared statements are MUCH more secure than a standard query and automatically handle issues like escaping strings.

I would also look into using try/catch statements. Try/catch statement allow you to produce error messages without having to build an HUGE layer of if statements which can save on resources and time. It's also easier on the eyes.

Also I would suggest making ALL of the checks against your post values BEFORE opening your connection or accessing the database. It's good practice to not access or use the database until necessary.

It's also good practice to only access and use the database as least amount of times as possible. Pulling emails and usernames can be done with one query instead of two. Then loop through your data to do your checks. This saves on database activity and resources.

I have provided an example below using your submitted code, which also includes the answer to your original question.

# Start your try/catch statement to check for thrown exceptions (error messages)
try {

    # Check for $_POST to initiate script
    if( !empty($_POST) ){

        # Loop through each post value
        foreach( $_POST as $key => $val ){

            # Check if each post value is empty and throw and exception and if not set it as a variable
            if( !empty($val) ){

                ${$key} = trim($val);

            }

            else {

                # Throw Exception (error message)
                throw new Exception("Error, missing fields.");

            }

        }

        # Check if $user is alphanumeric and is at least 3 to 20 characters (THE ANSWER TO YOUR ORIGINAL QUESTION!!!)
        if( !ctype_alnum($user) || strlen($user) < 3 || strlen($user) > 20 ){

            # Throw Exception (error message)
            throw new Exception("Error, username must be alphanumeric and at least 3 to 20 characters.");

        }

        # Check if $email is valid
        if( filter_var($email, FILTER_VALIDATE_EMAIL) ){

            # Throw Exception (error message)
            throw new Exception("Error, invalid email.");

        }

        # Check if $pass1 and $pass2 are the same value
        if( $pass1 != $pass2 ){

            # Throw Exception (error message)
            throw new Exception("Error, passwords do not match.");

        }

        # Make MYSQLI Connection
        $mysqli = new mysqli($servername, $username, $password, $dbname);

        if ( $mysqli->connect_errno ) {

            # Throw connections error message
            throw new Exception("Error, could not connect to database.");

        }

        # Prepare your query for execution
        $stmt = $mysqli->prepare("SELECT `username`,`email` FROM `users` WHERE `username` = ? OR `email` = ?");

        # Bind the two parameters to your statement
        $stmt->bind_param("ss", $user, $email);

        if ( $stmt === false ) {

            # Throw Exception (error message)
            throw new Exception("Error, could not process data submitted.");

        }

        # Excecute your query
        $stmt->execute();

        if ( $stmt === false ) {

            # Throw Exception (error message)
            throw new Exception("Error, count not execute database query.");

        }

        # Bind the results to a variable
        $stmt->bind_result($users);

        # Fetch your data from results
        while($stmt->fetch()){

            $foundusers = $users;

        }

        if ( $stmt === false ) {

            # Throw Exception (error message)
            throw new Exception("Error, could not get results from database.");

        }

        # Set counters for username and emails found
        $usernames = 0;
        $emails = 0;

        # Loop through each database entry retrieved and check for matching usernames and emails
        foreach( $foundusers as $thisuser ){

            if( !empty($thisuser["email"]) && $thisuser["email"] == $email ){

                # Add 1 to the $emails counter
                $emails++;

            }

            if( !empty($thisuser["username"]) && $thisuser["username"] == $user ){

                # Add 1 to the $usernames counter
                $usernames++;

            }

        }

        # close your statement
        $stmt->close();

        #Check if matching usernames OR emails were found
        if( $usernames > 0 || $emails > 0 ){

            # Check if $usernames and $emails counter is great than 0
            if( $usernames >= 1 && $emails >= 1 ){

                # Throw Exception (error message)
                throw new Exception("Error, username & email are taken.");

            }

            # Check if $usernames counter is great than 0
            if( $usernames >= 1 ) {

                # Throw Exception (error message)
                throw new Exception("Error, username is taken.");

            }

            # Check if $emails counter is great than 0
            if( $emails >= 1 ) {

                # Throw Exception (error message)
                throw new Exception("Error, email is taken.");

            }

        }

    }
    else {

        # Throw Exception (error message)
        throw new Exception("Error, could not initiate script.");

    }

    # Report no usernames were found (only shows if no exceptions are thrown prior to this code)
    $output = "<div onclick=\"this.style.display = 'none'\"><header><h1>Success, username & email are available.</h1><a href='#'><i class='fa fa-times'></i></a></header></div>";

}

# Catch any exceptions thrown and output the error
catch( Exception $e ) {

    # Check if statement is still open and close it
    if($stmt){
        $stmt->close();
    }

    # Create your error response
    $output = "<div onclick=\"this.style.display = 'none'\"><header><h1>" . $e->getMessage() . "</h1><a href='#'><i class='fa fa-times'></i></a></header></div>";

}
Shane Henry
  • 308
  • 1
  • 7
  • Made some edits and added commenting to help you better understand what's happening. – Shane Henry Mar 23 '17 at 02:19
  • Plus I made a little tweak to your javascript onclick. You can actually add it to the div element and use "this" to call the DOM element instead of defining an id and calling the id. Much easier to read. :-) – Shane Henry Mar 23 '17 at 02:40