php - Can't get the right if else condition in regards of variables connecting to the database

Question

Really can't finish my project because of this condition. My project is a Login Page. It always continues to the first if condition, but it is for else condition. I just only started studying this php now, although it is being taught to us last week.

login.php

<?php
session_start();
include 'register/dbconnect.php';

$uid = $_POST['uid'];
$pwd = $_POST['pwd'];

$sql = "SELECT * FROM user WHERE uid='$uid' AND pwd='$pwd'";
$result = mysqli_query($connect, $sql);

if (!$row = mysqli_fetch_assoc($result)) {
    header("Location: index.php");
} else {
    header("Location: login/home/home.php");
}
?>

index.php (in the login form part)

<form action="login.php" method="POST">

    <input type="text" name="uid" class="loginField" placeholder="Enter your username" required><br />

    <input type="password" name="pwd" id="passss" class="loginField" placeholder="Enter your password" required>

    <img src="withAcc/img/blindeye.png" onMouseOver="showPass()" onMouseOut="hidePass()" id="eye1" class="eyes"><br /><br />

    <p><input type="checkbox" id="keepSigned"  value=""> <label for="keepSigned">Stay signed in</label> &nbsp;&nbsp;&nbsp;&nbsp; 

    Forgot <a href="" style="color: #AA7F03">password</a>?</p>

    <input type="Submit" value="Login" id="logInBut" ><br />

    <p>Do you have an account? <a href="register/register.php" style="color: #AA7F03">Register</a></p>

</form>

Edited login.php

if (!$row = $result->fetch_assoc()) {
    header("Location: index.php");
} else {
    header("Location: login/home/home.php");
}

I edited it like this, but it neither works. It always going to index.php, although the username and password is stored in the database.

That's a very strange `if` condition. Wouldn't it be more intuitive to check the count of the returned rows instead? Also, your code is wide open to SQL injection and you're not checking for errors after executing the query. — David, Apr 08 '17 at 13:15
Oh, i am really sorry. Im new to this php and sql. I just want to make a login form, wherein it can check whether the account exists or not. I just gather that code from net. Didn't know also what sql injection is — Mark Joshua Fajardo, Apr 08 '17 at 13:29
No problem, now's a great time to learn. Stack Overflow has a pretty good set of answers on that subject already: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php It's not necessarily directly related to the problem you're having now, but it will be very helpful in general. (As an added bonus, prepared statements are easier to debug and maintain, so fewer problems occur to begin with.) — David, Apr 08 '17 at 13:31

score 0 · Answer 1 · edited May 23 '17 at 10:30

This is a pretty strange condition:

if (!$row = mysqli_fetch_assoc($result)) {

And I can't help but wonder if some combination of operator precedence and query success/failure is going to silently produce unexpected results here. I imagine a more intuitive way of doing this would be to check the number of rows in the query result:

if (mysqli_num_rows($result) > 0) {

However, there are a couple other improvements you should make as well to round this out a bit better. First, as mentioned in comments on the question above, you should definitely make use of prepared statements to avoid SQL injection. This is important not only for security but also for general stability and debugging of your code. (Directly using input as code like you currently do makes you much more responsible for the syntax of that code, which often leads to errors.)

Additionally, after executing a query (and before examining the results of that query, so before your if statement) you should check if the query was successful. If it's not successful, your $result variable will be null. So check if $result is null and if it is, don't try to use it for logging in. Instead, examine the error from the database. Something as simple as:

echo mysqli_error($connect);

Also, and this is very important, you are currently storing user passwords in plain text. This is a very, very bad thing. User passwords should always be hashed so that they can't be retrieved in their original form. PHP has some built-in functionality to help with this.

Thank you for helping me, but I think the number of rows are not the ones I needed. Instead, the data stored in my database. How to get the data in the database, and compare it to users input is the question I want to solve. — Mark Joshua Fajardo, Apr 08 '17 at 13:50
@M.Fajardo: That's a pretty fundamental change to what you're doing in the code, and to what you're asking in the question. For that I guess all I can do is recommend that you take a look at some tutorials on PHP/MySQL. Now you're basically asking "how do I read data from a database" which is a bit too broad for a Stack Overflow question. We can help with specific problems, such as the one in the question. But we don't really offer complete tutorials. — David, Apr 08 '17 at 14:05

php - Can't get the right if else condition in regards of variables connecting to the database

1 Answers1