What the..this should be so easy

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
influx
Forum Commoner
Posts: 31
Joined: Fri Aug 05, 2005 9:28 am

What the..this should be so easy

Post by influx »

I have the following code in profile_add.php.

Code: Select all

$code=$_GET['c'];

//check and see if propagated code is between 1-7 to prevent anything fishy
if($code && ($code!=1 || $code!=2 || $code!=3 || $code!=4 || $code!=5 || $code!=6 || $code!=7))
{
   die('Invalid code. Please contact us if this problem persists.');
}
When I access this page as "profile_add.php?c=7" it is constantly executing the die() function!

How come?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

your logic says: if $code is not 1, or $code is not 2, or $code is not 3, etc.. and if $code is non-zero.

you want ands. Switch || to &&
User avatar
nielsene
DevNet Resident
Posts: 1834
Joined: Fri Aug 16, 2002 8:57 am
Location: Watertown, MA

Post by nielsene »

You have effectively written

Code: Select all

if ($code!="" && (TRUE))
remember that || is short circuited. So

Code: Select all

$code!=1 || $code!=2
will always be TRUE. (if $code=1, then the second term is TRUE, if $code!=1 then the first term is TRUE)

You probably want something like

Code: Select all

if ($code && !in_array($code,array(1,2,3,4,5,6,7))
influx
Forum Commoner
Posts: 31
Joined: Fri Aug 05, 2005 9:28 am

Post by influx »

Alright, that makes sense...

However, on every other one of my pages where I execute the code after accepting a form (via the POST method), I use the || statements when checking the code, and it works fine.

Take for example this one for id_add.php:

Code: Select all

if($code && ($code!=1 || $code!=2 || $code!=3 || $code!=4 || $code!=5 || $code!=6 || !is_int($user_id)))
{
	die('Invalid code/User ID. Please contact us if this problem persists.');
}
What could be allowing this to pull through?
User avatar
nielsene
DevNet Resident
Posts: 1834
Joined: Fri Aug 16, 2002 8:57 am
Location: Watertown, MA

Post by nielsene »

The only possible way for that to succeed would be for $code to be 0 so that the initial term evaluates to false and short-circuits the &&. If you've forgotten to assign $code from $_POST["code"] that could also explain it as the undefined variable will default to false...

Its almost never correct to have

Code: Select all

if ($x!=1 || $x!=2)
// handle negative
as that will reduce to TRUE in all cases

Normally, you want to either do:

Code: Select all

if ($x==1 || $x==2)
 // handle positive
else
 // handle negative
or

Code: Select all

if (!($x==1 || $x==2))
// handle negative
else
//handle positive
Using DeMorgan's on the last example would yield

Code: Select all

if ($x!=1 && $x!=2)
// handle negative
else
// handle positive
(which matches what feyd was suggesting.)
influx
Forum Commoner
Posts: 31
Joined: Fri Aug 05, 2005 9:28 am

Post by influx »

thanks a lot guys, very helpful...I'm looking into it
Post Reply