elsif loop

Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

I am working on a logon script for a web applicaton. I am trying to create a clause whereby if the correct username and password are entered proceed to the next page, but if a wrong username is entered give an error, or if the wrong password is entered give an error, and as that hasn't seemed to suffice, if a correct username but wrong password is entered give and error and visa versa. Below is the code i have been testing, it works for all occurrances except where a correct username is entered with a wrong password or via versa, in this case i get an internal server error, instead of my own specified error.

#after reading in the values
if($username eq "" || $hashPassword == $null) {

        
        error();

}else{
#after comparing them to the actual passwords stored in
$sth = $db->prepare(q{SELECT userid, password from user where userid =
+ ?})
        or die("usr prepare error: $!");
$usr->execute($username)
        or die("usr prepare error: $!");


   while(@row  = $sth->fetchrow_array){
        if($username eq $row[0] && $hashPassword eq $row[1])    {

               #Go to the new page
       }elsif($username ne $row[0] || $hashPassword != $row[1]) {

                  error();

       }elsif($username eq $row[0] && $hashPassword != $row[1]) {

                  error();

       }elsif($username ne $row[0] && $hashPassword == $row[1]) {

                 error();

       }elsif($username ne $row[0] && $hashPassword != $row[1]) {

                 error();

}
}
[download]

Can anybody see what I am doing wrong? Thanks

Comment on elsif loop Download Code

Replies are listed 'Best First'.
Re: elsif loop by liverpole (Monsignor) on Dec 12, 2006 at 14:26 UTC
It looks like you're doing string comparisons, but you're not being consistent. Sometimes you use `eq`, and sometimes you use `==`. Try using `eq` for checking equality, and `ne` for checking inequality (instead of `==` and `!=`, respectively). Your use of `$null` is also a little suspicious; are you sure you don't mean `$hashPassword eq ""` or perhaps `!defined $hashPassword`? s''(q.S:$/9=(T1';s;(..)(..);$..=substr+crypt($1,$2),2,3;eg;print$..$/	[reply] [d/l] [select]
Re: elsif loop by jbert (Priest) on Dec 12, 2006 at 14:35 UTC
A few things. Is it fair to say that your desired logic is "if the username and password match, go to the new page, otherwise generate an error"? If so, you can simply write: `if($username eq $row[0] && $hashPassword eq $row[1]) { # Go to the new page } else { error(); }` [download] You seem to be mixing up `eq` and `==` in your hash tests. I would guess you want to treat the hashed password as a string, so you should use `eq` and `ne`. If you find yourself legitimately writing a long if/elsif cascade, it's generally a good idea to put an 'else' clause on the end, to catch any unexpected situations. If you don't think it can ever happen, feel free to make the contents `die "horribly"` or something, but if you have 4 or 5 tests, it's a fair bet you may have missed one. Lastly, your SQL query is going to guarantee that `$username eq $row[1]` is true, as long as some data is returned. Presumably you want to go to your error page if the number of rows returned is zero. You probably also want to have an error condition (perhaps a different one) if you get more than one row back from that query. So (assuming your error routine doesn't return): `my @rows = $sth->fetchrow_array; if (scalar @rows == 0) { # Unknown user error(); } if (scalar @rows > 1) { # DB in bad state - more than one record for user some_other_error(); } if ($hashPassword ne $rows[1]) { # wrong password error(); } # ...show the page...` [download] Lastly, in case you actually have different error cases in your different branches above, I'd counsel you not to do that. If someone can get a different error depending on whether they pass in a bad username or a bad password, that allows them to guess usernames. That weakens your security.	[reply] [d/l] [select]
Re: elsif loop by tinita (Parson) on Dec 12, 2006 at 14:46 UTC
first - you select records (in fact, very probably only one record that has the userid $username). i'm pretty sure you only have one row with that userid. second - then you go over that row(s) and compare $username to the fetched userid. don't you trust your database? for what reason do you do a 'where userid = ?' anyway? third - you're preparing $sth and execute $usr - a mistake? `my $sth = $db->prepare( q{SELECT userid, password from user where user +id = ?} ) or die $dbh->errstr; $sth->execute($username) or die $dbh->errstr; my ($uid, $pass) = $sth->fetchrow_array; if ($pass ne $hashPassword) { error(); }` [download]	[reply] [d/l]
Re^2: elsif loop by Anonymous Monk on Dec 12, 2006 at 15:55 UTC
Thanks for all the comments. I have considered each of them. I don't think i have access to the server logs for the server i am working on, so i can't check the exact perl error when i get an internal server error. Where should these logs be? When i applied jberts first example example all cases work fine except when an incorrect username and password are entered. In that case i get an internal server error. `if($username eq $row[0] && $hashPassword eq $row[1]) { # Go to the new page } else { error(); }` [download] Using the second example i am given access to the next page with the correct user name and an incorrect password. Does anyone know why this is happening?	[reply] [d/l]
Re^3: elsif loop by nimdokk (Vicar) on Dec 12, 2006 at 17:26 UTC
Just a suggestion for troubleshooting. Especially since you may not have access to server logs. Try taking the logic you are using and putting it into a script you can run from the commandline to make sure the logic works the way you expect before putting it into the CGI script. Once you get that working (and know it works their), then put it back in and test it with the CGI app. I've often found simplifying a script will help get to the root of the problem and a fix is fairly apparent at that point. Adding `use strict` and `use warnings` in the test script will also help. Hope that helps.	[reply] [d/l] [select]
Re: elsif loop by blue_cowdawg (Monsignor) on Dec 12, 2006 at 14:26 UTC
in this case i get an internal server error, instead of my own specified error. Internal server error is a web server's way of telling you your code has exploded like the penguin on top of your television. My suggestion would be to check the server logs for the real Perl error that is being emitted and troubleshoot off of that. Peter L. Berghold -- Unix Professional Peter -at- Berghold -dot- Net; AOL IM redcowdawg Yahoo IM: blue_cowdawg	[reply]


"be consistent"
	PerlMonks