Flawed GET/Cookie logic, need clarification...

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
JAB Creations
DevNet Resident
Posts: 2341
Joined: Thu Jan 13, 2005 6:44 pm
Location: Sarasota Florida
Contact:

Flawed GET/Cookie logic, need clarification...

Post by JAB Creations »

My PHP logic arguments are flawed and I'm looking for clarification on what I'm screwing up here...

$_GET['audio'] has precedence over $_GET['audio'] unless there is no $_GET['audio']...I sort of have this functioning though I could definitely do a better job of clarifying it. I also know this code (even when we get it working) will not be anywhere close to ideal PHP code though I'm just trying to get the logic/script to work correctly. Audio values are 0 for no audio, 1 for dialup, and 2 for broadband. This script is exclusive to only determining if a broadband audio link should be provided or not based on the visitor's preference. Also I'm providing some additional code to help with testing below...

Code: Select all

<?php

 if (isset($_GET['audio'],$_COOKIE['audio']) && $_GET['audio']===0 || $_COOKIE['audio']==0) {?>

<span class="neutral">Hi-Fi Disabled</span>

<?php }
 else if (isset($_GET['audio'],$_COOKIE['audio']) && $_GET['audio']===1 || $_COOKIE['audio']==1) {?>

<span class="neutral">Hi-Fi Disabled</span>

<? }
 else if ($_GET['audio']===2 || !isset($_GET['audio']) && $_COOKIE['audio']==2) { ?>

<a class="icon music" href="javascript:if (location== top.location) {alert ('Audio Locked, Frame Unavailable!')}" onmousedown="if (location!= top.location) {parent.mplayer.location.href='../mplayer/jab-creations-2.0-01-hi.php';} else {return false;}" rel="mplayer" title="Play the high quality preview of this song.">Hi-Fi Play</a>

<?php } ?>
header.php example

Code: Select all

if ($_GET['audio'] == "0") {setcookie('audio','0',time()+2592000,'/');}
else if ($_GET['audio'] == "1") {setcookie('audio','1',time()+2592000,'/');}
else if ($_GET['audio'] == "2") {setcookie('audio','2',time()+2592000,'/');}
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

Code: Select all

$_GET['audio']===0
everything in $_GET/$_POST/$_COOKIE is string, so it couldn't be strictly equal to integer zero
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

i'd rewrite it as follows:

Code: Select all

$allowed = array(0, 1, 2);
if (isset($_GET['audio']) && strlen($_GET['audio'])) {
   $audio = (int) $_GET['audio'];
} else if (isset($_COOKIE['audio']) && strlen($_COOKIE['audio'])) {
   $audio = (int) $_COOKIE['audio'];
} else {
   $audio = 0;
}

if (!in_array($audio, $allowed)) {
   $audio = 0;
}

setcookie('audio', $audio, time()+2592000, '/');

switch ($audio) {
   case 0:
       // sound disabled
   break;
   case 1:
       // lo-fi
   break;
   case 2:
       // hi-fi
   break;
}
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

Another suggestion:

Code: Select all

$audio = !empty($_GET['audio']) ? intval($_GET['audio']) : (!empty($_COOKIE['audio']) ? intval($_COOKIE['audio']) : 0);
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

Another suggestion:

Code: Select all

$_GET['audio'] = '0';
var_dump(!empty($_GET['audio'])); // == false => your logic flawed
and what about $_GET['audio'] = '999' ? You aren't checking if it's in range of allowed values.
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

Weirdan wrote:
Another suggestion:

Code: Select all

$_GET['audio'] = '0';
var_dump(!empty($_GET['audio'])); // == false => your logic flawed
Yeah, should be is_set() instead of !empty()
Weirdan wrote:and what about $_GET['audio'] = '999' ? You aren't checking if it's in range of allowed values.
I'd follow your checking/case method :) I've rewritten only the if/else blocks for GET/COOKIE precedence.
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
JAB Creations
DevNet Resident
Posts: 2341
Joined: Thu Jan 13, 2005 6:44 pm
Location: Sarasota Florida
Contact:

Post by JAB Creations »

My PHP starts and ends frequently on the pages I want to redo the script for...is there a way to make a global for only this page without the security implications or somehow make this code jump between the frequent opening/closing of PHP syntax? I would imagine the manner I'm approaching this is to some extent memory intensive? :?
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

My PHP starts and ends frequently on the pages I want to redo the script for...is there a way to make a global for only this page without the security implications
do you mean something like this?:

Code: Select all

<?php
$allowed = array(0, 1, 2);
if (isset($_GET['audio']) && strlen($_GET['audio'])) {
   $audio = (int) $_GET['audio'];
} else if (isset($_COOKIE['audio']) && strlen($_COOKIE['audio'])) {
   $audio = (int) $_COOKIE['audio'];
} else {
   $audio = 0;
}

if (!in_array($audio, $allowed)) {
   $audio = 0;
}

setcookie('audio', $audio, time()+2592000, '/');
?>
some html heere
<?php switch ($audio) :
         case 0: ?>
sound is disabled, sorry
<?php    break;
         case 1: ?>
lo-fi version
<?php    break;
         case 2: ?>
hi-fi version
<?php    break; 
      endswitch; ?>

You have selected <?php echo $audio; ?> as your audio quality

more html....

<?php if ($audio > 0) : ?>
turn on your speakers, please
<?php endif; ?>
User avatar
JAB Creations
DevNet Resident
Posts: 2341
Joined: Thu Jan 13, 2005 6:44 pm
Location: Sarasota Florida
Contact:

Post by JAB Creations »

Thank you, that works great, though could you please explain the fundamental points of how you're able to skip between PHP opening and closing throughout the page? For example I am noticing the usage of (int) though I haven't seen this before? I assume this makes the variable available as a page-only global in some sense?
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

Thank you, that works great, though could you please explain the fundamental points of how you're able to skip between PHP opening and closing throughout the page?
PHP has two scopes (until namespaces get introduced): local and global. Variables in the local scope are those you define in a function, and they are available inside the function they were defined only. Variables defined outside any function are in the global scope, and they are available everywhere except where local scope is in effect (that means global variables are invisible inside a function unless you import them into the function scope using 'global' keyword).

File boundaries are not considered when evaluating visibility, nor are considered opening/closing tags. Thus you may define your variable in one code block, close php tag, start another code block with opening php tag and have your variable visible there without any additional efforts.
For example I am noticing the usage of (int) though I haven't seen this before? I assume this makes the variable available as a page-only global in some sense?
No, it's typecasting. Basically it means 'whatever that was, make me integer out of that'.
Post Reply