I'm fairly new to php and trying out some "tutorials" floating around on the web. One problem I have run into is older tutorials on "up-to-date" php/MySQL platforms.
For instance:
public static function getList( $numRows=1000000, $order="publicationDate DESC" ) {
$conn = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
$sql = "SELECT SQL_CALC_FOUND_ROWS *, UNIX_TIMESTAMP(publicationDate) AS publicationDate FROM articles
ORDER BY " . mysql_escape_string($order) . " LIMIT :numRows";
$st = $conn->prepare( $sql );
$st->bindValue( ":numRows", $numRows, PDO::PARAM_INT );
$st->execute();
$list = array();
while ( $row = $st->fetch() ) {
$article = new Article( $row );
$list[] = $article;
}
Because mysql_escape
.. is deprecated, I'm trying to replace it with mysqli_escape_string($conn, $order)
. But i believe my $st = $conn->prepare($sql)
is throwing monkey wrench in the mix.
Any suggestions/explanations?
You're using PDO, so you should be using prepared queries anyway.
That said, prepared queries won't work for what you're trying to do. And in any case, mysql_real_escape_string
WILL NOT SAVE YOU HERE.
Instead, you could try some custom escaping:
"... ORDER BY `".str_replace(array("`","\\"),"",$order)."` LIMIT ...";
Backticks are USEFUL! :p
Okay several issues:
You can't mix mysql_*
functions with PDO. I know it's confusing, because PHP has three different API's for MySQL. But you have to use one at a time.
The escaping functions, including PDO::quote(), are only for escaping string literals and date literals. They don't do the right thing for column names.
Your $order
variable has two syntax elements: the column, and the direction. Even if you were to quote the column properly, you wouldn't want to include DESC
inside the quotes.
The direction (ASC
/DESC
) is an SQL keyword, so it does not go into quotes. Therefore there's no escaping possible to make it safe against SQL injection.
My solution to this is not to use quoting/escaping, but to use whitelisting. And keep the sort column separate from the sort direction.
public static function getList( $numRows=1000000, $order="publicationDate", $direction="DESC" ) {
$columns = array("publicationDate"=>"publicationDate", "name"=>"authorName", "DEFAULT"=>"publicationDate");
$directions = array("up"=>"ASC", "down"=>"DESC", "DEFAULT"=>"DESC");
$order_column = $columns[$order] ?: $columns["DEFAULT"];
$order_direction = $directions[$direction] ?: $directions["DEFAULT"];
$conn = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
$sql = "SELECT *, UNIX_TIMESTAMP(publicationDate) AS publicationDate
FROM articles
ORDER BY $order_column $order_direction LIMIT :numRows";
. . .
}
Tip: I never use SQL_CALC_FOUND_ROWS
because it is such a dire performance killer. It's actually much better to run two queries, one to get a SELECT COUNT(*)...
and the second to get the limited result set.
See tests at To SQL_CALC_FOUND_ROWS or not to SQL_CALC_FOUND_ROWS?