Skip navigation.
Home

When a function just won't do...

Hall of Fame | Bad Architecture

Thanks to Michael for sending this in! He was doing a code audit on a PHP application developed by a contractor. Our story starts with an include file named magdit.inc, which is Dutch for "isthisallowed".

This example makes it into the WTF Hall of Fame because it violates so many good design principles. Take a look at magdit.inc to get you started...

includes/magdit.inc
<?php
$cyfer
= $magdit;
$magdit = "";

// hier testen we de list functions.. het blijkt makkelijk een list om te zetten in een array en er dan mee te werken dusse
$variablelist = $access;
eval(
"\$arraytest = array($variablelist);");
foreach(
$arraytest as $loopje) {
if (
$cyfer == $loopje) {
$magdit = "ja";
}
}
if (
$magdit == null) {
$magdit = "nee";
}
?>

Can you spot the multiple WTF's in magdit.php? The most blatent ones are the use of eval() (why?!), and at the end of the file where $magdit is either "ja" (yes) or "nee" (no). I bet you're wondering what's the point of this code if it just ends?! There's also another WTF in there that's a doozy. Can you spot it? Read on, it gets worse...

left_frame.php
<?php
/**** Comments are editor's ****/

$magdit = 6; // WTF does 6 mean?!
include 'includes/magdit.inc'; // ahh here it is.
$magdit2 = $magdit; // $magdit (integer) goes in, $magdit (string) comes out
$magdit = 7;
include
'includes/magdit.inc'; // here it is again...
$magdit3 = $magdit;
$magdit = 12;
include
'includes/magdit.inc'; // and again...
if ($magdit == "ja" or $magdit2 == "ja" or $magdit3 == "ja") {
echo
"<tr><td><a href=users.php target=mainFrame
class=\"linkmenu\">Gebruikers</a></td></tr>"
;
}
$magdit = 13;
include
'includes/magdit.inc';
$magdit2 = $magdit;
$magdit = 14;
include
'includes/magdit.inc';
$magdit3 = $magdit;
$magdit = 15;
include
'includes/magdit.inc';
$magdit4 = $magdit;
$magdit = 29;

// ... ohh the agony!
?>

In left_frame.php we see why magdit.inc ended so abruptly! It is used to reuse PHP logic! I particularly like how $magdit is the hardest working variable in PHP. It's an integer then a string, then an integer again, etc.

Another thing that needs to be pointed out is the use of literals in the code. What does 6,7,12,13,14,15,29 really mean? Integer literals in code are the only thing more confusing than building a PHP application with billions of includes. This WTF makes me dizzy. Thanks again to Michael for sending it in, and we all pity you.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.

Hmm... Very strange piece o

Hmm... Very strange piece of code..

<?php
$magedit = "";
// ...
if ( $magdit == null )
{
    $magdit = "nee";
}
?>

This is a subtle WTF.. This code only works because ( "" == NULL ) which in my eyes is a WTF in PHP. "" == 0 == NULL ... Riiiight..

And why oh why is $access first put into $variablelist which is then put into an array $arraytest via an eval (!!) before finally being compared to $cyfer via a foreach...?! Why not just go <?php ( $magdit == $access ) ? 'ja' : 'nee' ; ?> ?!


Even better.. Just use the very conveniently built in boolean logic ... :-P

(erlando) This code only wo

(erlando) This code only works because ( "" == NULL ) which in my eyes is a WTF in PHP. "" == 0 == NULL ... Riiiight..

Well, this is due to PHPs weak types; something one should be aware of. A better way to check against NULL would be is_null() or comparing with ===. But this would be expecting too much when looking at this code I guess ;)

When code built in another

When code built in another language contains english chunks, you can be certain of one thing: It was copy-pasted from some web site. Too bad there isn't an instant recognition method like that for English code.

best code ever seen and not e

best code ever seen and not even german!!!the magdit.inc can be summarized to
if( $magdit==$magdit {
 $magdit="ja";
}else{
 $magdit="nee";
}
thats faster, shorter but pointless too,

looking a little closer at eval()...

Hmm... looking at eval("\$arraytest = array($variablelist);"); a little more I think I see WHY it is used now. I'm thinking that $access is a string that contains something like 6,7,12,13,14,15,29.

That way when it's run through the eval() function it will look something like: eval("\$arraytest = array(6,7,12,13,14,15,29);");. hehehe... wow.

It could have been so easy ..

It could have been so easy ...
<?php
$vlist=explode(',',$variablelist);
if(
in_array(6,$vlist) or
in_array(7,$vlist) or
in_array(12,$vlist)
)
{
echo "<tr><td><a href=users.php target=mainFrame
class="linkmenu">Gebruikers</a></td></tr>";
}
// ...

The eval segment must be fr

The eval segment must be from one of those people who honestly believe that eval() is the only way to do math in javascript.The sheer amount of x = eval("1 + " + x) I've seen over the years is mindboggling.

Post new comment




*

  • Web and e-mail addresses are automatically converted into links.
  • Allowed HTML tags: <a> <em> <strong> <cite> <code> <ul> <ol> <li> <dl> <dt> <dd> <pre> <p> <br /> <br>