whats happened here?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
User avatar
sandersonlj1529
Forum Newbie
Posts: 7
Joined: Fri Aug 07, 2009 7:45 am

whats happened here?

Post 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";
        }
 
 
User avatar
sandersonlj1529
Forum Newbie
Posts: 7
Joined: Fri Aug 07, 2009 7:45 am

Re: whats happened here?

Post 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?
cpetercarter
Forum Contributor
Posts: 474
Joined: Sat Jul 25, 2009 2:00 am

Re: whats happened here?

Post 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.
User avatar
sandersonlj1529
Forum Newbie
Posts: 7
Joined: Fri Aug 07, 2009 7:45 am

Re: whats happened here?

Post 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)
User avatar
califdon
Jack of Zircons
Posts: 4484
Joined: Thu Nov 09, 2006 8:30 pm
Location: California, USA

Re: whats happened here?

Post 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.
cpetercarter
Forum Contributor
Posts: 474
Joined: Sat Jul 25, 2009 2:00 am

Re: whats happened here?

Post 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.
User avatar
sandersonlj1529
Forum Newbie
Posts: 7
Joined: Fri Aug 07, 2009 7:45 am

Re: whats happened here?

Post 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:
Post Reply