By day, Curtis was a PHP developer for a small ISV. The people he worked with were enjoyable and the work itself was decently challenging. All in all, it was nothing special.

However, in the evening, Curtis changed. Well, not 'changed', so much as charged. His clients, that is. You see, Curtis moonlighted for a number of small, developer-challenged companies, helping them build and maintain their Internet presence.

This work was a little more interesting, in that Curtis could use (within reason) some of the newer frameworks and techniques that were available. But it did leave Curtis open to getting the occasional emergency phone call if something unexpected happened. Which is where we find Curtis at the moment.

The manager at Rekall, a relatively new client of Curtis', called him shortly after dinner. The company's web site displayed a number of images. These images were stored in a directory on the web server and the names of the files were formatted to allow for the web site to display a subset as the need arose. The result was that, given the necessary permissions, non-technical people could add or delete the images as needed. While this might not be the best possible architecture, it was decently workable and had been successful since Curtis started working with this client a few months ago.

But tonight was different. Tonight, only some of the images were being displayed on the site. Hence the call to Curtis.

The image gathering section of the web site had been written by Curtis' predecessor. So Curtis settled down for a consultant's favourite pastime: code spelunking. Where consultants looked into the nooks and crannies of an application, peering into crevices not visited since the dawn of time (that is 1/1/70). Not deeply enough in the bowels of the server code, this is what he found.

function getpictures($directoryurl=0, $local_id=0, $thumps=FALSE) {
   $fd = fopen($directoryurl, "r");
   $newfield = array();
   $counter = 0;
   $hlp = 1;
   while (!feof($fd)) {
      $buffer = fgets($fd, 4096);
      $ausgabe = strip_tags($buffer, "


");
      $searcharray = explode(" ", $ausgabe);
      foreach ($searcharray as $element) {
         $upstr = strtoupper($element);
      // try{
      // if (!empty($upstr) && isset($upstr)&&($upstr!="")) {
         if ((strlen($upstr) > 7) && (substr_count($upstr, strtoupper($local_id))) && (substr_count($upstr, strtoupper('t'))))
         {
            if (strpos($upstr, '.JPG')) {
               $rest = substr($element, 0, (strpos($upstr, '.JPG') + 4));
               $newfield[$hlp] = $rest;
               $counter+=1;
               $hlp = $hlp + 1;
            } else
            if (strpos($upstr, '.JPEG')) {
               $rest = substr($upstr, 0, (strpos($upstr, '.JPEG') + 5));
               $newfield[$hlp] = $rest;
               $hlp = $hlp + 1;
               $counter+=1;
            } else
            if (strpos($upstr, '.PNG')) {
               $rest = substr($upstr, 0, (strpos($upstr, '.PNG') + 4));
               $newfield[$hlp] = $rest;
               $hlp = $hlp + 1;
               $counter+=1;
            } else
            if (strpos($upstr, '.GIF')) {
               $rest = substr($upstr, 0, (strpos($upstr, '.GIF') + 4));
               $newfield[$hlp] = $rest;
               $hlp = $hlp + 1;
               $counter+=1;
            }
          }
       // }
       // }catch(Exception $e){
       // return 'Error: exception '.get_class($e).', '.$e->getMessage().'.';
       // }
      }
   }
   fclose($fd);
   return $newfield;
}

The purpose of this method is to retrieve the file names that match a particular pattern found in the specified directory. Now PHP has a number of different ways to get a list of the files that are in a particular directory. opendir()/readdir() is one, fairly obvious, choice. scandir() is another. Or glob(). Or, if you prefer objects, there is a Directory class that wraps opendir()/readdir(). The options are there for the choosing. But I can pretty much guarantee that this function wouldn't be on your list.

The list of WTFs starts with the parameter list. Given that the function returns a list of items in a directory, it seems unlikely that the parameter that indicates the directory of interest should be optional. And the default value probably shouldn't be '0'. It's not nice to criticize the try/catch block that didn't seem to do much, because it has been commented out. However the $thumbs parameter isn't being used anywhere in the function. And that's a strike against any API.

But that's not all. Because the original developer seemed to be distrustful of basic O/S functions, they inadvertently introduce a particularly pernicious bug. If you look closely, you'll notice that the list of files is processed in 4K chunks. If an entry happens to cross over that 4K barrier, there is nothing in the code to pick up that entry. Which, by pure 'luck', is the reason why the entries weren't being retrieved properly in the first place.

For Curtis, the fix was easy and implementing it took less time than it did to figure out what this monstrosity was actually supposed to be doing. Curtis replaced the call with the appropriate opendir()/readdir() statements. And, for good measure, he added RegEx to find the correct files and life was good. On the bright side, since he was a consultant, he didn't have to remember the image files for them wholesale.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!