Showing all records on a webpage

Question

I have a connection with my database to show all my records on a webpage, and I'm not sure if this code is safe:

define('DB_HOSTNAME', 'localhost');
define('DB_USERNAME', 'root');
define('DB_PASSWORD', null);
define('DB_DATABASE', 'publicacoes');
define('DB_PREFIX', 'bn');
define('DB_CHARSET', 'utf8');
function DBEscape($dados){
 $link = DBConnect();
 if(!is_array($dados))
 $dados = mysqli_real_escape_string($link, $dados);
 else{
 $arr = $dados;
 foreach ($arr as $key => $value) {
 $key = mysqli_real_escape_string($link, $key);
 $value = mysqli_real_escape_string($link, $value);
 $dados[$key] = $value;
 }
 }
 DBclose($link);
 return $dados;
}
function DBclose($link){
 @mysqli_close($link) or die(mysqli_error($link));
}
function DBConnect(){
 $link = @mysqli_connect(DB_HOSTNAME, DB_USERNAME, DB_PASSWORD, DB_DATABASE) or die(mysqli_connect_error());
 mysqli_set_charset($link, DB_CHARSET) or die(mysqli_error($link));
 return $link;
}
function DBRead($table, $params = null, $fields = '*'){
 $table = DB_PREFIX.'_'.$table;
 $params = ($params) ? " {$params}" : null;
 $query = "SELECT {$fields} FROM {$table}{$params}";
 $result = DBExecute($query);
 if(!mysqli_num_rows($result))
 return false;
 else{
 while ($res = mysqli_fetch_assoc($result)){
 $data[] = $res;
 }
 return $data;
 }
}
function DBExecute($query, $insertId = false){
 $link = DBConnect();
 $result = @mysqli_query($link, $query) or die(mysqli_error($link));
 if($insertId)
 $result = mysqli_insert_id($link);
 DBClose($link);
 return $result;
}
$dadosIDrow1 = DBRead('publicacao', "ORDER BY id DESC LIMIT 2");
<?php foreach ($dadosUN as $UN): ?>
//some html here to display the records
<?php endforeach; ?

This is the first piece of code that I've written to show the records on my website. At the moment I have 2 connections with my DB. One of them I use PDO prepared statement to a pagination and search box.

I want to know if I should delete this script and make a new one with prepared statements.

Answer 1

I want to know if I should delete this script and make a new one with prepared statements.

Yes, definitely.
This set of functions is just unacceptable, being so bad on so many levels.

• There will be dozens to hundreds connections made in your script, not just "2 connections" for starter, as your code will connect anew every time it executes a query.
• There is, as you rightfully noted, no support for prepared statements, which is a disaster alone. That escaping routine is based on the false notion that mysqli_real_escape_string's purpose is to protect you from SQL injection. Well, it's but a grave delusion.
• error reporting is completely flawed. What a site user is supposed to do with a blank page telling them that a column not found?
• a DBRead() function is silly. All its purpose is to save you a typing of two words. Seriously? And this trifle gain at the expense of making a gibberish out of the powerful language of SQL.

To show 2 records from a table (as it's stated in the code as opposed to the question title) you need only two lines with vanilla PDO, so you better stick with it:

$sql = 'SELECT * FROM publicacao ORDER BY id DESC LIMIT 2';
$dadosUN = $pdo->query($sql)->fetchAll();
?>
<?php foreach ($dadosUN as $UN): ?>
//some html here to display the records
<?php endforeach ?>

• Susi: thank you for the explanation! But, what is the purpose of using $pdo->query istead of prepare?, i was thinking more about this: $stmtIDrow1 = $conn->prepare("SELECT * FROM bn_publicacao ORDER BY id DESC LIMIT 2"); $stmtIDrow1->execute()
• Your Common Sense: You don't have to prepare queries that do not accept data variables.