-4

Here is my code I can't get the if statement to work, if name does not exist it reads "Record found" and the page3.php says Passwords do not match can someone please help me with this Thank you

<?php
session_start();
//$_SESSION["authorized"]=0;
$name =  $_POST["name"];
$pass = ($_POST["password"]);

$connect = mysql_connect("localhost","tina","tinapassword") or die("Could not connect");

$selected = mysql_select_db("tinadatabase", $connect) or die("Could not connect to database");

$query = "SELECT * FROM users WHERE Uname='$name'"; 
$result = mysql_query($query, $connect);

$row = mysql_fetch_assoc($result);

if ($result)
{
    //$row ==1;
    print "Record found";

}
else
{
    //$row == 0;
    print "Record not found";   
}
print "<br>";

md5($pass);
if($name == $result["Uname"] && md5('$pass') == $result["Upassword"])
{   
    $_SESSION["authorized"] = 1;    
}
else
{
    $_SESSION["authorized"] = 0;
}
print "<br>";
print"<a href='page3.php'> continue</a>";
?>
  • Welcome to programmers. Please read the [about](http://programmers.stackexchange.com/about) page. This question belongs to StackOverflow. I'll flag it to be migrated. – Aseem Bansal Jul 20 '13 at 17:57
  • sorry didn't know how to use this page I appreciate the help – Tina Woodbury Jul 20 '13 at 18:02
  • 2
    You are using [an **obsolete** database API](http://stackoverflow.com/q/12859942/19068) and should use a [modern replacement](http://php.net/manual/en/mysqlinfo.api.choosing.php). You are also **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that a modern API would make it easier to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Jul 20 '13 at 18:10
  • You are using [an unsuitable hashing algorithm](http://php.net/manual/en/faq.passwords.php) and need to [take better care](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) of your users' passwords. – Quentin Jul 20 '13 at 18:11

4 Answers4

0

mysql_query returns whether a query succeded. Findig zero records is a success.

You have to check $row, or check the length of result set.

Note: check the comments, your code has a lot of problems.

Karoly Horvath
  • 94,607
  • 11
  • 117
  • 176
0

$result is going to be a true value — even if no rows were returned — unless there was an error with the query. You need to count the number of rows to see if there were any matches.

(But see my comments on the question, you shouldn't be using that database API in the first place).

Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
0

As the others have mentioned, mysql_num_rows() is your answer here. One major thing here is that you're also using $result as an associative array when you should be using $row. I have rewritten your code:

<?php

session_start();

$name =  $_POST["name"];
$pass = $_POST["password"];

$conn = mysql_connect("localhost","tina","tinapassword") or die("Could not connect to MySQL server.");

if (!$conn) {
    echo "Unable to connect to DB: " . mysql_error();
    exit;
}

if (!mysql_select_db("tinadatabase")) {
    echo "Unable to select <strong>tinadatabase</strong>: " . mysql_error();
    exit;
}

$sql = "SELECT * FROM users WHERE Uname='$name'";

$result = mysql_query($sql);

if (!$result) {
    echo "Could not successfully run query (<strong>$sql<strong>) on DB: " . mysql_error();
    exit;
}

if (mysql_num_rows($result) == 0) {
    echo "Record not found.";
    exit;
} else
    echo "Record found."

$row = mysql_fetch_assoc($result);

print "<br>";

md5($pass);

if($name == $row["Uname"] && md5($pass) == $row["Upassword"]) {
    $_SESSION["authorized"] = 1;
    print "<a href='page3.php'>continue</a>";
} else {
    $_SESSION["authorized"] = 0;
    print "Username or password incorrect.";
}

mysql_free_result($result);

?>

Now, I have not tested it but it looks about right to me. Let me know if it works!

Gabriel Nahmias
  • 920
  • 3
  • 15
  • 20
0

I think the comments above are useful advice - look at using prepare command and bind variables. Apart from that though, You are referencing $result instead of $row.

I would also do the fetch after your check on $result. It is also good practice to use (!($result === false)) (i.e. test that it is not specifically false). I also think it is good practice to use !strcmp() to compare strings accurately. Having said all that, you could simplify things by doing a count on the matches for both username and password (hashed). Obviously this wouldn't be suitable if you need to select other details from the users table in your query. This method saves you having to do a check on num_rows etc.emphasized text

I don't use mysql as much as Oracle and Postgres so excuse me if the syntax is slightly wrong but.. assuming your UPassowrd is using the md5() hash of the password...

$hpass=md5($pass);
$query = "SELECT count(1) FROM users WHERE Uname='$name' and UPassword='$hpass'"; 
$result = mysql_query($query, $connect);

$_SESSION["authorized"] = 0;
if (!($result === false))
{
  $row = mysql_fetch_row($result);

  echo "Query Worked<br>";
  // count(1) result of anything other than 0 is a match - though any more than 1 might be an issue.
  $_SESSION['authorized']=((intval($row[0]) == 0)?0:1);
} else {
    echo "Query Failed<br>";
    exit;
}
echo "<br>";
echo"<a href='page3.php'> continue</a>";

Not sure whether your continue href to page3 is conditonal on the authorized value.

Arthur Nicoll
  • 391
  • 1
  • 2
  • 8
  • yes my page3.php is conditional on the authorized value and this is what it is it
    You must log in first"; } ?>now it says Passwords don't match,
    – Tina Woodbury Jul 20 '13 at 19:08