It’s a typical story. Dan on a rescue mission, fixing a mess that some clown(s) left behind. PHP. No framework to speak of, riddled with SQL injection holes, a TABLE-based layout – and it doesn’t get any better from there.

For the love of all things holy, why do people have to do stuff like this:

$sql = "SELECT user_id,user_status FROM users WHERE user_name='$username' AND user_password='$p'";
$r = mysql_fetch_assoc(mysql_query($sql));

For the record, $username and $p were just grabbed right out of $_POST.

If you spent 30 seconds to write even a crappy inefficient function to actually do something intelligent, not only would you not have code that’s riddled with SQL injection vulnerabilities (did I mention that this snippet of joy came out of a 3112 line file without a SINGLE comment?), but it might actually make your life easier because your code won’t suck so much – and you can stop repeating yourself.

I’m no 1337 PHP h4×0r, but how about – oh, I don’t know – something like this:

function fetch_associative_array_safely( $array ){
  $sql = $array[0];
  foreach ($array as $index => $value) {
    $sql = str_replace( "?".$index, addslashes($value), $sql );
  }
  return mysql_fetch_assoc( mysql_query( $sql ) );
}

And just execute that bad boy like so:

$r = fetch_associative_array_safely( 
      array( "SELECT user_id, user_status FROM users WHERE user_name='?1' AND user_password='?2'", 
             $username, $p) );

It’s not overly elegant, beautiful or efficient. But I don’t think that really matters. It helps me to not repeat myself, and by golly – at least someone can’t drop tables from my database anymore. It’s a bit Rails-esque, at least as far the the conditions portion of ActiveRecord::Base.find(...).

What do you think? I haven’t done any significant PHP coding in years.