Very simple function question

cemlouis

Hello, I wrote the below code it is so simple the output of the test.php should be "not blocked" but getting "blocked" instead. Why?

Thank you,

<?php 
/********** blockedsites.php ************/ 
$blockedlist = array("theothersite.com","othersite.com", "hissite.com"); 
?>

<?php 
/********** test.php ************/ 
include "blockedsites.php"; 

function isblocked($postedurl){ 
    for($i=0; $i<count($blockedlist); $i++){ 
        if (strpos($postedurl, $blockedlist[$i]) === true) { 

        $blocked = true; 
        break; 
    } else { 
        $blocked = false; 
    } 
} 
return $blocked; 
} 

$postedurl = "www.mysite.com"; 


if (isblocked($postedurl) === false){ 
    echo "not blocked"; 
} 
else 
{ 
    echo "blocked"; 
} 

?>

NogDog

$blockedlist is not within the scope of function isblocked().

Solution #1 (my preferred method):

function isblocked($postedurl, $blockedlist){ 
// ...function definition...
}
// ...
if (isblocked($postedurl, $blockedlist) === false){

Solution 2 (making the array variable global within the function):

function isblocked($postedurl) {
   global $blockedlist);
// ...
}

Piranha

First up in the first if-statement you use ===. I think it should be == since it will not really return true. That is probably the error that blocks all pages.

Usually when I work with booleans I set it to one value in the beginning and only change it if it should be changed. The reason is that it is easy to make a mistake otherwise, like you have done. Let's say that we input "othersite.com", the second blocked site and go through the code.

Function isblocked(othersite.com) is called
The loop is called, $i = 1
if-statement is false
$blocked = false
the loop is called, $i = 2
if-statement is true
$blocked = true
the loop is called, $i = 3
if-statement is false
$blocked = false
end of loop
false is returned, but you want true to be returned.

To avoid this problem set $blocked=false before the loop starts. Then only change it if $blocked should be true.

Edit: NogDog is correct, my first paragraph might be way off. But the rest still applies.

cemlouis

Thank you both, I applied the ethics to the code but it is now always giving "not blocked" for postedurl: www.hissite.com

The modified code as belows:

<?php
/********** test.php ************/
include "blockedsites.php";

function isblocked($postedurl){
	global $blockedlist;
	$blocked = false;
	for($i=0; $i<count($blockedlist); $i++){
		if (strpos($postedurl, $blockedlist[$i]) === true) {
			$blocked = true;
		}
	}
	return $blocked;
}

$postedurl = "www.hissite.com";


if (isblocked($postedurl) === false){
    echo "not blocked";
}
else
{
	echo "blocked";
}

?>

bradgrafelman

Probably because [man]strpos/man doesn't return true if it finds a string.

Roger_Ramjet

What about using in_array() instead of some cumbersome loop construct === its what array functions exist for.

bradgrafelman

Probably because using [man]strpos[/man] would allow "hissite.com" in the block list be applied to "www.hissite.com" and so on. [man]in_array/man would simply search for a literally "hissite.com".

Now if you strip off everything except the domain name of the site, you could then use [man]in_array/man against an array of domain names.

Infestor1

You guys rock!