The PHP WTF The PHP WTF - Examples of really bad PHP en-US Why use MD5 when you got MD4? <p>Okay before we get deeper into this craziness I would like to remind people that <em>MD5()</em> has been available since php3. Plus <em>MD5</em> is way more secure than MD4... so I introduce you to today's PHP WTF.</p> <pre><code><?php function getMd4Pwd($pwd) { $pwd = trim($pwd); if (strlen($pwd) <= 0) return ""; unset($arrOut); <em><strong>$strCmd = "/usr/local/bin/md4sum ".$pwd;</strong></em> exec($strCmd,$arrOut); return strtoupper($arrOut[0]); } ?></code></pre> <p> But wait! It gets worse... not only are they not using <em>md5()</em>, they execute a shell script to get an MD4 hash! Really you can't make this stuff up... </p> <p>And what is /usr/local/bin/md4sum you may ask? Well let me show you...</p> <pre><code>#!/usr/bin/perl -w use Digest::MD4; use Unicode::String qw( utf8 ); Unicode::String->stringify_as( "utf16" ); $u8 = utf8( shift ); print Digest::MD4->hexhash($u8->byteswap), "\n"; </code></pre> <p> So we have a PHP script that calls a Perl script to generate an obsolete, insecure MD4 hash. Not only that but Perl doesn't even have MD4 by default, you have explicitly install it. Um...WTF?! </p> Not so Source Control... <p>There is an interesting <a href="">post</a> on the <a href="">The Daily WTF</a> about Source Control, or the lack of it. </p> <p>I wanted to share this. The previous developers didn't use any source control, they simply did the same thing by renaming the old files and added new ones. The file names have been changed to protect the guilty: <img src="" border="0" alt="Not so Source Control" /></p> <p>This WTF is completely a human problem. Given a bad situation people (especially bad programmers) will find the ways to make it worse. The problem gets worse when dumb developers start using those old libraries! Now your messy directories have turned into software requirements! This may sound retarded but this has happened, and will happen!</p> <p>Remember <b>source control is to your code as accounting is to your business</b>. </p> When Newbies Attack! <p>I think inexperienced web programmers all make common DB WTFs when starting out. <b>Jim Grill</b> sent in a prime example from a project that he inherited. I'm sure we've all seen similar code before and we've all said, "wtf?!", if not "ytf?!"</p> <pre><code><?php $query = 'SELECT * FROM sometable'; $result = mysql_query($query,$connection); $count = mysql_num_rows($result); ?> </code></pre> <p> It should be obvious what's wrong in the example. To count the number of rows all data is needlessly requested and the rows counted in PHP. As the table grows these three lines will get slower and slower. Most people take data transfer from the DB server for granted. However we always should be as efficient as possible, since little things can quickly multiply into big problems. </p> <p>The fix is relatively simple: </p> <pre><code><?php $query = 'SELECT COUNT(*) FROM sometable'; $result = mysql_query($query,$connection); list($count) = mysql_fetch_array($result); ?> </code></pre> <p> Using <code>count(*)</code> will return just the number of rows. Much more efficient. </p> Probably the worse way to pad a string... <p>Everyday I find code examples that make me go WTF. This is a prime example of one. It is a function that takes an item's id number and generates a left zero padded string with the id number. </p> <p>I guess nobody told them about the str_pad() function. </p> <pre><code> function getVanPic($item_id) { $pic_path = "/path/to/pic/dir"; if ($item_id >= 1000000) { $strPicName = $item_id; } else if ($item_id >= 100000) { $strPicName = "0".$item_id; } else if ($item_id >= 10000) { $strPicName = "00".$item_id; } else if ($item_id >= 1000) { $strPicName = "000".$item_id; } else if ($item_id >= 100) { $strPicName = "0000".$item_id; } else if ($item_id >= 10) { $strPicName = "00000".$item_id; } else { $strPicName = "000000".$item_id; } $strPicName1 = "w".$strPicName.".jpg"; if (file_exists($pic_path."/".$strPicName1)) { return "van/".$strPicName1; } $strPicName1 = "W".$strPicName.".JPG"; if (file_exists($pic_path."/".$strPicName1)) { return "van/".$strPicName1; } return ""; } </code></pre> The Launch of the PHP WTF <p>Welcome to The PHP WTF. The idea came from Alex of <a href="">The Daily WTF</a>. Alex has many examples of VB/Java nightmares and I figured the World needs examples of the bad PHP that people write. </p> <p>To start things off here's an example from my personal collection. Originally published on my personal blog at <a href=""></a>, it demonstrates one of the worse things in PHP, variable variables! Ugh!<br /> <hr /><br /> I have found that explaining bad code to non-programmers is usually followed up by, "isn't it a matter of style?". Programming styles may differ but there is a difference between bad style and bad code.</p> <p>This is today's example of bad code. </p> <pre><code>case 2: $stSQL = "UPDATE associate set "; for ($i = 0; $i < sizeof($arrReferralName); $i++) { if (strlen(trim(${"assc_".$arrReferralName[$i]."code"})) > 0) $stSQL = $stSQL." assc_".$arrReferralName[$i]. "code=".${"assc_".$arrReferralName[$i]."code"}.","; else $stSQL = $stSQL." assc_".$arrReferralName[$i]."code=NULL,"; $stSQL = $stSQL." assc_".$arrReferralName[$i]."='".${"assc_".$arrReferralName[$i]}."'"; if ($i < (sizeof($arrReferralName) - 1)) $stSQL = $stSQL.","; } $stSQL = $stSQL." WHERE assc_id=".$ul["assc_id"]; mysql_db_query($mydatabase,$stSQL); break; </code> </pre> <p>Other than generating a SQL query, nobody, including the original author can tell what the above code is supposed to do at a glance. The code's purpose has been hidden behind variable variables, a PHP 'feature' that should rarely be used.</p> <p>A requirement of good code is its purpose is immediately apparent. The example requires a lot of investigation to figure out what it does. Investigation takes a lot of effort, it is frustrating, it is aggrivating and it is a waste of time (and thus money).</p> <p>This is what I rewrote it to. The functionality has been updated to include better data checking and the structure changed to better show the code's purpose. </p> <pre><code>case 2: // Update the Phone Numbers $stSQL = "UPDATE associate set "; foreach ($arrReferralName as $refname) { // $arrReferralName from inc_referral.php (in common/) // DB Column Names $areacode = 'assc_'.$refname.'code'; $phonenum = 'assc_'.$refname; // DB Column Data $codeData = format_phonecode($_POST[$areacode]); $phoneData = format_phonenum($_POST[$phonenum]); if ($codeData == '') $codeData = 'NULL'; // update the area code $stSQL .= "$areacode=$codeData, "; // update the phone num $stSQL .= "$phonenum='$phoneData' ,"; } // chop the trailing comma $stSQL = substr($stSQL,0,-1); $stSQL .=" WHERE assc_id=".$ul["assc_id"]; mysql_db_query($mydatabase,$stSQL); break; </code></pre> <p>The new code is double the length of the original, but it is much simplier. It is now apparent that the code uses POST data to generate a SQL query for updating phone number data. The new code uses the $_POST super-variable, and references the data as elements of the array. <code>$_POST[$varname]</code> is a lot easier to understand than <code>${"assc_".$arrReferralName[$i]."code"}</code>. Especially since the code is old and written to use automatically registered globals!</p> <p>Being clever is not always a good thing. There should be a balance between clever code and good code. Good code is considerate to those that come after you. Clever code, like above, leaves a legacy of frustration, resentment and expense in time and money. </p>