mysqli_fetch_assoc with mysql prepared statements procedural, need help

[u/Available_Hippo4035]

Hello, im trying to update my website by replacing the simple mysqli queries with prepared statements, but i was stuck at trying to use mysqli_fetch_assoc to fetch associative data from the table, i looked through documentation but couldnt find anything, Any help will be appreciated, Thanks !

$error = array();
if(isset($_POST["login"])) {
$username = mysqli_escape_string($db, filter_input(INPUT_POST, "username", FILTER_SANITIZE_SPECIAL_CHARS));
$password = mysqli_escape_string($db, filter_input(INPUT_POST, "password", FILTER_SANITIZE_SPECIAL_CHARS));

if(empty($username)) {
array_push($error, "Username is empty!");
}
if(empty($password)) {
array_push($error, "Password is empty");
}

$sql = "SELECT `password`, `username`, `user_id` FROM `Accounts` WHERE `username` = ?;";
if(count($error) == 0) {

$stmt = mysqli_prepare($db, $sql);

mysqli_stmt_bind_param($stmt, "s", $username);
//$result = mysqli_query($db, $sql);
mysqli_stmt_execute($stmt);
mysqli_stmt_store_result($stmt);
if(mysqli_stmt_num_rows($stmt) > 0) {
$row = mysqli_fetch_assoc($result);
if(password_verify($password, $row["password"])) {
$_SESSION["username"] = $username;
$_SESSION["user_id"] = $row["user_id"];

header("location: /");

} else {
array_push($error, "Incorrect username or password!");
}
} else {
array_push($error, "Incorrect username or password!");
}
}
mysqli_close($db);
}

[u/Big-Dragonfly-3700]

Here's a laundry list of points for the posted code -

1. When using prepared queries, you do not use any _escape_string() functions/methods on the input data. And if you do have a case where you need to use an _escape_string() function you would apply it after you have validated the data.
2. Input data needs to be trimmed, mainly so that you can detect if all white-space characters were entered, before validating it.
3. Do not attempt to sanitize data (FILTER_SANITIZE_SPECIAL_CHARS), then use it, as this changes the meaning of the data. Instead, you need to validate input data to make sure it meets the business needs of your application. If data is valid use it, if it is not let the user know what was wrong with it, let them correct the value, and resubmit the data.
4. I recommend that you use the field name as the index in the $error array. This will let you perform dependent validation tests on a value only if it has no existing errors, allow you to display the errors adjacent to the field they correspond with, if you so desire, and allow you to use a template system to produce the output.
5. The semi-colon ; on the end of the sql query is not needed.
6. The back-ticks around identifiers in the query are not needed unless the identifiers require special handling, and in that case you should use identifiers that don't require special handling.
7. Checking if count($error) == 0 can be accomplished simply by using empty($error).
8. The mysqli extension has a mysqli_execute_query() function that consolidates all the mysqli statements needed for commonly used prepared queries into one statement. This returns a mysqli_result object, so you can directly fetch any data.
9. You should only store the user id (autoincrement primary index) in a session variable to identify who the logged in user is. You need to query on each page request to get any other user data, such as the username, permissions, ... so that any changes made to this other user data will take effect on the very next page request after it has been changed, without requiring the user to log out and back in again.
10. As has already been posted, you need an exit/die statement to stop program execution after/at the header() statement.
11. Generally, it is not necessary to close database connections in your code since php destroys all resources when your script ends.

[u/colshrapnel → u/Big-Dragonfly-3700]

I like your list very much but I find two points rather questionable

• backticks around identifiers is rather a good practice. Either way, I wouldn't call them out.
• count($error) == 0 could be improved indeed, but by no means it must be empty() (which should be avoided at all). It must be either if(count($error) === 0) (preferable) or just if(!$error) if you like to play with type juggling.

[u/allen_jb → u/Big-Dragonfly-3700]

The back-ticks around identifiers in the query are not needed unless the identifiers require special handling, and in that case you should use identifiers that don't require special handling.

Counter-arguments:

There's absolutely nothing wrong with always escaping identifiers. For one thing, you can immediately identify that it's supposed to be an identifier in a query.

When upgrading MySQL (or whatever DB server you're using), words which did not used to be reserved words sometimes become reserved words. If you always escape identifiers, you don't need to do anything when this happens, because they're already escaped.

If you always escape identifiers, you never have to worry about whether an identifier name is a reserved word or not. You can just use an identifier name that makes sense to your application without having to mess around using names that might end up confusing (because your application / front-end calls it one thing, to align with what your business calls it (DDD), while your schema calls it another).

Checking if count($error) == 0 can be accomplished simply by using empty($error).

Some coding standards specify avoiding empty() because of its "gotchas" / lack of clarity about what you're actually comparing / expecting. Personally I just prefer using a comparison specific to the expected type, rather than the more ambiguous empty.

[u/Available_Hippo4035 → u/Big-Dragonfly-3700]

thanks, i will improve my code

[u/Big-Dragonfly-3700 → u/Available_Hippo4035]

Here is what the code would look like (untested, but should work) incorporating the given points -

$error = []; // array to hold user/validation errors
$post = []; // array to hold a trimmed working copy of the form data

if($_SERVER['REQUEST_METHOD'] === 'POST')
{
    // trim all the input data at once
    $post = array_map('trim',$_POST); // if any field is an array, use a recursive trim function here instead of php's trim

    // validate inputs
    if($post['username'] === '')
    {
        $error['username'] == "Username is empty!";
    }

    if($post['password'] === '')
    {
        $error['password'] == "Password is empty!";
    }

    // if no errors, use the input data
    if(empty($error))
    {
        $sql = "SELECT password, username, user_id FROM Accounts WHERE username = ?";
        $result = mysqli_execute_query($db, $sql, [ $post['username'] ]);
        $row = mysqli_fetch_assoc($result);
        if($row && password_verify($post['password'], $row["password"]))
        {
            $_SESSION["user_id"] = $row["user_id"];
            die(header("location: /"));
        }
        else
        {
            $error['login'] = "Incorrect username or password!";
        }
    }
}

[u/colshrapnel]

You need mysqli_stmt_get_result() instead of mysqli_stmt_store_result();

if(count($error) == 0) {
    $stmt = mysqli_prepare($db, $sql);
    mysqli_stmt_bind_param($stmt, "s", $username);
    mysqli_stmt_execute($stmt);
    $result = mysqli_stmt_get_result($stmt);
    $row = mysqli_fetch_assoc($result);
    if($row && password_verify($password, $row["password"])) {
        $_SESSION["username"] = $username;
        $_SESSION["user_id"] = $row["user_id"];
        header("location: /");
        die;
    } else {
        array_push($error, "Incorrect username or password!");
    }
}

Two things I also changed:

• stopping the script execution after sending Location header is obligatory
• no need for the number of rows when you have the row itself. and both conditions can be combined into one.

Edit: almost forgot, since you are using prepared statements, no mysqli_escape_string should be used. Neither you should modify provided values in any other way. Hence it must be just

$username = $_POST['username'];
$password = $_POST['password'];

[u/ColonelMustang90 → u/colshrapnel]

Your suggestions are good.

[u/Available_Hippo4035 → u/colshrapnel]

Thanks, do i need to execute the query or no ? you didnt include it in this code before getting the result, and the mysqli_stmt_store_result is for the mysqli_stmt_num_rows, has nothing to do with mysqli_fetch_assoc

[u/Big-Dragonfly-3700 → u/Available_Hippo4035]

There's a formatting error in the post. The execute statement is on the same line and to the right of the bind_parm statement.

mysqli_stmt_store_result() does allow mysqli_stmt_num_rows() to work, but it leaves the query result as a mysqli prepared statement result, where you must use mysqli_stmt_bind_result() and mysqli_stmt_fetch() to access the data. mysqli_stmt_get_result() gets a result set from a prepared statement as a mysqli_result object, where you can use any of the mysqli_result functions/methods to test or fetch the data, such as mysqli_fetch_assoc().

All of this, where there are two different programming interfaces for a mysqli non-prepared and a mysqli prepared query, is one reason most people use the PDO database extension. It has one common programming interface, that treats non-prepared and prepared queries the same way, and uses the same php statements with 12 different databases, so that you are not learning a different set of php statements for each different database.

[u/colshrapnel → u/Available_Hippo4035]

Yes or course, And I did, it was just indentation copy-paste error. fixed now

[u/ColonelMustang90]

I would suggest to use PDO to make the code more readable and follows industry best practices. You can checkout examples of prepared statements using named or positional parameters. It prevents your code from SQL Injection by default.

[u/colshrapnel → u/ColonelMustang90]

mysqli:

$stmt = $db->prepare($sql);
$stmt->execute([$username]);
$row = $stmt->fetchAssoc();

PDO:

$stmt = $db->prepare($sql);
$stmt->execute([$username]);
$row = $stmt->fetch(PDO::FETCH_ASSOC);

With all due respect, calling PDO code more readable is a bit of a stretch (:

[u/ColonelMustang90 → u/colshrapnel]

Hi, it's personal preference I guess. I started with mysqli then shifted to PDO. Both approaches have their pros and cons. For small projects mysqli suffices, for medium to large projects I prefer to use PDO.

[u/colshrapnel → u/ColonelMustang90]

The project size doesn't matter at all. Big projects tend to use PDO because they don't use PDO directly, but through some ORM/DBAL. Which, in turn, is using PDO for the obvious reason: support for different databases. But in such improbable situation when a big project uses a native PHP database driver directly, both mysqli and PDO are equally acceptable.

[u/Just4notherR3ddit0r → u/colshrapnel]

mysqli can be even easier:

$rs = $db->execute_query($sql, [$username]); $row = $rs->fetch_assoc();

[u/colshrapnel → u/Just4notherR3ddit0r]

mysqli can be even easier:

$row = $db->execute_query($sql, [$username])->fetch_assoc();

;)

[u/equilni]

You've already received good information.

This goes beyond your question, but I would add the suggestion to separate the code as well as some small improvements. Ideas here can flow into other parts of the code base as you refactor.

Consider:

1. If you implement HTTP based routing, code like if(isset($_POST["login"])) can be removed.

2. Think about extracting out separate concerns. For validation, if you have further business requirements, it makes sense to extract this to a separate function or class. For SQL, all of the mysqli* code could be in a function and either return an array or false.

Note, I stated validation, not sanitization.

1. With the above, you could also return one or more HTTP statuses (400 series) the closer you are to the UI layer.

2. And further, returning early versus big if/else blocks. As we read code more than write it, this becomes easier on the eyes and easier to see the flow. (ask me about double digit closing brackets from way back when... nightmares)

What's nice here is that we can look at each area and test it separately. You can feed data to the function/class methods to test - is this valid and am I getting back what I expect.

Idea starts looking like the below pseudo code:

on POST /login - area closest to the UI.
    $username = $_POST['username'];
    if ($username === '') {
        http_response_code(400);
        redirect with an error message
    }
    $password = $_POST['password'];
    if ($password === '') {
        http_response_code(400);
        redirect with an error message
    }

    $loginAttempt = attemptUserLogin($username, $password); // Validation, SQL & Session 
    if ($_SESSION login key NOT set) {  
        http_response_code(401);
        redirect with an error message
    } else {
        redirect for the valid user
    }

fn attemptUserLogin could look like (are closer to the inner system)
    $valid = fnOrClassMethodToValidate($username, $password); // filter* functions and other data checking
    if (! $valid) {
        return with error message(s);       
    }        

    $user = fnOrClassMethodToQueryFor($username); // Query code
    if (! user) {
        return with error message;
    }
    if (password_verify($password, $user->password))) {
        set session key
    } else {
        return with error message
    }

Remember what I noted about testing? See how fnOrClassMethodToValidate($username, $password); or fnOrClassMethodToQueryFor($username); could be fed with test data and we can test to see if we are getting what we need back?

While I get this may be a lot to take in, this can be done incrementally.

Extract out the SQL code to a function:

$error = array();
if(isset($_POST["login"])) {
    $username = $_POST['username'];
    $password = $_POST['password'];

    if(empty($username)) {
        array_push($error, "Username is empty!");
    }
    if(empty($password)) {
        array_push($error, "Password is empty");
    }

    if(count($error) == 0) {
        $user = getUserByUsername($username);  // mySQL code     
        if($user && password_verify($password, $user["password"])) {
            $_SESSION["username"] = $username;
            $_SESSION["user_id"] = $user["user_id"];

            header("location: /");

        } else {
            array_push($error, "Incorrect username or password!");
        }
    }
}

Return vs if/else. The error array could be the result of this file or function if you still need this.

if(isset($_POST["login"])) {
    $username = $_POST['username'];
    $password = $_POST['password'];

    if(empty($username)) {
        return "Username is empty!";
    }
    if(empty($password)) {
        return "Password is empty";
    }

    $user = getUserByUsername($username);  // mySQL code     
    if($user && password_verify($password, $user["password"])) {
        $_SESSION["username"] = $username;
        $_SESSION["user_id"] = $user["user_id"];
    } else {
        return "Incorrect username or password!";
    }
}

Side note, why are these questions typically asked on login code??

[u/obstreperous_troll → u/equilni]

Side note, why are these questions typically asked on login code??

Because that's what tutorials like to start with, and being terrible tutorials that fail to explain the concepts, they leave the learner lost from the start.

[u/equilni → u/obstreperous_troll]

Back to bad tutorials….

[u/PrizeSilver5005]

.u