1
\$\begingroup\$

I made my first script of MySQLi. I have just learnt it from 3rd party website. So, I am not sure I am using functions which are not deprecated or outdated. I should start to practice good scripts to access databases. So, I have posted here. Any mistake you see, please point it out.

<?php

include "functions.php";

$a = checkLogin();

if($a==1) {
    $id = $_COOKIE['id'];

    include "databaseConnector.php";

    $query = $mysqli->query("SELECT * FROM users WHERE id = '$id'") or die($mysqli->error.__LINE__);

    if($query->num_rows > 0) {
        while($row = $query->fetch_assoc()) {
?>
            <?php echo stripslashes($row['firstname'])." ".stripslashes($row['lastname']); ?>   
<?php
        }
    }
    else {
        return false;   
    }
}
else {
    redirectIndex();
}

?>
\$\endgroup\$
3
\$\begingroup\$

You are vulnerable to SQL injection. Always use prepared statements with parameterized queries.

The below example fixes these issues. In this example I am using PDO instead, which is similar to mysqli but more powerful and supports other databases than just MySQL.

<?php

include "functions.php";

$a = checkLogin();

if ($a == 1) {
    $id = $_COOKIE['id'];

    // include "databaseConnector.php";
    // --------------------------------------------------------------------------------------------------
        try {
            $handle = new PDO("mysql:host=$host_addr;dbname=$database", $username, $password);
            $handle->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        } catch(PDOException $e) {
            echo 'ERROR: ' . $e->getMessage();
        }
    // --------------------------------------------------------------------------------------------------

    // UNBOUND PARAMETERS VERSION
        $query = "SELECT * FROM users WHERE id = '$id'";
        $statement = $handle->prepare($query);
        $statement->execute();

    // BOUND PARAMETERS VERSION
        $query = "SELECT * FROM users WHERE id = :id";
        $statement = $handle->prepare($query);
        $params = array(":id" => $id);
        $statement->execute($params);

    $row_count = $statement->rowCount();
    $result    = $statement->fetchAll(PDO::FETCH_ASSOC);

    if($row_count > 0) {
        foreach ($result as $row => $col) {
            echo $col['firstname']." ".$col['lastname']; 
        }
    } else {
        return false;   
    }
} else {
    redirectIndex();
}

?>
\$\endgroup\$
3
\$\begingroup\$

Functions used in your code are not deprecated.

However, most of them are highly not recommended in your use case, namely

  • die() should never be used to report errors. There are much better ways. See this answer on Stack Overflow for the details.
  • $mysqli->query should never be used to run a query that includes a variable or your site is busted through an infamous SQL injection. See this answer on Stack Overflow for the details.

Besides, your code is just generally not optimal and untidy, with many unnecessary conditions. To avoid them you must have the mysqli connection properly configured in your databaseConnector.php file. After than you can make your code much simpler, cleaner and safer.

Last but not least. Always separate your database interactions from the output. It will make your code even more cleaner and versatile.

<?php

include "functions.php";

if(checkLogin(){

    $id = $_COOKIE['id'];

    include "databaseConnector.php";

    $stmt = $conn->prepare("SELECT * FROM users WHERE id=?");
    $stmt->bind_param("i", $id);
    $Stmt->execute();
    $result = $stmt->get_result(); //get mysqli result
    $row = $result->fetch_assoc(); // fetch data   
} else {
    redirectIndex();
}

here your PHP part should stop and then in the HTML part you can continue

<?php if($row): ?>
    <?= htmlspecialchars($row['firstname']) ?>
    <?= htmlspecialchars($row['lastname']) ?>
<?php endif ?>
\$\endgroup\$
-2
\$\begingroup\$

mysqli is somewhat outdated, you should see PDO that is much better both with security and database support (12 drivers vs. 1).

You can see here details of mysqli vs. PDO: http://net.tutsplus.com/tutorials/php/pdo-vs-mysqli-which-should-you-use/

\$\endgroup\$
  • \$\begingroup\$ If MySQL is all you need, then I honestly don't think sticking with mysqli is that bad. It's also not marked as deprecated. \$\endgroup\$ – Simon Forsberg Oct 17 at 20:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.