[PHP] Verbesserungsvorschläge für Datenbankklasse

[Scream]

Ich möchte mein CMS ein wenig verbessern und habe dazu eine Datenbankklases geschrieben.
Wo seht ihr noch Verbesserungsmöglichkeiten? Gibts noch Sicherheitslücken?

<?php
/**
* Database class to implement singleton pattern on top of mysqli
*/
class Database extends mysqli {

/**
* @var object Singleton instance
*/
private static $instance = null;

// DB connection parameters:
private $dbHost = 'localhost';
private $dbUser = 'root';
private $dbPwd = 'test';
private $dbName = 'test';

/**
* Constructor
* @return void
*/
private function __construct() {
@parent::__construct($this->dbHost, $this->dbUser, $this->dbPwd, $this->dbName);
if(mysqli_connect_errno()) { // Keine Verbindung zur Datenbank möglich
$nachrichtenbox->set_get_nachricht("fehler","Verbindung fehlgeschlagen: ".mysqli_connect_error());
}
}

/**
* singleton
* @return object Database
*/
public function getInstance() {
if(self::$instance === null) {
$c = __CLASS__;
self::$instance = new $c;
self::$instance->set_charset("UTF8");
}
return self::$instance;
}

public function __clone() {
$nachrichtenbox->set_get_nachricht("fehler",__CLASS__." clonen nicht erlaubt.");
}

public function queryArray($sql) {
$db = Database::getInstance();
$result = $db->query($sql);
if($result) {
if($result->num_rows) {
while($row = $result->fetch_assoc())
$result_array[] = $row;
return $result_array;
}else {
return FALSE;
}
} else {
echo $nachrichtenbox->set_get_nachricht("fehler","FEHLER SELECT: " . $db->error . "<br />");
return FALSE;
}
}

public function querySingleItem($sql) {
$db = Database::getInstance();
$result = $db->query($sql);
if($result) {
if($result->num_rows) {
$row = $result->fetch_assoc();
return $row;
}else {
return FALSE;
}
} else {
echo $nachrichtenbox->set_get_nachricht("fehler","FEHLER SELECT: " . $db->error . "<br />");
return FALSE;
}
}

public function queryObjectArray($sql) {
$db = Database::getInstance();
$result = $db->query($sql);
if($result) {
if($result->num_rows) {
while($row = $result->fetch_object())
$result_array[] = $row;
return $result_array;
}else {
return FALSE;
}
} else {
echo $nachrichtenbox->set_get_nachricht("fehler","FEHLER SELECT: " . $db->error . "<br />");
return FALSE;
}
}


function Execute($sql) {
$db = Database::getInstance();
$result = $db->real_query($sql);
if($result) {
// return TRUE;
return $db->affected_rows;
} else {
echo $nachrichtenbox->set_get_nachricht("fehler","FEHLER EXECUTE: " . $db->error . "<br />");
return FALSE;
}
}

}
?>

[Kinman]

Bei einen Fehler gib das SQL Statement mit aus (ich glaub die mysqli->error() gibt es nämlich nicht aus) - Ist sehr praktisch.

Und mach Debug- / Errorausgaben ein und ausschaltbar.

mfg Kinman

[Scream]

Bei einen Fehler gib das SQL Statement mit aus (ich glaub die mysqli->error() gibt es nämlich nicht aus) - Ist sehr praktisch.

mfg Kinman

möchte ich eigentlich nicht direkt ausgeben
aber ich plane noch eine Logging-Klasse die Fehler extra mitloggt

[rotalever]

Zur Sicherheit:
Du übergibst das SQL für Queries immer direkt als kompletten String an die Funktionen, oder? Das hat zur Folge, dass der Benutzer der Datenbankklasse immer alles escapen muss. Das wird er natürlich von Zeit zu Zeit vergessen. Ich würde ein array von Parametern übergeben und prepared Statements verwenden.

[RMC]

Autsch .. das tut ja weh, deutsche Variablennamen X-(

Und was zur Hölle macht ein "set_get_nachricht" ?

Hm, vielleicht solltest du erstmal sinnvolle Variablen- und Funktionsnamen finden bevor du auch nur annähernd über "wichtigere" Dinge nachdenkst. Die größte Sicherheitslücke ist hier deine Namensgebung :D

Und die Variablen $host, $user, $pwd etc. sollte man besser nicht hardcoden, sondern der Klasse übergeben oder setzen.

[Scream]

Autsch .. das tut ja weh, deutsche Variablennamen X-(

Hm, vielleicht solltest du erstmal sinnvolle Variablen- und Funktionsnamen finden bevor du auch nur annähernd über "wichtigere" Dinge nachdenkst. Die größte Sicherheitslücke ist hier deine Namensgebung :D

meine namensgebung ist nicht optimal da muss ich dir zustimmen,aber ich denke auch die namen kann man vergeben wie man möchte ;)
Wo soll das bitteschön ne Sicherheitslücke sein? ;-)


Und was zur Hölle macht ein "set_get_nachricht" ?

das setzt eine nachricht und gibt sie gleich aus, werde ich aber später vermutlich mit meiner logging- bzw. error-Klasse kombinieren



Und die Variablen $host, $user, $pwd etc. sollte man besser nicht hardcoden, sondern der Klasse übergeben oder setzen.

da hast du natürlich recht, werde ich noch ändern