Aufbau Datenbankschnittstelle

[dave]

Hi,

ich habe eine Website auf der Daten verwaltet werden.
Angemeldete Benutzer können Datensätze hinzufügen, ändern und löschen.
Ich habe jetzt für diese 3 Möglichkeiten 1 PHP-Skript welches die Anfragen bearbeitet.

Welche Aktion ausgeführt werden soll, welcher Datensatz betroffen ist (id) und in welcher Tabelle übergebe ich als URL-Parameter.
Die Daten übergebe ich als POST. Also z.B. welche Spalte mit welchem Wert belegt werden soll.

Jetzt habe ich ein paar Probleme:
1. Ich weiß nicht welches Format eine Spalte hat, muss das aber bei einem Prepared-Statement wissen.
Meine Lösung ist im Moment ein Assoziatives Array in dem alle Spalten mit entsprechendem Typ stehen ($fieldTypes).
Es ist sehr unwahrscheinlich dass neue Spalten hinzukommen.
Gibt es hierfür eine schönere Lösung?

2. Es wird eine variable Anzahl an Spalten geupdatet.
Meine Lösung ist hier für jede Spalte ein eigenes Update abzuschicken.
Hier stört mich dass ich den Spaltennamen nicht als Parameter über bind() mit angeben kann, sondern jeweils einen neuen Query-String zusammenbauen muss.
Geht das doch irgendwie?
Oder hat jemand eine schönere Lösung?

Hier mein derzeitiger Code, nicht hier dabei ist die Prüfung ob alle nötigen Variablen übergeben wurden und das $fieldTypes-Array.
Falls euch etwas davon helfen sollte mir zu helfen sagt bitte Bescheid.

In $fieldValues stehen die Daten aus $_POST. Das ist so da ich zum Testen diese Daten über URL-Parameter übergeben habe und durch dieses umkopieren nur an einer Stelle zwischen $_POST und json_decode($_GET['values']) wechseln muss.

switch($_GET['action']){  
	case 'delete':  
		$stmt = $mysqli->prepare('DELETE FROM '.mysql_real_escape_string($_GET['table']).' WHERE id = ?');  
		$stmt->bind_param($fieldTypes['id'], $_GET['id']);  
		$stmt->execute();  
		$stmt->close();  
		break;  
	case 'insert':  
		$sql = 'INSERT INTO '.mysql_real_escape_string($_GET['table']).' (id) VALUES(?)';  
		$stmt = $mysqli->prepare($sql);  
		$stmt->bind_param($fieldTypes['id'], $_GET['id']);  
		$stmt->execute();  
		$stmt->close();  
	// Kein Break! Nachdem Datensatz mit entsprechender ID angelegt wurde, werden die einzelnen Felder entsprechend gesetzt  
	case 'update':  
		$sqlPre = 'UPDATE '.mysql_real_escape_string($_GET['table']).' SET ';  
		$sqlPost= ' WHERE id = ?';  
		// Jedes zu ändernde Feld bekommt ein eigenes Update-Statement  
		foreach($fieldValues as $key => $value){  
			if(isset($fieldTypes[$key])){  
				$sql = $sqlPre.mysql_real_escape_string($key).' = ?'.$sqlPost;  
				$stmt = $mysqli->prepare($sql);  
				$stmt->bind_param($fieldTypes[$key].$fieldTypes['id'], $value, $_GET['id']);  
				$stmt->execute();  
				$stmt->close();  
			}  
		}  
		break;  
	default:  
		throw new Exception('Unbekannte Aktion');  
}

Auch über Hinweise zu anderweitigen Fehler freue ich mich.

~dave

[dedlfix]

Hi!

1. Ich weiß nicht welches Format eine Spalte hat, muss das aber bei einem Prepared-Statement wissen.

Zahlenwerte können auch als String übergeben werden. Probier mal alle relevanten Typen aus, ob sie sich als s übergeben lassen.

2. Es wird eine variable Anzahl an Spalten geupdatet.
   Hier stört mich dass ich den Spaltennamen nicht als Parameter über bind() mit angeben kann, sondern jeweils einen neuen Query-String zusammenbauen muss.
   Geht das doch irgendwie?

Nein, du kannst nur Werte binden, keine Bezeichner. Aber was genau stört dich? Code zu schreiben, der aus einem Array mit Spaltennamen ein INSERT-Statement zusammenbaut ist eine einmalige Angelegenheit und auch keine Raketenwissenschaft.

[code lang=php]switch($_GET['action']){

Ich würde den Code kapseln (Funktion oder Objekt) und dabei keinen direkten Zugriff auf externe Dinge verwenden, sondern alle benötigten Werte per Parameter übergeben. Dantenbankzugriff ist etwas sehr allgemeines, was man gut wiederverwenden kann, wenn man es nicht mit projektspezifischen Dingen spickt.

  $stmt = $mysqli->prepare('DELETE FROM '.mysql\_real\_escape\_string($\_GET['table']).' WHERE id = ?');  

Gute Idee, die Bezeichner zu maskieren, aber falsche Ausführung. Da sind gleich drei Fehler enthalten.

• mysql_real_escape_string() maskiert nur, es quotiert nicht. Wenn du keinen String einleitest, bringt es keine Punkte, stringspezifische Sonderzeichen zu behandeln. Ein Angreifer ist nämlich nicht im String-Kontext gelandet und muss daraus nicht auszubrechen versuchen. Stattdessen ist er im Code-Kontext geblieben und kann mit einem Leerzeichen getrennt nach dem Tabellennamen einfach weiterschreiben. Es fehlen also die Anführungszeichen um den Tabellennamen.
• Bezeichner werden (falls nötig - und ja, es ist in deinem Fall dringend geboten, um explizit in den Bezeichner-Kontext zu gelangen, weil du Benutzereingaben durchreichst) unter MySQL mit Backticks eingerahmt. Zudem folgen sie anderen Maskier-Regeln als mysql_real_escape_string() behandelt. Es gibt keine spezielle Funktion zu ihrer Behandlung, aber es reicht, einfache Backticks zu verdoppeln, was mit str_replace() machbar ist.
• mysql_real_escape_string() benötigt zum Arbeiten eine Verbindung zum DBMS. Vermutlich findet es deine mysqli-Verbindung, aber genau weiß ich nicht, ob es da PHP-Extension-übergreifend vorgeht. Wenn nicht, baut es sich selbst eine auf, wozu es aber Default-Konfigurationswerte benötigt, die üblicherweise nicht gesetzt sind, was wiederum dafür spricht, dass es deine vorhandene Verbindung nimmt, sonst gäbe es nur false zurück. Jedenfalls, wenn du schon objektorientiert mit mysqli arbeitest, solltest du auch dessen Methoden verwenden. (Allgemein gesagt, denn in deinem speziellen Fall ist das Escapen mit mysqli::real_escape_string() ja nicht richtig.)

Lo!

[dave]

Hi,

Zahlenwerte können auch als String übergeben werden. Probier mal alle relevanten Typen aus, ob sie sich als s übergeben lassen.

Funktioniert.
Welchen Nutzen hat es dann überhaupt hier etwas anderes zu verwenden?

Nein, du kannst nur Werte binden, keine Bezeichner. Aber was genau stört dich? Code zu schreiben, der aus einem Array mit Spaltennamen ein INSERT-Statement zusammenbaut ist eine einmalige Angelegenheit und auch keine Raketenwissenschaft.

Das Problem ist nicht das Zusammenbauen des SQLs, sondern das übergeben der Paramter an bind().
->bind('s',$fieldValue['first'], $fieldValue['second'], ...)

Ich weiß nicht wie ich die verschiedene Anzahl an Parametern variabel machen könnte.
Ohne Prepared-Statement wär's kein Problem, aber dann müsste ich mich ja selber um den Kontextwechsel kümmern :-(
Zudem werden AFAIK Prepared-Statements effizienter gecachet(?)

Ich würde den Code kapseln (Funktion oder Objekt) und dabei keinen direkten Zugriff auf externe Dinge verwenden, sondern alle benötigten Werte per Parameter übergeben. Dantenbankzugriff ist etwas sehr allgemeines, was man gut wiederverwenden kann, wenn man es nicht mit projektspezifischen Dingen spickt.

Da hast du Recht. Wird gemacht.

Gute Idee, die Bezeichner zu maskieren, aber falsche Ausführung. Da sind gleich drei Fehler enthalten.

$mysqli->prepare('DELETE FROM '.str_replace('','``',$tableName).' WHERE id = ?');`

So dann richtig?

~dave

[dedlfix]

Hi!

Zahlenwerte können auch als String übergeben werden. Probier mal alle relevanten Typen aus, ob sie sich als s übergeben lassen.
Welchen Nutzen hat es dann überhaupt hier etwas anderes zu verwenden?

Vermutlich kommt das von den typisierten Sprachen. Da kann die Funktion nur generische Typen fordern und man sagt ihr so, was wirklich kommt.

Das Problem ist nicht das Zusammenbauen des SQLs, sondern das übergeben der Paramter an bind().
->bind('s',$fieldValue['first'], $fieldValue['second'], ...)
Ich weiß nicht wie ich die verschiedene Anzahl an Parametern variabel machen könnte.

Ja, das ist eine etwas fiese Geschichte. Da sollen ja unbedingt Variablen und die auch noch per Referenz übergeben werden. Man kann da was mit call_user_func_array() hinbekommen. Bevor ich es lang und breit schreibe, so sieht das bei mir aus:

array_unshift($params, str_repeat('s', count($params)));  
if (!call_user_func_array(array($stmt, 'bind_param'), $params))  
  throw new DbException ...  
// ansonsten weiter mit execute

$params ist ein indexiertes Array, da stecken deine Parameter in der Reihenfolge, wie sie das SQL-Statement haben möchte. Am besten ist es, wenn die Query-Ausführung und damit auch das Binding in einer Funktion gekapselt ist. $params wird als Funktionsparameter übergeben. Die Funktion erhält nun eine Kopie des Arrays. Da das Binding mit Referenzen geschieht, reduzierst du eventuelle Nebenwirkungen, wenn du mit einer dedizierten Kopie arbeitest.

Das sieht jedenfalls noch recht einfach aus. Wenn du jedoch mehrfach exekutieren willst, wird es ungefähr so aufwendig wie beim nachfolgend beschriebenen Fetchen.

Ohne Prepared-Statement wär's kein Problem, aber dann müsste ich mich ja selber um den Kontextwechsel kümmern :-(

Die Frage ist, was am Ende aufwendiger ist. Das Problem des Bindings zu umgehen oder selbst behandeln. Richtig dämlich wird es nämlich erst beim Abfragen des Results. Da musst du nämlich auch wieder mit Binding arbeiten und du willst sicherlich eine variable Anzahl Felder abfragen können. Die erste Schwierigkeit hier ist, erstmal ein Array voller Referenzen zu erzeugen, das man bind_result() übergeben kann:

$values = array();  
foreach ($stmt->result_metadata()->fetch_fields() as $field)  
  $values[$field->name] =& $values[$field->name];  
  
if (!call_user_func_array(array($stmt, 'bind_result'), $values))  
  throw new DbException ...  
// ansonsten fetchen

Die Zeile

$values[$field->name] =& $values[$field->name];

sieht eigentümlich aus, aber das funktioniert und sogar ohne Notice. Beim Erstellen der Referenz findet kein Lesevorgang statt, weswegen die Notice keinen Grund zum Erscheinen hat.

Wenn du das Ergebnis gleich beim Fetchen ausgeben willst, ist mit einfachen Lesezugriffen auf $values alles bestens. Willst du es jedoch erst in einem Array sammeln (natürlich willst du das, denn du willst ja Datenabfrage und Geschäftslogik trennen), dann musst du die referenzierten Werte wieder dereferenzieren. Wenn du das so machst

$rows = array();  
while ($stmt->fetch()) {  
  $rows[] = $values;  
}

zeigt dir ein var_dump($rows); anschließend x-mal den letzten Datensatz, wobei die Elemente immer ein & zum Zeichen der Referenz davorstehen haben.

$rows = array();  
while ($stmt->fetch()) {  
  $row = array();  
  foreach ($values as $key => $value) // dereference bound fetched values  
    $row[$key] = $value;  
  $rows[] = $row;  
}

So wird ein Schuh draus. Und wie du siehst, ist das alles deutlich aufwendiger als Selbstmaskieren, mysqli::query() aufrufen und das zurückgegebene MySQLi_Result-Objekt zu befetchen. PDO ist bei Prepared Statemens übrigens deutlich anwenderfreundlicher.

Zudem werden AFAIK Prepared-Statements effizienter gecachet(?)

Da lies mal lieber die entsprechenden Kapitel im MySQL-Handbuch. Ein Prepared Statement muss zwar nur einmal geparst werden und kann dann beliebig oft exekutiert werden. Aber wenn du nur ein execute() pro Request verwendest, bringt dir das nichts, denn der nächste Request mit neuer Verbindung muss erneut prepare() aufrufen und parsen lassen.

Gute Idee, die Bezeichner zu maskieren, aber falsche Ausführung. Da sind gleich drei Fehler enthalten.
$mysqli->prepare('DELETE FROM '.str_replace('','``',$tableName).' WHERE id = ?');`
So dann richtig?

Ja. Ich würde dir noch sprintf() empfehlen, das sieht meines Erachtens übersichtlicher aus, weil das SQL-Statement zusammenhängend geschrieben werden kann:

$sql = sprintf('DELETE FROM `%s` WHERE id = ?', str_replace('`', '``',$tableName));  
$mysqli->prepare($sql);

Lo!