[PHP/MYSQL]MySQL-class auf dem Prüfstand + ein paar Fragen

BartTheDevil89

Devilution Media
ID: 87739
L
2 Mai 2006
3.960
103
Hallo,

also habe mir in letzter Zeit mal die MySQL-Klassen so angeschaut und für meine Page mir jetzt eine eigene erstellt. Ist zwar nur ne recht einfache, aber für den Einsatz gut geeignet. Meine class schaut wie folgt aus:

PHP:
<?
class db{

var $_dbHost        = "localhost";
var $_dbDatabase    = "";
var $_dbUsername    = "";
var $_dbPassword    = "";
var $_dbcon    = NULL;
   
    function start()
    {
        $this->_dbcon = mysql_connect( $this->_dbHost, $this->_dbUsername, $this->_dbPassword );
        mysql_select_db( $this->_dbDatabase, $this->_dbcon );
    }
	
	function close() {

      mysql_close($this->_dbcon);

    }
	
	function last(){ //letzte id wird geladen und zurückgegeben

    return mysql_insert_id($this->_dbcon); 

	}
	
	function query($value){ //Einfache Abfrage für Insert, Update
     
    return mysql_query($value, $this->_dbcon); 

	}
	
	function anzahl($value){ //Anzahl von Einträgen
     
    return mysql_num_rows(mysql_query($value, $this->_dbcon));
 
	}
	
	function data($value){ //Daten für einen Eintrag
     
    return mysql_fetch_array(mysql_query($value, $this->_dbcon));
 
	}
	
}
?>

Also damit sollte eigentlich alles abgedeckt sein, was so an SQL-Queries auf mich zukommt. Habe mir dazu auch die passende index-Datei erstellt, die mir alle Möglichkeiten von Abfragne mal auflistet. (durch $action ==... getrennt um es einfacher testen zu können)
PHP:
<?
include("class.mySQL.php"); 

$db = new db();
$db->start();

$action = $_GET['action'];
if($action == "insert"){ //Insert-Befehle
$db->query("insert into table set name = 'TEST2'");
$lastid = $db->last(); //Insert-ID wird geladen
echo "ID $lastid wurde erstellt...";
}
if($action == "update"){ //Updatebefehl
$db->query("update table set name = 'TEST' where lft = '1' and rgt='2'");
}
if($action == "anzahl"){//Anzahl an Einträgen
$eintragsanzahl = $db->anzahl("Select * from table where id = '1'");
echo "Es wurden $eintragsanzahl gefunden...";
}
if($action == "list"){ //Auflisten einer DB
$result = $db->query("Select * from table");
while($bit = mysql_fetch_array($result)){
$id = $bit['id'];
echo "ID $id wird geladen...<br>";
}
}
if($action == "show"){//Ein spezieller Datensatz wird geladen
$data = $db->data("Select * from table where id = '1'");
echo "$data[name] geladen...";
}

$db->close();
?>

Jetzt kommt allerdings die Frage: Was haltet ihr von? Und vor allem fehlt natürlich bisschen was. Also was auf jedenfall noch rein muss ist eine Fehlerausgabesystem...also sollte wirklich mal ein Fehler auftreten, dass der dann in ner einfachen Form ausgeben wird. Allerdings hat es an der Stelle dann bei mir aufgehört, wie ich das einbauen kann. Also wie schaffe ich es da ne Fehlermeldung ala: "Fehler! Die Abfrage:...(wäre ja $value), Server meldet:....)" noch mit reinkommt?
Natürlich hätte ich auch gern in dieser Query die Daten vor sql-injections geschützt, aber mit dem $value ist das nicht möglich oder hab ich was übersehen? Hatte es vorher, dass ich versucht habe die where-Daten, die Insert-Daten, etc... alles mit ner extra variable an die Funktion zu übergeben, sodass ich ja dan mysql_escape_real_string anwenden hätte können, aber das ist bei den komplexen Abfragen und vor allem anderswertigen Abfragen über diese Query einfach nicht möglich.
Hab ich sonst noch was vergessen, bzw. kann ich sonst noch irgendwas gutes mit einbauen? Und hab ich irgendwie ne Abfrageart vergessen?

Danke für die Hilfe und fürs Drüberschauen:roll:
 
Ich würde überall noch escapes gegen die SQL-Injections einbauen...irgendwo gabs da mal im Forum eine super Klasse, die auch dagegen abgesichert hat. Weiß jemand noch, wie die ging??
 
Ja aber kann ich das denn bei der nur übergebenen Variable $value?

nö.
darum solltest du die funktion so machen, dass du eine variable $sql übergibst, die den sql-code enthält und ein array, das die enthaltenten werte enthält...
siehe vprintf()

die funktion start() kann der konstruktor sein, damit sparst du dir 1 zeile code, die funktion close() der destruktor.

die funktion anzahl() sollte die anzahl der wertte mit SELECT COUNT(xxx) auslesen, das geht schneller.

in data() nutzt du wieder die funktion mysql_query... stattdessen kannst du auf die klassenfunktion $this->query zugreifen, dann musst du das escapen nicht 2 mal schreiben.
außerdem gibt data() nur den ersten (oder letzten!?) datensatz zurück... du könntest stattdessen die werte mit einer while()-schleife durchlaufen, sämtliche daten in einem array speichern und das dann zurückgeben.
 
Also ich habe meine Datenbankklasse (die im Übrigen erst einmal nur abstrakt ist und dann für den speziellen Fall z.B. als MySQL-Klasse abgeleitet wird) so angelegt, dass...
  • die Verbindung nur bei Bedarf hergestellt wird. D.h. man kann auf jeder Seite erst einmal die Klasse einbinden und das Objekt erstellen, da passiert noch gar nichts. Erst wenn eine Query abgeschickt werden soll, wird die Verbindung -falls noch nicht geschehen- aufgebaut.
  • ein gewisser Schutz vor SQL-Injections besteht (mittels vsprintf() und z.B. mysql_real_escape_string())
  • gleichzeitig mir die Klasse viele häufig benutzte Funktionen direkt anbietet (z.B. das komplette Ergebnis als ein großes Array zurückgeben, oder auch, falls nur ein einzige Feld abgefragt wird, nur dieses direkt als Variable zurückgeben)
  • durch die Aufteilung in abstrakte und abgeleitete Klasse das ganze mit minimalem Aufwand auf ein anderes Datenbanksystem portierbar ist (die Klasse+MySQL-Ableitung hat derzeit 14 Methoden und -ausführlich kommentiert- 12KiB Code, wovon aber lediglich 28 Code-Zeilen gebraucht wurden, um die Basisklasse in eine MySQL-Klasse abzuleiten.
Bisher ist mir an dem ganzen auch nur ein Punkt aufgefallen, der mir nicht so wirklich gut gefällt: Es besteht (noch) keine Möglichkeit, die mit MySQLi eingeführten prepared statements zu nutzen, die einen so weit ich weiß unumgehbaren Schutz vor SQL-Injections bieten würden.
 
Du könntest zum Beispiel bei Fehlern Exceptions werfen, was die Fehlerbehandlung um einiges vereinfachen wird.
Wie schon gesagt bei einem Insert könntest du auch bei einem Update ein Array mit "feld" => "wert" paaren übergeben. Das erpart dir ne Menge unübersichtliches Query tippen:
PHP:
::update($ayData, $table, $condition=null)

Dazu könntest du einbauen das sämtliche abgesetzte Queries in ein Logfile kommen. Eventuell gleich mit der Zeit die das jeweilige Query benötigt hat.

Oder intern mitzählen wieviel Selects/Updates/Inserts etc. abgesetzt wurden. Ich lass mir das dann immer am Ende der Seite ausgeben. Is manchmal schon ganz interessant.

Und der Support für Transaktionen, Prozeduren/Funktionen fehlt (Sofern du sowas irgendwann mal benötigst).
 
nö.
darum solltest du die funktion so machen, dass du eine variable $sql übergibst, die den sql-code enthält und ein array, das die enthaltenten werte enthält...
siehe vprintf()

die funktion anzahl() sollte die anzahl der wertte mit SELECT COUNT(xxx) auslesen, das geht schneller.

Ich hab es schon probiert update-Daten, Insert, etc. über ein array reinzubringen und dort dann auf sql-injections zu prüfen, aber da kommen so verschiedene Abfragen rein, dass es leider so nicht richtig läuft. Ich muss also die SQL-Injections schon im Script rausfiltern, bevor sie an die Query übergeben werden.

Ich hab mit dem anzahl() eben genau was anderes gelesen, dass es besser ist über num_rows zu arbeiten...hmh, schwierig.

Bisher ist mir an dem ganzen auch nur ein Punkt aufgefallen, der mir nicht so wirklich gut gefällt: Es besteht (noch) keine Möglichkeit, die mit MySQLi eingeführten prepared statements zu nutzen, die einen so weit ich weiß unumgehbaren Schutz vor SQL-Injections bieten würden.

Kannst du das bisschen näher erklären?

Und der Support für Transaktionen, Prozeduren/Funktionen fehlt (Sofern du sowas irgendwann mal benötigst).

Hast du bisschen nähere Infos, was du meinst?

@all: Bin jetzt hier nicht auf alles eingegeben, aber hab es auch nicht überlesen. ;)

Hab in meine class jetzt noch ein error-System eingebaut, das mir eben Fehlermeldung direkt rausbringt und das ganze danach beendet:

PHP:
<?
class db{

var $_dbHost        = "localhost";
var $_dbDatabase    = "";
var $_dbUsername    = "";
var $_dbPassword    = "";
var $_dbcon    = NULL;
   
    function start()
    {
        $this->_dbcon = @mysql_connect( $this->_dbHost, $this->_dbUsername, $this->_dbPassword );
        
		if (!$this->_dbcon){
		$this->error("con","");//Fehler: Verbindung konnte nicht hergestellt werden
        }
		
		$this->_select = @mysql_select_db( $this->_dbDatabase, $this->_dbcon );
		
		if (!$this->_select){
		$this->error("sel","");//Fehler: Verbindung mit der Datenbank nicht möglich
        }
    }
	
	function close() {

		$this->_close = @mysql_close($this->_dbcon);
	  
	  	if (!$this->_close){
		$this->error("close","");//Fehler: Beenden der Datenbank nicht möglich
        }

    }
	
	function last(){ //letzte id wird geladen und zurückgegeben
	
		$this->_insertid = @mysql_insert_id($this->_dbcon);

		if (!$this->_insertid){
		$this->error("last","");//Fehler: Insert-Id kann nicht übergben werden
        }else{
		return $this->_insertid;
		}
	}
	
	function query($value){ //Einfache Abfrage für Insert, Update
     
		$this->_querydata = @mysql_query($value, $this->_dbcon); 
	
		if (!$this->_querydata){
		$this->error("query",$value);//Fehler: Query nicht möglich
        }else{
		return $this->_querydata;
		}

	}
	
	function anzahl($value){ //Anzahl von Einträgen
     
		$this->_nums = @mysql_num_rows(mysql_query($value, $this->_dbcon));
	
		if (!$this->_nums){
		$this->error("anzahl",$value);//Fehler: Anzahl kann nicht ausgeben werden
        }else{
		return $this->_nums;
		}
 
	}
	
	function data($value){ //Daten für einen Eintrag
     
		$this->_datas = @mysql_fetch_array(mysql_query($value, $this->_dbcon));
	
		if (!$this->_datas){
		$this->error("data",$value);//Fehler: bei der Query
        }else{
		return $this->_datas;
		}
 
	}
	
	function error($error_type,$errorsql) { 
		switch ($error_type) { 
		case 'con': 
		$text = "Es konnte leider keine Verbindung zur Datenbank hergestellt werden!"; 
		$showmysql = 1;
		break; 

		case 'close': 
		$text = "Die Verbindung mit der Datenbank konnte leider nicht beendet werden!"; 
		break; 

		case 'sel': 
		$text = "Die angegebene Datenbank ist nicht vorhanden!"; 
		break; 

		case 'last': 
		$text = "Dieser Datenbankeintrag ist nicht vorhanden: <i>".$errorsql."</i>"; 
		break; 
		
		case 'query': 
		$text = "Während dieser Abfrage ist ein Fehler aufgetreten: <i>".$errorsql."</i>"; 
		break; 
		
		case 'data': 
		$text = "Während dieser Abfrage ist ein Fehler aufgetreten: <i>".$errorsql."</i>"; 
		break; 
		
		case 'anzahl': 
		$text = "Die Anzahl dieser Datenbankeinträge kann nicht geladen werden: <i>".$errorsql."</i>"; 
		break; 
		}
		
		echo "<b>Fehler:</b><br><br>$text";
		if($showmysql != 1){
		echo "<br><br><u>SQL meldet:</u><br><br>Fehler-Nummer: ".@mysql_errno($this->_dbcon)."<br>Fehler-Beschreibung: ".@mysql_error($this->_dbcon); 
		}
		die();
	}
	
}
?>
 
Kannst du das bisschen näher erklären?

Hättest du den Link aufmerksam gelesen, wüsstest du wahrscheinlich worum es geht, aber falls du es Englischen nicht mächtig bist, eine kurze Beschreibung:
Die Datenbankabfrage wird zunächst nur mit Platzhaltern an den Server geschickt, später werden die entsprechenden Werte nachgereicht, die dann der Server für die Platzhalter einsetzt. Da der Server aber schon weiß, dass es sich dabei um Werte handelt, können auch eingebaute Steuerzeichen die Abfrage so abändern, dass etwas Unvorhergesehenes passiert.

Alles klar?
 
Hast du bisschen nähere Infos, was du meinst?
Funktionen/Prozeduren sind serverseitig in der DB gespeichert und werden durch ein Query aufgerufen. Der Vorteil daran ist das man viele Sachen die man normalerweise in PHP macht gleich direkt in der Datenbank machen kann. Ist um einiges schneller ->link.
Und zu Transaktionen: link, auch da kommt man meiner Meinung nach nich drumherum. Dazu müsstest du aber von den myISAM Tabellen weg (welche ich mal vermute du benutzt).

Und:
Dein Error- Handling in allen Ehren, aber du wirst sicherlich in Situationen kommen wo es nich so prickelnd ist wenn die Klasse sich im Fehlerfall mit einem echo meldet. Die Kunden/User sollen/wollen ja nicht mitbekommen wenn da was nich stimmt.
 
Bisher ist mir an dem ganzen auch nur ein Punkt aufgefallen, der mir nicht so wirklich gut gefällt: Es besteht (noch) keine Möglichkeit, die mit MySQLi eingeführten prepared statements zu nutzen, die einen so weit ich weiß unumgehbaren Schutz vor SQL-Injections bieten würden.

klingt so, als wäre mysql_real_escape_string() nicht unumgehbar? hast du genauere informationen dazu?

@Bart:
1. argument, $sql, beinhaltet zb folgenden string:
"SELECT username,password FROM users WHERE id = %d AND session_code = '%s'".
2. Argument, $values, beinhaltet:
array(1,'hc8794hf734hf97g5t4795g');
 
Ah ok...werde mich mal durch all eure Links und Beschreibungen durcharbeiten. Hab allerdings noch nen Fehler, den ich grad gefunden habe. Und zwar meldet mir mein error-Handling einen Fehler, wenn bei der anzahl() eine 0 rauskommt. Also ich meine diese Stelle hier:

function anzahl($value){ //Anzahl von Einträgen

$this->_nums = @mysql_num_rows(mysql_query($value, $this->_dbcon));

if (!$this->_nums){
$this->error("anzahl",$value);//Fehler: Anzahl kann nicht ausgeben werden
}else{
return $this->_nums;
}

}

Also wenn die Anzahl 0 ist, dann geht er auch in den error() rein. Hab es schon so probiert,

if (!$this->_nums and $this->_nums != 0){

aber damit geht er ganichtmehr in den error rein.
 
Probier's mal mit:
PHP:
if ($this->_nums===false or $this->_nums<0)

Nein, damit gehts auch nicht...hab auch schon mit and anstatt or versucht (denn würde mich für logischer klingen), aber geht nicht. Denn das ding da oben heißt doch wenn die Abfrage schief ging (false) oder die Anzahl der Einträge < 0 ist, dann Fehler. Aber ich will doch nur, dass der Fehler kommt, wenn die Abfrage schief ging. Also wenn die Abfrage richtig lief, mir nur eine 0 als Anzahl der Einträge lieft, dann soll er mir die auch einfach ausliefern und nicht den Fehler bringen. Deswegen war ja meine Idee anfangs

if (!$this->_nums and $this->_nums != 0{

Aber damit gehts auch nicht...
 
Naja, wenn die Abfrage fehl schlägt, solltest Du sie auch gar nicht durch mysql_num_rows() jagen, da kann nix Vernünftiges bei raus kommen. mysql_query() gibt false zurück bei einer falschen Abfrage, das kannst Du überprüfen und wenn Du innerhalb der Funktion anzahl() Deine eigene Queryfunktion nimmst, musst Du die Fehlerausgabe nur ein einziges Mal implementieren.
 
Naja, wenn die Abfrage fehl schlägt, solltest Du sie auch gar nicht durch mysql_num_rows() jagen, da kann nix Vernünftiges bei raus kommen. mysql_query() gibt false zurück bei einer falschen Abfrage, das kannst Du überprüfen und wenn Du innerhalb der Funktion anzahl() Deine eigene Queryfunktion nimmst, musst Du die Fehlerausgabe nur ein einziges Mal implementieren.

:wall: ...danke. Hätt ich eigentlich auch selbst draufkommen können, dass ich das einfache teile. Damit läuft es jetzt:

PHP:
	function anzahl($value){ //Anzahl von Einträgen
     
		$this->_nums = @mysql_query($value, $this->_dbcon); 
	
		if (!$this->_nums){
			$this->error("anzahl",$value);//Fehler: Query nicht möglich
        }else{
			$this->_nums2 = @mysql_num_rows($this->_nums);
			return $this->_nums2;
		}
 
	}
 
klingt so, als wäre mysql_real_escape_string() nicht unumgehbar? hast du genauere informationen dazu?

Nein habe ich nicht, weil ich das auch gar nicht behaupten wollte :) Aber früher habe ich auch immer gedacht, addslashes() sei eine sichere Lösung, bis ich dann irgendwann mal über einen Link hier aus dem Forum lernen musste, dass es das nicht ist (weil es mit Multibyte-Zeichen nicht korrekt bzw. auf jeden Fall anders als der MySQL-Server umgeht). Und ich denke allein aufgrund der Funktionsweise müssten prepared statements noch um einiges "unumgehbarer" sein als mysql_real_escape_string(), denn wer sagt denn das in der Funktion nicht auch irgendwo noch ein Bug steckt?
 
Ich habe mir nicht ganz genau alle Antworten durchgelesen, vielleicht hat das also schon mal jemand angemerkt. In diesem Fall einfach überlesen ;)

Du hast die Methoden query() und data(), die aber inhaltlich eigentlich komplett identisch sind (außer das einige Sachen anders heißen). Aber sie machen genau das selbe. Warum also die Funktionalität doppelt?

Die Methode anzahl() führt immer die Query aus. Das finde ich persönlich merkwürdig. Wenn man z.B. zuerst data() aufruft, dann wurde die Query ja schonmal ausgeführt. Das Ergebnis ist also schon vorhanden und ich kann mysql_num_rows() auf das Ergebnis anwenden, ohne das ich die eigentliche Abfrage nochmals machen muss. (Man müßte sich merken, ob die Abfrage schon gemacht wurde.)