Page 1 of 1

whats happened here?

Posted: Fri Aug 07, 2009 3:52 pm
by sandersonlj1529
Hi, I know this code is quite messy but I am very new to php (please suggest how to make this cleaner)

i have made a simple login form and I have these 2 functions to deal with the log in process. the clean_input seems to work with the username but not the password and the validate_user although it will log the user in will no longer return "you have entered a incorrect username or password"; why is this?

Code: Select all

 
 
function clean_input($username, $password) { 
        if(isset($_POST['submit'])) {
            if(preg_match('/^([A-Za-z1-9_-]+)$/', $username,$password)) {
                $this->validate_user($username,$password); 
                } else return "you can only use letters,numbers _ and -";   
            }
     }
 
function validate_user($username, $password) {
            $query="SELECT * FROM users WHERE username='$username' OR email='$username' AND password= md5('$password')";
            $result=mysql_query($query);
            
            if(mysql_num_rows($result)>0) {
                $_SESSION['status']= 'authorized';
                header("location: index.php");
            } else return "you have entered a incorrect username or password";
        }
 
 

Re: whats happened here?

Posted: Fri Aug 07, 2009 4:14 pm
by sandersonlj1529
dont worry think I have fixed it by making it just one function not sure why I didn't just do it this way in the first place

Code: Select all

    
function validate_user($username, $password) {
            if(isset($_POST['submit'])) {
            if(preg_match('/^([A-Za-z1-9_-]+)$/', $username,$password)) {
            $query="SELECT * FROM users WHERE username='$username' OR email='$username' AND password= md5('$password')";
            $result=mysql_query($query);
              if(mysql_num_rows($result)>0) {
                $_SESSION['status']= 'authorized';
                header("location: index.php");
              } else return "you have entered a incorrect username or password";
             } else return "you can only enter letters, numbers _ and -";  
            }
        }
 
look better?

Re: whats happened here?

Posted: Fri Aug 07, 2009 4:16 pm
by cpetercarter
Have a look at the php manual section on preg_match.. If you have two strings to test - $username and $password - you can't do this just by placing them as the second and third arguments in the preg_match function. You could in this case concatenate them (ie place a full stop between them) since you are testing whether they contain only permitted characters. But it would be clearer to test them separately:

Code: Select all

$regex = '/^([A-Za-z1-9_-]+)$/';
if preg_match($regex, $username) && preg_match($regex, $password) {
...do something
 
In your second function, I guess you mean

[sql]WHERE (username = '$username' OR email = '$username') AND password= 'md5($password)'[/sql]

The brackets are important.

Re: whats happened here?

Posted: Fri Aug 07, 2009 4:21 pm
by sandersonlj1529
cpetercarter wrote:Have a look at the php manual section on preg_match.. If you have two strings to test - $username and $password - you can't do this just by placing them as the second and third arguments in the preg_match function. You could in this case concatenate them (ie place a full stop between them) since you are testing whether they contain only permitted characters. But it would be clearer to test them separately:

Code: Select all

$regex = '/^([A-Za-z1-9_-]+)$/';
if preg_match($regex, $username) && preg_match($regex, $password) {
...do something
 
Thanks I will look in to this I have only just been looking at regular expressions and don't fully understand them so it was just a guess..

EDIT: i have had to use this:
if(preg_match('/^([A-Za-z1-9_-]+)$/', $username) && preg_match('/^([A-Za-z1-9_-]+)$/', $password)) { do something
if I use your code I get an error

In your second function, I guess you mean

[sql]WHERE (username = '$username' OR email = '$username') AND password= 'md5($password)'[/sql]

The brackets are important.
why are these brackets so important? (i can't find any information about the OR function)

Re: whats happened here?

Posted: Fri Aug 07, 2009 6:15 pm
by califdon
Your original syntax with respect to the md5() function was correct. The issue of parentheses (not brackets) with AND and OR is covered in this link: http://www.databasedev.co.uk/sql-multip ... tions.html. It is indeed important, not only in SQL but anyplace that you are expressing boolean logic involving both AND and OR operators, because without an indication of the grouping, ambiguous interpretations may be possible. What does it mean to say that you will pay $20 for a shirt if it is a Large size AND it has long sleeves OR it is blue? That could logically mean either:
(1) You want a Large shirt that is either blue or has long sleeves;
or it could mean:
(2) You want a shirt that is EITHER Large and has long sleeves OR it is blue.

Re: whats happened here?

Posted: Sat Aug 08, 2009 2:02 am
by cpetercarter
Is the original syntax of the md5() function correct? I think the op wants a user to be able to log in with either a username or an e-mail address, but in both cases to test whether the password is correct.

The AND operator is evaluated before the OR operator. So the original form of the query means:
[sql] WHERE username ='$username' OR email = '$email' AND password = '$password' [/sql]
In other words, the password is tested only in the case where a user submits an email address, not where she/he submits a username.

The brackets are needed to force MySQL to evaluate the query as :

[sql]  WHERE username = '$username' OR email = '$email' AND password = '$password' [/sql]

On the regex function, sorry my mistake. There should of course be brackets around the 'if' statement.

Re: whats happened here?

Posted: Sat Aug 08, 2009 4:52 am
by sandersonlj1529
Thank you both I understand what I did no regarding the brackets in the SQL . funny thing was; I was changing my regex to allow the @ symbol but It wasn't working with my e-mail address. It took me a good 15mins to realise I had not allowed the period. :lol: