Reference: What is a perfect code sample using the MySQL extension? [closed]

This is to create a community learning resource. The goal is to have examples of good code that do not repeat the awful mistakes that can so often be found in copy/pasted PHP code. I have requested it be made Community Wiki.

This is not meant as a coding contest. It's not about finding the fastest or most compact way to do a query - it's to provide a good, readable reference especially for newbies.

Every day, there is a huge influx of questions with really bad code snippets using the mysql_* family of functions on Stack Overflow. While it is usually best to direct those people towards PDO, it sometimes is neither possible (e.g. inherited legacy software) nor a realistic expectation (users are already using it in their project).

Common problems with code using the mysql_* library include:

• SQL injection in values
• SQL injection in LIMIT clauses and dynamic table names
• No error reporting ("Why does this query not work?")
• Broken error reporting (that is, errors always occur even when the code is put into production)
• Cross-site scripting (XSS) injection in value output

Let's write a PHP code sample that does the following using the mySQL_* family of functions:

• Accept two POST values, id (numeric) and name (a string)
• Do an UPDATE query on a table tablename, changing the name column in the row with the ID id
• On failure, exit graciously, but show the detailed error only in production mode. trigger_error() will suffice; alternatively use a method of your choosing
• Output the message "$name updated."

And does not show any of the weaknesses listed above.

It should be as simple as possible. It ideally doesn't contain any functions or classes. The goal is not to create a copy/pasteable library, but to show the minimum of what needs to be done to make database querying safe.

Bonus points for good comments.

The goal is to make this question a resource that a user can link to when encountering a question asker who has bad code (even though it isn't the focus of the question at all) or is confronted with a failing query and doesn't know how to fix it.

To pre-empt PDO discussion:

Yes, it will often be preferable to direct the individuals writing those questions to PDO. When it is an option, we should do so. It is, however, not always possible - sometimes, the question asker is working on legacy code, or has already come a long way with this library, and is unlikely to change it now. Also, the mysql_* family of functions is perfectly safe if used properly. So no "use PDO" answers here please.

Solution 1:

My stab at it. Tried to keep it as simple as possible, while still maintaining some real-world conveniences.

Handles unicode and uses loose comparison for readability. Be nice ;-)

<?php

header('Content-type: text/html; charset=utf-8');
error_reporting(E_ALL | E_STRICT);
ini_set('display_errors', 1);
// display_errors can be changed to 0 in production mode to
// suppress PHP's error messages

/*
Can be used for testing
$_POST['id'] = 1;
$_POST['name'] = 'Markus';
*/

$config = array(
    'host' => '127.0.0.1', 
    'user' => 'my_user', 
    'pass' => 'my_pass', 
    'db' => 'my_database'
);

# Connect and disable mysql error output
$connection = @mysql_connect($config['host'], 
    $config['user'], $config['pass']);

if (!$connection) {
    trigger_error('Unable to connect to database: ' 
        . mysql_error(), E_USER_ERROR);
}

if (!mysql_select_db($config['db'])) {
    trigger_error('Unable to select db: ' . mysql_error(), 
        E_USER_ERROR);
}

if (!mysql_set_charset('utf8')) {
    trigger_error('Unable to set charset for db connection: ' 
        . mysql_error(), E_USER_ERROR);
}

$result = mysql_query(
    'UPDATE tablename SET name = "' 
    . mysql_real_escape_string($_POST['name']) 
    . '" WHERE id = "' 
    . mysql_real_escape_string($_POST['id']) . '"'
);

if ($result) {
    echo htmlentities($_POST['name'], ENT_COMPAT, 'utf-8') 
        . ' updated.';
} else {
    trigger_error('Unable to update db: ' 
        . mysql_error(), E_USER_ERROR);
}

Solution 2:

I decided to jump the gun and just put something up. It's something to start with. Throws an exception on error.

function executeQuery($query, $args) {
    $cleaned = array_map('mysql_real_escape_string', $args);

    if($result = mysql_query(vsprintf($query, $cleaned))) {
        return $result;
    } else {
        throw new Exception('MySQL Query Error: ' . mysql_error());
    }
}

function updateTablenameName($id, $name) {
    $query = "UPDATE tablename SET name = '%s' WHERE id = %d";

    return executeQuery($query, array($name, $id));
}

try {
    updateTablenameName($_POST['id'], $_POST['name']);
} catch(Exception $e) {
    echo $e->getMessage();
    exit();
}