Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 9 of 9
  1. #1
    New Coder
    Join Date
    Jul 2018
    Posts
    10
    Thanks
    2
    Thanked 0 Times in 0 Posts

    Hashed password not inserting into mysql database

    Hello all,

    I've been revising code written in mysqli prepared statements that worked in php5 but no longer does in php7... specifically salting passwords.
    I've read that creating a salt is deprecated and one should use the password_hash function.

    In my registration script it first checks if a username and/or email address exists. If not proceed to inserting username, email and password into the database.

    But it's not working. Nothing is being inserted and I can't figure out why.

    Here is my script...

    <?php
    $dbhost = 'localhost';
    $dbuser = 'me';
    $dbpass = '123456';
    $dbname = 'testdb';

    $nameTable = 'test';
    $string = '';

    $conn = new mysqli($dbhost,$dbuser,$dbpass,$dbname);

    if($conn->connect_error) {
    echo $string = 'valid=8';
    }
    else {

    $username = '';
    $userpass = '';
    $usermail = '';
    $todo = '';

    if (isset($_POST['username'])) {
    $username = $_POST['username'];
    }
    if (isset($_POST['userpass'])) {
    $userpass = $_POST['userpass'];
    }
    if (isset($_POST['usermail'])) {
    $usermail = $_POST['usermail'];
    }

    if (isset($_POST['todo'])) {
    $todo = $_POST['todo'];
    }

    ///////////////////// Registration block /////////////////////////////

    if ($todo == 'reg') {
    // create a prepared statement
    if ($s = mysqli_prepare($conn, "SELECT `id` FROM `{$nameTable}` WHERE `username`=? OR `usermail`=?")) {
    // bind parameters for markers
    mysqli_stmt_bind_param($s, "ss", $username, $usermail);
    // execute query
    mysqli_stmt_execute($s);
    // bind result variables
    mysqli_stmt_bind_result($s, $id);
    // fetch count
    $num_rows = 0;
    while (mysqli_stmt_fetch($s)) {
    $num_rows++;
    }
    // close statement
    mysqli_stmt_close($s);

    if ($num_rows > 0) {

    echo $string = 'valid=0&err=double data';
    } else {
    $hashedpass = password_hash($userpass, PASSWORD_DEFAULT);
    if ($s = mysqli_prepare($conn, "INSERT INTO `{$nameTable}` ".
    "(id,username,userpass,usermail) ".
    "VALUES (NULL,?,?,?)")) {
    mysqli_stmt_bind_param($s, "sss", $username, $hashedpass, $usermail);
    mysqli_stmt_execute($s);

    if (1 == mysqli_stmt_affected_rows($s)) {
    echo $string = 'valid=1&err=success';
    } else {
    echo $string = 'valid=2&err=insert to db fail';
    }
    mysqli_stmt_close($s);
    } else {

    echo $string = 'valid=3&err=fail';
    }
    }

    } else {
    echo $string = 'valid=4&err=id fail';
    }

    }
    ////////////////////////////////////////////////
    }

    $conn->close();

    ?>

    The error I'm getting is echo $string = 'valid=0&err=insert to db fail';

    Obviously no rows are being affected so hence the error. Why? What am I doing wrong?

    Been banging my head against a wall for the last couple of weeks on this. Any help would be much appreciated.

    Cheers

  2. #2
    Senior Coder deathshadow's Avatar
    Join Date
    Feb 2016
    Location
    Keene, NH
    Posts
    3,126
    Thanks
    4
    Thanked 455 Times in 444 Posts
    The user ID should be an auto-increment, shouldn't it? If so why are you declaring it and passing null? I mean if this is an insert, I'd lose that altogether...

    Code:
    if ($s = $conn->prepare('
    	INSERT INTO `' . $nameTable . '` (
    		username, userpass, usermail
    	) VALUES (
    		?, ?, ?
    	)
    ')) {
    Should be all you need.

    I'd also suggest you swing an axe at all the 'variables for nothing'.. you've got prepare/execute you don't need to be screwing around wasting RAM and processing time copying your post variables out of $_POST anymore. Likewise going full Gimli on the (pointless overhead inducing) procedural wrappers for mysqli's object model wouldn't hurt, though in general given what it is you're trying to do I would highly suggest you forget mysqli and migrate over to PDO. It's just easier to deal with on stuff like this.

    You also could REALLY use more 'else' logic in there, as you're checking a bunch of values for existing but don't actually USE them! There's literally zero error handling in there.

    Your first query is also a bit of a wonk, since blindly sending unset values to a bind isn't the greatest idea. You should be building that query as it goes. That handling of username || usermail is a bit sketchy.
    “There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies.” – C.A.R. Hoare, The 1980 ACM Turing Award Lecture
    http://www.cutcodedown.com

  3. #3
    Senior Coder CFMaBiSmAd's Avatar
    Join Date
    Oct 2006
    Location
    Denver, Colorado USA
    Posts
    4,210
    Thanks
    3
    Thanked 544 Times in 530 Posts
    The execute() calls can fail as well, but you have no error handling for them. Also, you need to determine what value IS being returned by the mysqli_stmt_affected_rows($s) call. Just because it isn't a one, doesn't mean it is a zero. You could have two or more rows in your database table with the same values, because running a SELECT query to find if data already exists has a race condition where concurrent instances of your script can try to test the same values, find that they don't exist, and end up inserting the same data.

    For handling database statement (connection, query, prepare, and execute) errors, you should use exceptions and in most cases let php catch the exception, where it will use its error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. Using exceptions for errors will let you remove the error handling logic you have now, which will simplify and clean up the code.

    For handling duplicate data, your database table needs to be defined with the two columns as unique indexes. This will prevent duplicate data from being inserted. All you need to do then is try and insert the data and detect if a duplicate key error occurred. This is a case where your code would catch the exception from the execute() call, and detect if the error was a duplicate key error. If it is, you can then run a SELECT query to find which of the two values are the duplicate(s). If the error is not a duplicate key error, you would re-throw the exception and let php handle it.

    You also need to add validation logic for the input data - what should your code do if any of the required values are empty? And eliminate unnecessary code and variables - all of the isset($_POST[...]) are pointless, because all form fields (except for unchecked checkbox and radio) will be set after the form has been submitted.
    Finding out HOW to do something is called research, i.e. keep searching until you find the answer. After you attempt to do something and cannot solve a problem with it yourself, would be when you ask others for help.

  4. #4
    New Coder
    Join Date
    Jul 2018
    Posts
    10
    Thanks
    2
    Thanked 0 Times in 0 Posts
    I probably should have mentioned that I'm a total newb. I followed a tutorial years back. However, as said all seemed to work ... once LOL.
    So can you be more specific re 'swinging an axe' at the variables and full gimli on procedural wrappers?
    Re the else logic and error handling... I've got that. I just removed them to post here for brevity's sake and to show where the error was coming from.

  5. #5
    New Coder
    Join Date
    Jul 2018
    Posts
    10
    Thanks
    2
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by CFMaBiSmAd View Post

    For handling duplicate data, your database table needs to be defined with the two columns as unique indexes. This will prevent duplicate data from being inserted. All you need to do then is try and insert the data and detect if a duplicate key error occurred. This is a case where your code would catch the exception from the execute() call, and detect if the error was a duplicate key error. If it is, you can then run a SELECT query to find which of the two values are the duplicate(s). If the error is not a duplicate key error, you would re-throw the exception and let php handle it.

    And eliminate unnecessary code and variables - all of the isset($_POST[...]) are pointless, because all form fields (except for unchecked checkbox and radio) will be set after the form has been submitted.
    As said above am new to this. How do I define the 2 columns as unique indexes?

    I'm not sure if your latter comment applies in my case. I don't use a form (I think you're referring to html?). The php works with an old flash actionscript3 game I made. Those variables are declared in the AS
    code. One of the reasons why I want to update all the php is that I'm looking for a developer to rewrite the AS3 into a language like Unity3d. A potential developer would need to see the game in action before considering taking on the job and I'm not handing over all my work until I've hired someone. And as said my original php is no longer working.

  6. #6
    Senior Coder CFMaBiSmAd's Avatar
    Join Date
    Oct 2006
    Location
    Denver, Colorado USA
    Posts
    4,210
    Thanks
    3
    Thanked 544 Times in 530 Posts
    How do I define the 2 columns as unique indexes?
    Using your database management tool (phpmyadmin or similar), you would define a unique index for both the username and usermail columns.

    Your flash/action-script is making a post request to the server-side code and sending it the post data. Your server-side code needs to validate the data, because it is the last line of defense before using the data. A hacker can submit his own data to your server-side code and the code must deal with whatever gets submitted. By only using data that is all valid, you can reduce server-side resources, by avoiding running code and queries unless all expected data is valid.
    Finding out HOW to do something is called research, i.e. keep searching until you find the answer. After you attempt to do something and cannot solve a problem with it yourself, would be when you ask others for help.

  7. #7
    New Coder
    Join Date
    Jul 2018
    Posts
    10
    Thanks
    2
    Thanked 0 Times in 0 Posts
    I understand your first comment.

    But is the second related to the first? ie hackers. Would having unique id's for user and email prevent that? Or are you referring to something separate.

    Could you provide an example of how I should avoid running code and queries unless all the data is valid?

    I'm told I should do this or not do this and I'm totally confused now. Sorry.

  8. #8
    Senior Coder CFMaBiSmAd's Avatar
    Join Date
    Oct 2006
    Location
    Denver, Colorado USA
    Posts
    4,210
    Thanks
    3
    Thanked 544 Times in 530 Posts
    Validating the input data before using it will do three things -

    1) For legitimate visitors who are using your client-side front-end to submit the data, it will improve the User eXperiance (UX.) If they leave a required field empty or they submit a value that doesn't meet the requirements (length, character mix, format) you have defined for the value, your code will output a unique and helpful error message for each detected problem telling the visitor exactly what they did that was not correct, so that they can correct the input value and try again.

    2) It will help make your code self-troubleshooting. If you have a coding mistake somewhere, and keep getting a validation error for a particular value, even when the submitted value is valid, you will know there is a mistake associated with that value, or if you keep getting a same type validation error for all values, that there is shared mistake somewhere in the coding.

    3) For nefarious visitors and bot scripts submitting data to your site, it will filter out submissions that don't have expected data values.

    Could you provide an example of how I should avoid running code and queries unless all the data is valid?
    A general purpose method is to use an array to hold validation error messages. Each validation test that fails will add an element to the array. At any point, you can determine if there are errors or not by testing if the array is not empty() or is empty(). After all the validation tests, if the array is empty, you know that the submitted data is valid and can use it in the rest of the logic.

    See the following example code showing the recommendations that have been made -

    PHP Code:
    <?php

    // define 'helper' function(s) - these would typically be defined in an external .php file and 'required' when needed
    // recursive function to trim data
    function _trim($val){
        if(
    is_array($val)){
            return 
    array_map('_trim',$val);
        } else {
            return 
    trim($val);
        }
    }

    $dbhost 'localhost';
    $dbuser 'me';
    $dbpass '123456';
    $dbname 'testdb';

    $nameTable 'test';

    // set the mysqli error mode to exceptions - then remove any existing error handling logic
    mysqli_report(MYSQLI_REPORT_ERROR MYSQLI_REPORT_STRICT);

    $conn = new mysqli($dbhost,$dbuser,$dbpass,$dbname); // an error will throw an exception, which php will handle

    $errors = []; // define an array to hold errors
    $post = []; // define an array to hold a working copy of the submitted form data

    // post method form (or your choice of client-side front-end) processing code
    if($_SERVER['REQUEST_METHOD'] == 'POST'// this is a general purpose way of detecting if a post method form has been submitted.
    {
        
    // get a trimmed copy of the submitted form data
        
    $post array_map('_trim',$_POST);

        switch(
    $post['todo'])
        {
            case 
    'reg':
                
    ///////////////////// Registration block /////////////////////////////

                // validate the submitted data here and set errors in the $errors array
                // inputs - username, userpass, usermail
                // at a minimum, you would need to test if the values are not empty strings
                
                // a hard-coded example of testing if the username is an empty string
                // for more than about three input values, you would instead set up an array defining what validation tests you want to perform for each input, and dynamically validate the data by looping over the defining array and call defined functions to handle each different type of validation
                
    if($post['username'] == '')
                {
                    
    $errors['username'] = "The username is empty.";
                }
                
                
    // other validation tests would go here...
                
                // if no errors, use the submitted data
                
    if(empty($errors))
                {
                    
    // attempt to insert the data and detect if a duplicate key error occurs
                    
    $sql "INSERT INTO `{$nameTable}` (username,userpass,usermail) VALUES (?,?,?)";
                    
    $stmt $conn->prepare($sql); // an error will throw an exception, which php will handle
                    
    $hashedpass password_hash($post['userpass'], PASSWORD_DEFAULT);
                    
    $stmt->bind_param("sss"$post['username'], $hashedpass$post['usermail']);
                    try { 
    // a local try/catch to handle a specific error type
                        // attempt to execute the prepared query
                        
    $stmt->execute(); // an error will throw an exception, which this code will handle
                    
    } catch (mysqli_sql_exception $e) {
                        if(
    $e->getCode() == 1062// duplicate key error number
                        
    {
                            
    // if there is more than one unique column defined, you would run a SELECT query here to find which column(s) were duplicates and set up unique and helpful error messages for each column that was duplicated
                            // actual code is left up to you...
                            
                            // if there is a single unique column, just set the error for it
                            // $errors['username'] = "The username is already in use.";
                        
    } else {
                            throw 
    $e// re-throw the exception if the error number is not handled by this logic
                        
    }
                    }
                    
    // at this point, if there are no errors, registration was successful
                
    }
            break;
            
            
    // add other post method actions here...
            
        
    }
    }
    // note: php automatically closes the database connection when the script ends, so, you don't need to in your code

    // if there are errors, you would display them when you re-display the form, or if not using a form as the client-side front-end, output the errors back to the client-side code
    Finding out HOW to do something is called research, i.e. keep searching until you find the answer. After you attempt to do something and cannot solve a problem with it yourself, would be when you ask others for help.

  9. Users who have thanked CFMaBiSmAd for this post:

    whizzbang (Jul 12th, 2018)

  10. #9
    New Coder
    Join Date
    Jul 2018
    Posts
    10
    Thanks
    2
    Thanked 0 Times in 0 Posts
    Thank you my friend. It's very kind of you to have helped me so.... complete with detailed explanations.
    Will implement the code and see how I go.
    Am very grateful.


 

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •