dedlfix: Datenbankeintrag per Formular

Beitrag lesen

echo $begrüßung;

$connectionid = mysql_connect($db_host, $db_user, $db_pass);
$db_selected = mysql_select_db($db_name, $connectionid);

Du bist hier noch am Scriptanfang und weißt gar nicht, ob du überhaupt eine Datenbankverbindung brauchst. Im Falle eines Fehlers nämlich drehst du ja noch eine Formularausfüllrunde ohne das DBMS zu behelligen. Trotzdem öffnest du die Verbindung. Und ohne zu prüfen, ob das geklappt hat, versuchst du gleich darauf die zu verwendende Datenbank zu selektieren. Bau doch mal einen Tippfehler in die Zugangsdaten ein oder fahr den MySQL-Server runter. Dann siehst du, dass das Selektieren der Datenbank einen Folgefehler bringt. Erst viel weiter unten prüfst du, ob die Verbindung erfolgreich war.

Verbesserungsvorschläge: Prüfe direkt nach dem Verbindungsaufbau, ob er von Erfolg gekrönt war oder nicht. Prüfe beim Entwickeln immer auch den Schlechtfall, beispielsweise mit falschen Zugangsdaten. Gestalte die Vorgänge in deinem Ablauf so, dass sie nicht auseinandergenommen werden. Die DB-Verbindung brauchst du erst nach der erfolgreichen Prüfung der Eingabedaten, also erzeuge sie auch erst dann.

if (isset ($_POST['Datum'], $_POST['Zeit'], $_POST['Ort'], $_POST['Adresse'])) {
if (empty ($_POST['Datum']) || empty($_POST['Zeit']) || empty($_POST['Ort']) || empty($_POST['Adresse'])) {

isset() und empty() ist doppelt gemoppelt. Wenn du empty() verwendest, kannst du die isset()-Prüfung weglassen.

  $form\_error['Input'] = "Bitte alle Felder ausfüllen";  
  die(); // da sonst das script weiter durchlaufen wird (auch der DB-Eintrag)  

Haste ja selbst schon gemerkt.

// alle Slashes in superglobalen arrays löschen

Diesen Vorgang solltest du ganz am Scriptanfang einfügen. Eingabedaten in eine brauchbares Format (Rohform) zu bringen, sollte der erste Schritt sein (EVA-Prinzip). Und außerdem solltest du das gemäß dem Beispiel im PHP-Handbuch einfügen. Das berücksichtigt nämlich auch verschachtelte Strukturen.

  while (list($k,$v) = each($in)) {  
  	foreach ($v as $key => $val) {  

Seit es foreach gibt, braucht man das Gespann while-list-each nicht mehr.

if (count($form_error) == 0) {

Aufgrund PHPs automatischer Typkonvertierung brauchst du das Ergebnis von count() nicht noch mit einer Zahl zu vergleichen. Du kannst es gleich als boolschen Wert ansehen, allerdings mit umgekehrter Logik in dem Fall.

if (! count($form_error)) {

Und noch viel besser: Du kannst auch direkt auf das Array prüfen, denn ein leeres Array ergibt auch false.

if (! $form_error) {

Außerdem hast du vergessen, $form_error definitiv anzulegen. Wenn kein Fehler auftritt, schreibst du nichts rein, wodurch es nicht implizit angelegt wird. Ein Lesezugriff auf etwas nicht vorhandenes ist normalerweise ein Fehler. PHP informiert darüber aber nur, wenn man es explizit darum bittet: mit auf E_ALL gestelltem error_reporting. Das ist beim Entwickeln immer empfehlenswert, bekommt man doch damit auch fehlerhafte Zugriffe aufgrund von Schreibfehlern angezeigt. Bevor du also die Prüfungen beginnst:

$form_error = array();

  	if (!$connectionid) {  
  		die('Verbindung nicht möglich : ' . mysql\_error());  

Wie gesagt, schreib die mysql_connect-Zeile direkt vor diese beiden Zeilen und ...

  	if (!$db\_selected) {  

die ('Kann Datebank nicht benutzen : ' . mysql_error());

die mysql_select_db-Zeile hier davor.

Außerdem ist es besser, statt das Script sterben zu lassen und dabei noch alle Welt mit technischen Details zu behelligen, wenn du im Fehlerfall einen alternativen Weg gehst (if-else) und damit zum einen das Script ordentlich beendest und zum anderen die Fehlermeldung nur an den Administrator leitest.

  	else {  
  	$sql = "INSERT INTO agenda (Datum, Zeit, Ort, Adresse) VALUES ('".$\_POST['Datum']."', '".$\_POST['Zeit']."', '".$\_POST['Ort']."', '".$\_POST['Adresse']."')";  

Hier fehlt die kontextgerechte Behandlung der Werte. Wenn jemand als Ort D'dorf eingibt, bekommst du nur einen Syntaxfehler. Wenn jemand diese SQL-Injection-Lücke gezielter ausnutzt, ...
mysql_real_escape_string() heißt das Stichwort.

// Ausgabe des Formulars
include ('htmlform.php');

Nebenbei: include ist keine Funktion. Die Klammern kannst du weglassen. (Die Anführungszeichen müssen aber bleiben.)

while($r = mysql_fetch_assoc($q)) {
$arr[] = $r;

Auch hier wieder: Vor der Schleife sollte $arr explizit als leeres Array angelegt werden. Wenn es nämlich nichts zu Fetchen gibt, wird $arr nicht angelegt und ...

print_r($arr);

... auch wenn das hier nur eine Kontrollausgabe ist, greift in dem Fall auf etwas nicht vorhandenes zu.

[code lang=php]<form action="<?php htmlspecialchars($_SERVER['PHP_SELF'])?>" method="post">

Hier fehlt ein echo.

  	<li><label for="Datum">Datum:</label><input type="text" name="Datum" id="<?php if (isset($form\_error[Datum])){echo 'fehler';} ?>" class="text" /></li>  
  	<li><label for="Zeit">Zeit:</label><input type="text" name="Zeit" id="Zeit<?php if (isset($form\_error[Zeit])){echo 'fehler';} ?>" class="text" /></li>  

Eine id muss ein eindeutiger Wert sein. Im Falle mehrerer Fehler vergibst du die ID "fehler" mehrmals. Außerdem brauchst du die id eigentlich um mit <label for="..."> zusammenzuarbeiten. Besser ist es class zu verwenden. Dass du es hier schon mit "text" belegt hast, macht nichts, class kann man mehrere Werte durch Leerzeichen getrennt zuweisen.

Und dann möchtest du vielleicht das value-Attribut der input-Elemente mit dem Wert aus der Eingabe vorbelegen, so dass im Falle eines Fehlers in einem Feld der Anwender nicht alles noch einmal einzugeben hat.

Nun hab ich aber noch mehrere Probleme, bei denen ich nicht weiterkomme. Gerne würde ich das Schritt für Schritt durchgehen, weil ihr sonst wohl gleich wieder mit umfassenden Antworten auf mich einschlagt von welchen ich nur knapp 1% verstehe.

Tja, um meine Antwort kürzer ausfallen zu lassen, fand ich zu wenig Gründe.

Und bitte antwortet so einfach wie möglich, sonst komm ich bei euren Antworten nicht nach.

Wenn du etwas nicht verstehst, frag bitte konkret nach. Erläuter auch bitte kurz, inwieweit du es schon verstanden hast beziehungsweise was genau das Verständnisproblem ist.

Problem 2: Die Daten können zwar nur von einer Person eingegeben werden und die weiss wie sie sie eingeben muss, aber dennoch möchte ich die Eingaben überprüfen und in ein für die Datenbank sinnvolles Format bringen (sofern es nicht bereits so ist.) Wie kann ich also überprüfen ob eine gültige Zeit xx:xx und ein gültiges Datum xx.xx.xxxx oder xxxx-xx-xx  (--> für die Datenbank umwandeln in xx.xx.xxxx) eingegeben wurde?

MySQL hätte das Datum gern im Format JJJJ-MM-TT (oder als date()-Format-String: Y-m-d).

Es gibt (wie immer) mehrere Wege, so auch für eine solche Prüfung. Zum einen könntest du das Format auf syntaktische Korrektheit prüfen. Bei einer Uhrzeit wäre das

optionale Ziffer - Ziffer - Doppelpunkt (oder anderes Trennzeichen) - Ziffer - Ziffer

Für die Syntax-Prüfung kann man einen Regulären Ausdruck verwenden. Dann gibt es aber immer noch die Möglichkeit, ungültige Werte einzugeben wie 25:90. Eine Uhrzeit zu prüfen ist ja noch relativ einfach. Man trennt die Angabe in Einzelwerte und vergleicht auf das Einhalten der Grenzen (für Stunden, Minuten und Sekunden). Prinzipiell kann man das bei einem Datum genauso machen, allerdings ist die Prüfung hier komplexer. Aber PHP macht es einfach, es gibt die Funktion checkdate().

Die zum Prüfen erzeugten Einzelwerte fügst du bei Fehlerfreiheit mit dem korrekten Trennzeichen so zusammen, wie MySQL sie haben will und fertig ist.

Eine Alternative ist, mit strptime() die Einabe zu analysieren. Hier solltest du aber mit ein paar Tests auch unsinniger Eingaben Erfahrungen sammeln, wie sich strptime() verhält und ob dir das so gefällt.

Außerdem gibt es noch strtotime(), das in allen möglichen Werten was Brauchbares zu finden versucht und das vorzugsweise bei englischer Notation. Für deutsche Benutzereingaben würde ich diese Funktion nicht verwenden, dann schon eher strptime().

echo "$verabschiedung $name";