PHP Simple if statement not executing properly

In the following code, all entries result in ""entry must be at least 6 characters" no matter what's entered.  Not sure why this is happening.  Note: value of $max ($max = 6) is in validations file.  Any ideas?  Thanks.

<?php

include ("functions/functions.php");
include ("functions/validations.php");


if (isset($_POST["submit"])) {

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

	if (empty($username) || empty($password)) {
		$message = "fields are empty";
	
		} if (!is_string($username) || ($password)) {
			$message = "these cannot be numbers";
		
		} if (strlen($username || $password < $max)) {
			$message = "entry must be at least 6 characters";
		
		} else {
			redirect("welcome.php");
	}


} else {

$message = "POST NOT SET";
$username = "";
$password = "";

}


?>




<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Untitled Document</title>
</head>

<body>



<?php echo $message ?>
<form action="test.php" method="post">


Enter user name: <input name="username" type="text" value="<?php echo htmlspecialchars($username) ?>"> <br>
Enter password: <input name="password" type="password" value="<?php echo htmlspecialchars($password) ?>"> <br>
<input name="submit" type="submit" id="submit"><br>

</form>





</body>
</html>

Open in new window

LVL 1
LB1234Asked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Ess KayEntrapenuerCommented:
change

if (strlen($username || $password < $max))


to
 if (strlen($username || $password )< $max)
Lukasz ChmielewskiCommented:
Line 18 is
} if (strlen($username || $password < $max)) {
and should be
} if (strlen($username) < $max || strlen($password) < $max) {
Lukasz ChmielewskiCommented:
This won't work as expected, because you need to apply the function to all posted variables,
the whole if/else should look like this:

if (empty($username) || empty($password)) {
	$message = "fields are empty";
} 
if (!is_string($username) || !is_string($password)) {
	$message = "these cannot be numbers";
} 
if (strlen($username) < $max || strlen($password) < $max)) {
   // or even better: if(strlen($username . $password) < 12)
	$message = "entry must be at least 6 characters";
} 
else 
{
	redirect("welcome.php");
}

Open in new window

OWASP Proactive Controls

Learn the most important control and control categories that every architect and developer should include in their projects.

LB1234Author Commented:
Thanks people.  Can someone explain why mine is incorrect?
Marco GasiFreelancerCommented:
Also you can't set the condition once for several variables:

Line 15:

if (!is_string($username) || ($password)) {

must be

if (!is_string($username) || !is_string($password)) {

exactly as you did in line 12: if (empty($username) || empty($password)) {
Lukasz ChmielewskiCommented:
For example:

$username = "aaaaaaa";
$password = "zzzzzzz";

if (!is_string($username) || ($password)){
      echo "username is not string, password is not a string";
}
else{
      echo "these are strings allright...";
}

these are strings, but the IF goes into "username is not (...)"
because you do not apply the function call to every argument. The $password is converted to value of 1 because you check literally:

if( BOOLEAN (is the username a string) OR BOOLEAN password
the BOOL password is converted to 1, then
anything || 1 equals true
Ess KayEntrapenuerCommented:
the reason is, the computer is only as smart as you make it. you need to explain each function to it individually.

IIE
if (!is_string($username) || ($password)) {

must be

if (!is_string($username) || !is_string($password)) {


because it only processes one function at a time

is_String is a function, which can only take one arguement, due to the syntax

I'm not sure why the rest of it worked, but it shouldn't. i havent corrected it in my answer because if it aint broke, don't fix it.

But you might want to reevaluate if the first statements also work correctly, because they will most likely not do so.
Ray PaseurCommented:
You've got good advice from the other answers (evaluate each field separately), but I find this very interesting.  I am kind of surprised this even parsed correctly.  Maybe that is an artifact of PHP loose typing?  I can't figure out how PHP is evaluating this statement.  Even with error_reporting(E_ALL) set, no notice is raised by having $max be undefined!  That suggests to me that PHP is looking at strlen($username) alone and determining that to be "truthy."  As soon as a true condition appears in the evaluation, the entire statement would be treated as true unless some "and" condition was injected.

if (strlen($username || $password < $max))

http://www.php.net/manual/en/types.comparisons.php
http://www.php.net/manual/en/language.types.type-juggling.php
Ray PaseurCommented:
Here is something along the lines of how I might start doing this.  It's not perfect, of course, and there would be a lot more tests you might want to apply.  For example, if you're going to use this with a data base, you would want to be aware of case-sensitivity and the risk of collisions between, eg, northSide and Northside.  You might want to check to see if there were any "illegal" characters in the strings, or that the strings are less than a maximum length, etc.
http://www.laprbass.com/RAY_temp_lb1234.php

<?php // RAY_temp_lb1234.php
error_reporting(E_ALL);

// SEE http://www.experts-exchange.com/Web_Development/Web_Languages-Standards/PHP/Q_28324610.html

// INITIALIZE OUR VARIABLES
$u = $p = NULL;

// IF THE FORM HAS BEEN POSTED
if (!empty($_POST))
{
    // IF THERE ARE ERRORS WE WILL KEEP THEM HERE
    $err = array();

    // USE TERNARY OPERATOR TO ACQUIRE DATA FROM THE POST REQUEST
    $u = !empty($_POST['username']) ? trim($_POST['username']) : NULL;
    $p = !empty($_POST['password']) ? trim($_POST['password']) : NULL;

    // VALIDATE THE USERNAME ACCORDING TO SOME RULES
    if (empty($u))      $err[] = 'USERNAME IS EMPTY';
    if (strlen($u) < 6) $err[] = 'USERNAME IS LESS THAN 6 CHARACTERS';
    if (is_numeric($u)) $err[] = 'USERNAME IS NUMERIC';

    // VALIDATE THE PASSWORD ACCORDING TO SOME RULES
    if (empty($p))      $err[] = 'PASSWORD IS EMPTY';
    if (strlen($p) < 6) $err[] = 'PASSWORD IS LESS THAN 6 CHARACTERS';
    if (is_numeric($p)) $err[] = 'PASSWORD IS NUMERIC';

    // IF THERE ARE ANY ERRORS
    if (empty($err))
    {
        echo "SUCCESS!";
    }
    else
    {
        print_r($err);
        echo "TRY AGAIN.";
    }
}

// CREATE THE FORM WITH HEREDOC NOTATION
$form = <<<EOD
<form method="post">
Username: <input name="username" value="$u" />
<br>
Password: <input name="password" value="$p" />
<br>
<input type="submit" />
</form>
EOD;

echo $form;

Open in new window

HTH, ~Ray

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
LB1234Author Commented:
With the following i get.  line 18 below refers to the strlen statement.

Parse error: syntax error, unexpected T_BOOLEAN_OR in C:\wamp\www\PHP web development\test.php on line 18


	if (empty($username) || empty($password)) {
		$message = "fields are empty";
	
		} if (!is_string($username) || !is_string($password)) {
			$message = "these cannot be numbers";
		
		} if (strlen($username) < $max) || strlen($password) < $max)) {
			$message = "entry must be at least 6 characters";
		
		} else {
			redirect("welcome.php");
	}

Open in new window

LB1234Author Commented:
Ray that was really awesome!  Wow, much better way!
LB1234Author Commented:
also I'm using several "if" statements.  Is this bad form? Should i be using if elseif and else statements?
Ray PaseurCommented:
Should i be using if elseif and else statements?
I guess that depends on the flow of logic you want to create.  If you wanted to test for a cascade of errors, for example, you might do that.  It might be a way to make the error messages succinct and useful.  But there is nothing wrong with several if() statements, so long as you get the results you want from your tests.
Ess KayEntrapenuerCommented:
it really does matter on what you are trying to accomplish. ifelse, is like a SELECT statement, while regular if's will go through each IF statement regardless of the previous outcome. in your case, if you want to hault at the first error, use ifelse. if you want to show all errors at once use regular IF()



ie
DIM a = 0
DIM b = 0
Dim c = 0


IF true then a = 1
IF true then b = 1
IF true then c = 1

Open in new window



result will be 1,1,1



IF true then a = 1
elseIF true then b = 1
elseIF true then c = 1
end if 

Open in new window


result will be 1,0,0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
PHP

From novice to tech pro — start learning today.