Some PHP Syntax help

[JOKER_JOKER]

I've been learning php for the last week or so, and it doesn't seem like a very complicated language, but the one thing that keeps tripping me up is where it is required/proper to use single quotes ( '.....' ) around a variable.

For example, I've been working on a simple login script, but it seems to make a big difference depending on whether I include variables/column names(from a table) in single quotes.

$user = $_POST['username'];
$pass = sha1($_POST['password']);

$query = mysql_query("SELECT * FROM users WHERE username = '$user' AND password = '$pass'");
$row = mysql_fetch_array($query);

if ($user = $row['username'] AND $pass = $row['password'])
echo "Username and Password accepted.";
else
echo "Username and Password combination invalid. Try again.";

Could someone give me the general rule of thumb for something like this? I've googled but can't seem to find any information very useful.

 

[surazal]

In mysql, all strings should be enclosed in single or double quotes. Username and password in the code above are strings, so they must have quotes around them.

In php, single quotes doesn't evaluate any variables. Double quotes will parse and evaluate variables in the string. I've read that it is better to use single quotes since it is faster, but I haven't done any tests myself. Use double quotes only when you require the evaluation. For more info on strings, http://us3.php.net/types.string

For example.

$myvar = 'world';

echo 'Hello $myvar';
echo "Hello $myvar";

The first echo will output Hello $myvar, the second will output Hello world

 

[tim_m]

all is explained here:
http://us2.php.net/manual/en/language.types.string.php

how you need to use quotes for a mysql query isn't dependent on php (rather on SQL), but you need to understand how php works in order to get what you want.

for SQL, you use single quotes when quoting things. take the raw query


SELECT * FROM users WHERE username = 'foo' AND password = 'bar';


if you were using the plain mysql command line client that is exactly what you would type. no surrounding quotes.

in order to use such a query from php you need to make it a string by enclosing it with quotes, double or single. if you use single you need to escape the single quotes inside via \' or else you'll get a parser error. in order to substitute a variable's value you are required to use double quotes.

if it makes it simpler for you, you can always use double quotes surrounding your php strings. do not confuse that with the use of single quotes within your sql query though as they are not really related. the only reason you would use single quotes to surround a php string would be if you don't need/want variable substitution and have literal double quotes in your string that you would otherwise need to escape or you're paranoid about performance and you use single quotes everywhere where you don't have variable substitution and think you'll see a performance difference (but it would presumably be unnoticeable in all but the largest scripts).

 

[tim_m]

on a separate optimization note, i would remove

$row = mysql_fetch_array($query);

and as the condition for your if statement use

if (mysql_num_rows($query) == 1)

also you probably want to do some error checking to make sure the query was successful as opposed to assuming it was successful and continuing with the script (demonstrated at http://us2.php.net/mysql-query)

 

[devman]

Unrelated to the topic but I couldn't help noticing this in your code.

$user = $_POST['username'];
$pass = sha1($_POST['password']);

Your passwords are being transmitted in plaintext then hashed by the server.
Passwords should be hashed (preferably with some salt) first then sent over the internet to the server.

If you don't care about that level of security (or you are using SSL) then just ignore this.

 

[ameoba]

...and you should be escaping/sanitizing any string that you take from user input and use in SQL. google://sql injection

 

[vbrtrmn]

HOLY CRAP, listen to ameoba!

Here's a hint, in your SQL:

$query = mysql_query("SELECT * FROM users WHERE username = '$user' AND password = '$pass'");

Let's say I passed the following as my username:

'; TRUNCATE users; --

Then your SQL would come out something like this:

SELECT * FROM users WHERE username = ''; TRUNCATE users; -- AND password = ''[code]

Read up on mysql_escape_string or you will be very sorry.
http://us2.php.net/manual/en/function.mysql-escape-string.php

 

[surazal]

I just noticed an even more glaring error

if ($user = $row['username'] AND $pass = $row['password'])

I assume you meant to use == for comparison and not = for assignment. Using === for typed comparison would be even better. Your current code will return true for any username and pass.

 

[JOKER_JOKER]

Thanks for your help and tips guys, especially with the mysql_real_escape_string(). But I've ran into another problem, and instead of starting a new thread, I'll just post it here. For some reason, when I try to log in, it always says the email/password combination is incorrect. I've been trying to find out whats wrong for the past 2 hours, can you guys figure out where I screwed up?

$email = $_POST[email];
$password = sha1($_POST[password]);

$query = mysql_query("SELECT * FROM users WHERE email='$email'");
$row = mysql_fetch_array($query);

if ($row[password] == '$password')
echo 'Email address and password accepted.<br><br>';
else
echo 'Email address and password combination invalid. Try again.<br><br>';

 

[surazal]

if ($row[password] == '$password')

This line is wrong, I'm surprised it didn't throw an error. Indexes to associative arrays are strings so it needs quotes. Using single quotes around $password will evaluate the right side to the string $password and not what $password is equal to.

Use this

if ($row['password'] === $password)

 

[JOKER_JOKER]

Nevermind, I figured it out. Thanks anyway.