-5

I am following a video on YouTube on how to add a log in form on to a website, https://www.youtube.com/watch?v=4oSCuEtxRK8

At 8:37, when he clicks on username or password, nothing happens, no error message but my error message comes up. Here is my code

<?php

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

if ($username&&$password)
{

$connect = mysql_connect("localhost","root","root") or die("Couldn't connect!";
mysql_select_db("phplogin") or die ("Couldn't find db");

$query = mysql_query("SELECT * FROM "); users WHERE username='$username'");

$numrows = mysqal_num_rows($query);

echo $numrows;

}
else
    die("Please enter username and a password!");
?>

Error message I get is 

Parse error: syntax error, unexpected ';' in C:\xampp\htdocs\Hamza\login.php on line 9

I used localhost:8080 instead of just localhost

http://localhost:8080/Hamza/login.php

Would this be a problem?

h4mza
  • 9
  • 1

2 Answers2

2

I'm not sure even a professional cleaning firm could clean up this mess :) See updated code below, you have a couple of typo's which has been commented and replaced with a new line.

When learning from Youtube, pause sometimes and go over your code, try to understand how it works. Seems like you're just rushing to copy all of it and never even looks at the code.

Also read the error messages, they're there for a reason.

Parse error: syntax error, unexpected ';' in C:\xampp\htdocs\Hamza\login.php on line 9

Unexpected ';' on line 9 - take a look at line 9, you've forgot to close the die() bracket, so the ; was unexpected.

<?php

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

if ($username&&$password)
{

//$connect = mysql_connect("localhost","root","root") or die("Couldn't connect!";
$connect = mysql_connect("localhost","root","root")or die("Couldn't connect!");
mysql_select_db("phplogin") or die ("Couldn't find db");

//$query = mysql_query("SELECT * FROM "); users WHERE username='$username'");
$query = mysql_query("SELECT * FROM users WHERE username='$username'");

//$numrows = mysqal_num_rows($query);
$numrows = mysql_num_rows($query);

echo $numrows;

}
else
    die("Please enter username and a password!");
?>

A few (very important) side notes, which you must learn sooner or later:

  • Passwords should not be stored as plain text in your database (or any other place for that matter). If using PHP 5.5.0 or later you can use the php function password_hash() which creates a very secure hash. You can then use password_verify() for the login function. See http://php.net/manual/en/function.password-hash.php for more info.

  • Older hashing algorithms like md5 and sha1 should not be used for storing passwords. They are unsafe and can easily be cracked. If you can't use password_verify(), please also read up on salts. Salts can make the hashes even more secure by adding a unique salt to every hashed password.

  • Your current code only does a database query for the username. Needless to say, you should also check submitted password before logging in the user :)

  • Input sanitizing. It's important to sanitize all user input to avoid security breaches. Think of all users as hackers who are constantly attacking your website. Read the answer here for a few quick tips, but I recommend you to google and read more about input sanitizing.

  • Don't use mysql_* functions. You'd be better of starting right at mysqli (syntax is almost identical to mysql) or PDO. Read up on them and learn them, as you will need to at some point. (mysql_* is deprecated and will stop working in a future PHP version)

  • Both MySQLi and PDO supports prepared statements. It basically means the database is preparing and optimizing the query in advance, then binding all values into the query right before execution. This will also avoid SQL injections to some degree because the query is already prepared by the time you're binding the values (which may be user input) to the query.

Community
  • 1
  • 1
KEK
  • 473
  • 3
  • 15
  • *"I'm not sure even a professional cleaning firm could clean up this mess"* - lol good one ;) – Funk Forty Niner Dec 13 '14 at 18:57
  • Add the fact that the OP is storing plain text passwords (and shouldn't be), along with not sanitizing inputs, and should be using prepared statements and links to `password_hash()` and I'll upvote ;) – Funk Forty Niner Dec 13 '14 at 18:59
  • Haha, glad you liked the comment. I figured I should give him some slack at first, but you're right; the post has been updated with a few tips. Thanks for feedback fred -ii- :) Let me know if there's anything crucial I forgot/should add – KEK Dec 13 '14 at 19:34
  • I'd say that covers pretty well everything. +1, I'm a man of my words ;) – Funk Forty Niner Dec 13 '14 at 19:35
  • Thank you, much appreciated! Added a warning about md5/sha1 hashes as well, which are still regularly used unfortunately :) – KEK Dec 13 '14 at 19:40
  • You're welcome. Yeah, both of those were good and served a purpose at some point. However, they can still be used for other things besides password storage, so it's not a total fiasco. Let's just hope that the "question of time", won't apply anytime soon for `password_hash()`. – Funk Forty Niner Dec 13 '14 at 19:42
  • You're absolutely right, they are often quick and efficient for many things. My comment was a bit misleading, I meant I still see it used for password hashes which I consider bad in 2014 :) – KEK Dec 13 '14 at 19:48
  • No, it's ok; I understood what you meant. Yeah, some still do and it's probably due to the fact that they may not know about its vulnerabilities (till it's too late). Even I get surprised at the stuff I learn about here on Stack and other forums. – Funk Forty Niner Dec 13 '14 at 19:50
0

The query needs to be corrected:

$query = mysql_query("SELECT * FROM users WHERE username='$username'");

That was pretty basic. Understand you are new so You need to do your code review more carefully look at the error logs more closely.

nitigyan
  • 484
  • 2
  • 5
  • 18