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.
