• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 273
  • Last Modified:

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

0
LB1234
Asked:
LB1234
  • 4
  • 3
  • 3
  • +2
10 Solutions
 
Ess KayEntrapenuerCommented:
change

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


to
 if (strlen($username || $password )< $max)
0
 
Lukasz ChmielewskiCommented:
Line 18 is
} if (strlen($username || $password < $max)) {
and should be
} if (strlen($username) < $max || strlen($password) < $max) {
0
 
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

0
Cloud Class® Course: C++ 11 Fundamentals

This course will introduce you to C++ 11 and teach you about syntax fundamentals.

 
LB1234Author Commented:
Thanks people.  Can someone explain why mine is incorrect?
0
 
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)) {
0
 
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
0
 
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.
0
 
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
0
 
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
0
 
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

0
 
LB1234Author Commented:
Ray that was really awesome!  Wow, much better way!
0
 
LB1234Author Commented:
also I'm using several "if" statements.  Is this bad form? Should i be using if elseif and else statements?
0
 
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.
0
 
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
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

Get expert help—faster!

Need expert help—fast? Use the Help Bell for personalized assistance getting answers to your important questions.

  • 4
  • 3
  • 3
  • +2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now