PHP problem - unknown problem.

+2 Jad Samadi · December 23, 2014
So hey there guys i want to start first by thinking you for helping me in the previous problems i had and well yet i have a new one today .
As i was working on my login/Register system i actually was surprised to realize something new in PHP .... or maybe its just my fault .
Any way let me show you what is happening .
This is my code : 


<?php


include 'core/init.php';





if (empty($_POST) === false) {


$username = $_POST['username'];


$password = $_POST['password'];





          if (empty($username) === true || empty($password) === true) {


             $errors[] = 'You need to enter a username and a password';


          } else if(user_exists($username) === false) {


            $errors[] = 'We can\'t find that username. Have you registered ?';


          }else if (user_active() === false) {


            $errors[] = 'You haven\'t activated your account';


          } else { 


           //here


          }











print_r($errors);


}


?>



i don't suppose any thing is wrong in there yet when you test it you will notice that one of the 'else if' statements don't actually work . ... its like the file is only reading the first one.
Ill explain more . I opened my index page and tried to login with empty username and password boxes . It gives me this error :
Array ( [0] => You need to enter a username and a password )
witch is great ... that means it works !
Now i tried to test if the 'user_exists' function work . So i entered a username and password that isn't saved in my database and i got that error :
Array ( [0] => We can't find that username. Have you registered ? )
Works great ! Now the problem .... When i try to test the 'user_active' function it doesn't work ... I enter the username and password that is saved in my database with the active state equal to 0 where i should then get the error :
You haven\'t activated your account
But instead i get this error :
Array ( [0] => We can't find that username. Have you registered ? )
Now i know i probably made a mistake in the function so it does not work but no i didn't.
I tried to switch the active function and the exists one and my code became :


<?php


include 'core/init.php';





if (empty($_POST) === false) {


$username = $_POST['username'];


$password = $_POST['password'];





          if (empty($username) === true || empty($password) === true) {


             $errors[] = 'You need to enter a username and a password';


          }else if (user_active() === false) {


            $errors[] = 'You haven\'t activated your account';


          } else if(user_exists($username) === false) {


            $errors[] = 'We can\'t find that username. Have you registered ?';


          } else { 


           //here


          }











print_r($errors);


}


?>



And for some reason now i cant get this error :
Array ( [0] => We can't find that username. Have you registered ? )
instead i get this one :
Array ( [0] => You haven't activated your account )
So ye ... for both cases if i enter a correct or incorrect username/password.
I don't think i can explain more and i think this code would help as well(the functions) :


<?php





function user_exists($username){


         $username = sanitize($username);


         return (mysql_result(mysql_query("SELECT COUNT (`user_id`) FROM `users` WHERE `username` = '$username'"), 0) ==1) ? true : false;


}





function user_active($username){


         $username = sanitize($username);


         return (mysql_result(mysql_query("SELECT COUNT (`user_id`) FROM `users` WHERE `username` = '$username' AND `active` = 1"), 0) ==1) ? true : false;


}





?>




So if you can help me i would really appreciate it .Thanks for reading

Post a Reply

Replies

- page 1
Oldest  Newest  Rating
0 Jay Deshaun · December 26, 2014
I suggest converting to MySQLi like Ron Butcher said.

And 'mysql_result' still returns Strings not Numbers/Ints.
0 Ron Butcher · December 26, 2014
If your query fails it is the cause of your problems.  

In your user_exists function, you run the query that counts the number of users with the specific id and if the active field is 1.  If the query finds a match (like it should on a valid login) it returns true, otherwise it returns false.  So when the query runs and fails, it is equal to an error rather than 1 like it is supposed to.  Your statement therefore returns false and that is what your else if statement is looking for to execute its code.  The page will then populate the $errors array and exit the if else statements since it is only looking for one of those to be true.

The other thing is you need to make sure that you are checking to see if the user exists before checking to see if it is active.  I would use nested if statements instead of the else if statements.

I know that this is how the tutorials log in users, but to be honest I am not a huge fan of this style, but that is just me.  Here is how I do it in a nutshell:


<?php
$errors = '';

if(!empty($_POST))
{
$username = sanitize($_POST['username']);
$password = sanitize($_POST['password']);

if(!empty($username) || !empty($password))
{
$qry = "SELECT `id`, `username`, `password`, `active` FROM `users` WHERE `username` = '$username'";  //  Pull necessary information from the database about the user
$result = $mysqli->query($qry) //  I prefer the OOP method of mysqli, it is just a personal preference.

if($result->num_rows != 1)   // Check to make sure that only one result is present
{
$errors = 'Username or Password is Incorrect';  //  I don't tell the user which part they entered incorrectly to discourage hackers.
}
else
{
$userData = $result->fetch_object();   //  Gather all the users information to finish comparing
//  Right here is where I would normally hash and add salt to the $password variable to prepare it to compare to my database.
if($password != $userData->password)   //  Check to see if the password is correct
{
$errors = 'Username or Password is Incorrect';
}
else if(!$userData->active)  //  Check to see if the user is active or not
{
$errors = 'This Account is No Longer Active.  Please Contact the Site Administrator';
}
else
{
$login_user($userData);
}
}
}
else  //  This statement only runs if one of the fields was left blank.
{
$errors = 'Please Enter Both A User Name and Password';
}
}



It is a little more code, but using the nested if statements allow me a lot more control over the situation.  It allows me to run the database query only one time and use those results for the rest of the page.  That makes the page a bit quicker, especially if I have a huge database.

I can also check to see if the user is supposed to change their password, or if there is another specific alert they are supposed to get upon login.

I can also use this setup to log bad username attempts, or bad password attempts.  I can lock the user account after so many bad attempts, and the log allows me to watch for hackers using dictionary or similar attacks on my site.
0 Jad Samadi · December 26, 2014
alright thanks for your reply but after testing your code :

<?php
include 'core/init.php';

$errors = '';

if(!empty($_POST))
{
$username = sanitize($_POST['username']);
$password = sanitize($_POST['password']);

if(!empty($username) || !empty($password))
{
$qry = "SELECT `id`, `username`, `password`, `active` FROM `users` WHERE `username` = '$username'";
$result = $mysqli->query($qry)

if($result->num_rows != 1)
{
$errors = 'Username or Password is Incorrect';
}
else
{
$userData = $result->fetch_object();

if($password != $userData->password)
{
$errors = 'Username or Password is Incorrect';
}
else if(!$userData->active)
{
$errors = 'This Account is No Longer Active.  Please Contact the Site Administrator';
}
else
{
$login_user($userData);
}
}
}
else
{
$errors = 'Please Enter Both A User Name and Password';
}
}
          
          print_r($errors); 
}

?>

And after testing i got this error : ( ! ) Parse error: syntax error, unexpected 'if' (T_IF) in C:\wamp\www\lr\login.php on line 14
And then after going back to the code i found the 'if' on this line :

if($result->num_rows != 1)

And i tried to fix it but  i didn't find a thing .
Please help ?
0 Jay Deshaun · December 23, 2014
You forgot to add the '$username' between the brackets of 'user_active'...

Yours:
user_active()


Fix:
user_active($username)

By the way, what's 'sanitize'?
http://php.net/manual-lookup.php?pattern=sanitize&scope=quickref">http://php.net/manual-lookup.php?pattern=sanitize&scope=quickref
0 Jad Samadi · December 23, 2014
i actually did add $username ... but the problem isn't there the main problem is that the text editor or like Apache or what ever is ignoring the second command after the first .... 
0 Ron Butcher · December 26, 2014
Sorry, I missed the semicolon on the line above (line 12).  It should be 

$result = $mysqli->query($qry); // <-Missing ;

if($result->num_rows != 1)
//....... Rest of code
0 Jay Deshaun · December 23, 2014
The 
mysql_result


return's strings not numbers.

Try this:

<?php
include 'core/init.php';

if (empty($_POST) === false) {
$username = $_POST['username'];
$password = $_POST['password'];

if (empty($username) || empty($password))
$errors[] = 'You need to enter a username and a password';
else if(!user_exists($username))
$errors[] = 'We can\'t find that username. Have you registered?';
else if (!user_active($username))
$errors[] = 'You haven\'t activated your account';
else
//here

print_r($errors);
}

function user_exists($username){
$username = sanitize($username);
if($query = mysql_query("SELECT user_id FROM users WHERE username = '$username'"))
if(mysql_result($query,0)=="1")
return true;
return false;
}
function user_active($username){
$username = sanitize($username);
if($query = mysql_query("SELECT user_id FROM users WHERE username = '$username' AND active = 1"))
if(mysql_result($query,0)=="1")
return true;
return false;
}

?>
0 Jad Samadi · December 23, 2014
nope its not working :./ but thanks for trying to fix it :/
0 Jad Samadi · December 27, 2014
Alright sorry if im being annoying or any thing .
Thanks  for your reply and after testing the code :


<?php

include 'core/init.php';
ini_set('display_errors', FALSE);

$errors = '';

if(!empty($_POST))
{
$username = sanitize($_POST['username']);
$password = sanitize($_POST['password']);

if(!empty($username) || !empty($password))
{
$qry = "SELECT `id`, `username`, `password`, `active` FROM `users` WHERE `username` = '$username'";
$result = $mysqli->query($qry);

if($result->num_rows != 1)
{
$errors = 'Username or Password is Incorrect';
}
else
{
$userData = $result->fetch_object();

if($password != $userData->password)
{
$errors = 'Username or Password is Incorrect';
}
else if(!$userData->active)
{
$errors = 'This Account is No Longer Active.  Please Contact the Site Administrator';
}
else
{
$login_user($userData);
}
}
}
else
{
$errors = 'Please Enter Both A User Name and Password';
}



                  print_r($errors);
        }

          

?>


This part works :

$errors = 'Please Enter Both A User Name and Password';

Yet this whole part don't ;

$qry = "SELECT `id`, `username`, `password`, `active` FROM `users` WHERE `username` = '$username'";
$result = $mysqli->query($qry);

if($result->num_rows != 1)
{
$errors = 'Username or Password is Incorrect';
}
else
{
$userData = $result->fetch_object();

if($password != $userData->password)
{
$errors = 'Username or Password is Incorrect';
}
else if(!$userData->active)
{
$errors = 'This Account is No Longer Active.  Please Contact the Site Administrator';
}
else
{
$login_user($userData);
}
}
}

If i leave both the  username and password empty it give me the error :

Please Enter Both A User Name and Password

Witch is good but if i leave 1 empty it give me a blank page witch is bad for there were no errors
Im still really new to this language of PHP and to be honest im trying to understand your code and i did yet i don't get the problem.
So please help
Thanks for reading.
0 Ron Butcher · December 24, 2014
The problem on your code is in the COUNT function.  You need to remove the ticks (`) from inside the parenthesis and remove the space between the word COUNT and the parenthesis.

function user_exists($username){
$username = sanitize($username);
return (mysql_result(mysql_query("SELECT COUNT(user_id) FROM `users` WHERE `username` = '$username'"), 0) ==1) ? true : false;
}

function user_active($username){
$username = sanitize($username);
return (mysql_result(mysql_query("SELECT COUNT(user_id) FROM `users` WHERE `username` = '$username' AND `active` = 1"), 0) ==1) ? true : false;
}



The reason it always fails at the first else if statement is because the query fails, and then the function returns false.  It is always a good idea to add some error checking into your mysql queries.  Here is an example:


function user_active($username){
$username = sanitize($username);
$result = mysql_query("SELECT COUNT(user_id) FROM `users` WHERE `username` = '$username' AND `active` = 1") or die(mysql_error());
return (mysql_result($result, 0) ==1) ? true : false;
}

The or die statement will kill the page and print out the error.  Not the perfect solution when you go live with your site, but perfect for troubleshooting in development.


One more thing, mysql is depreciated, start looking into mysqli (MySQL Improved) instead.  It is almost the same as mysql so as you are going through the tutorials it will not take much to modify the code to mysqli.
  • 1
  • 2

PHP

117,991 followers
About

Server-side, HTML embedded scripting language used to create dynamic Web pages.

Links
Moderators