dedlfix: Dropdown Auswahl in Datenbank speichern

Beitrag lesen

Tach!

<form id="form1" name="Dropdown Auswahl" method="post" action="<?php echo $PHP_SELF; ?>">
PHP_SELF ist böse. Nimm stattdessen $_SERVER['SCRIPT_NAME'].

PHP_SELF ist nicht böse. Böse ist, wenn man manipulierbare Werte (hier durch Anhängen von beliebig geformten Querystrings) ohne Kontextwechselbeachtung ausgibt. Ein htmlspecialchars() drumherum, wenn es nach HTML ausgegeben wird, und PHP_SELF ist genauso ungefährlich wie alles andere. Am besten ist in dem Fall aber, wenn man action="" notiert, dann wird das Formular auch an sich selbst zurückgesendet.

[HTML-Krams]
    <?php
        session_start();

Das funktioniert so nicht, zumindest nicht, wenn die Session-ID per Cookie übertragen werden soll. Für das Cookie muss PHP einen Set-Cookie-HTTP-Header senden, und das kann es nicht mehr, wenn bereits eine Ausgabe erfolgt ist - hier in Form von HTML-Code vor dem ersten <?php.

$select = mysql_select_db($dbname, $verbindung) or die ("Datenbank konnte nicht ausgew&aumlhlt werden");

if (isset ($select)&&$select!=""){
    $select=$_POST ['Lehrer'];
  }


> Du versuchst eine Datenbank zu wählen und schreibst den Ausgang dieses Versuches in eine Variable. Diese wird entweder true oder false enthalten. Dann prüfst du, ob diese boolsche Variable existiert und ob sie ungleich einem leeren String ist. Ist dass der Fall, überschreibst du diese Variable mit einem Element aus dem Post-Array. Mir ist absolut nicht klar, was das bedeuten soll; abgesehen davon ist bei dem Ansprechen des Post-Arrays ein Leerzeichen zuviel.  
  
Aus syntaktischer Sicht stört das Leerzeichen nicht, es ist aber eher üblich dort keins zu verwenden. Üblich ist aber \_leider\_ die "or die()"-Fehlerbehandlung. Die ist sehr unschön, weil sie ein halbes Dokument hinterlässt und demjenigen, der sie zu sehen bekommt überhaupt nichts nützt. Besser ist im Falle eines Falles, die Fehlerbedingung abzufragen und das Script auf einem alternativen Weg kontrolliert zu Ende zu bringen. Der Admin sollte einerseits über den Fehler unterrichtet werden, und dem Anwender sollte man eine aus seiner Sicht akzeptable Antwort geben. Dass die Datenbank kaputt ist, intressiert ihn dabei überhaupt nicht, ebensowenig wie irgendwelche anderen internen Details.  
  
Die Sache mit dem if ist im Zuge des vorangehenden "or die()" sowieso komplett fehl am Platz. $select ist immer true, denn wenn mysql\_select\_db() ein false liefert, stirbt ja das Script. Und das Umkopieren von $\_POST['Lehrer'] nach $select ist nicht nur deswegen sinnlos, weil $select später nicht mehr verwendet wird. Andrerseits sollte das if-Konstrukt korrigiert und noch gemäß dem vorhergehenden Absatz erweitert werden, sowie das "or die()" weggelassen werden - und dies sollte an allen Stellen passieren, an denen "or die()" auftaucht.  
  
Zudem muss man Umlaute nicht verunstalten, und schon gar nicht indem man dafür unvollständige HTML-Entitys verwendet. Auf alle Fälle sollte man jedoch eine Angabe zur verwendeten Zeichkodierung machen. (Siehe zum Beispiel [Zeichencodierung für Anfänger](http://www.w3.org/International/questions/qa-what-is-encoding.de.php)).  
  

> ~~~php

while($row_list=mysql_fetch_assoc($list)){  

> ?>  
>   <option value="<?php echo $row_list['ID']; ?>"<?php if($row_list['ID']==$select){ echo "selected"; } ?>>  
>     <?php echo $row_list['Name'];?>  
>   </option>  
> <?php  
> }

Das ist ein sehr waghalsiges Konstrukt, da es den Umgang mit Kontextwechseln unnötig erschwert. Warum gibst du die relevanten Zeilen nicht mit PHP-internen Mitteln, wie echo oder print aus?

Bitte? Da wird doch alles mit echo ausgegeben, was mit echo ausgegeben werden muss. Und man kann (und sollte) da sehr wohl noch die für die Kontextwechselbeachtung notwendigen Funktionsaufrufe unterbringen. Es ist nicht notwendig, den kompletten HTML-Teil über String-verkettung zusammenzubauen und ihn erst dann auszugeben.

mysql_query("INSERT INTO auswahl (Lehrer) VALUES ($_POST ['Lehrer'])");
Jedes Mal, wenn deine Seite aufgerufen wird, schreibst du in eine Tabelle deiner Datenbank einen Datensatz, in dem die Spalte Lehrer mit dem entsprechenden Element des Post-Arrays gefüllt wird. Du prüfst weder, ob überhaupt ein Post-Array existiert, noch ob ein Element mit dem Schlüssel 'Lehrer' existiert. Demzufolge müsstest du entweder einen Fehler bekommen, oder haufenweise leere Datensätze in deiner Tabelle haben.

Abgesehen vom wieder nicht beachteten Kontextwechsel (diesmal in Richtung SQL, was eine SQL-Injection-Lücke darstellt), ist die gesamte Programmstruktur reichlich fragwürdig. Schon die einfachsten Programmiermuster wie EVA-Prinzip und Affenformular können eine deutliche Verbesserung in die Struktur bringen, womit dann auch die überflüssigen INSERTs von selbst verschwinden.

Ich habe die Übertragung bereits mit mysql_query("INSERT INTO auswahl (Zeit) VALUES ('".$_POST ['Zeit']."')"); versucht, doch leider klappt das nicht.
"Klappt nicht" ist keine Fehlerbeschreibung.
Mein Tipp: Lass dir in deinem Skript alle Fehler, Warnungen und Infos anzeigen, die PHP generiert. Das geht per error_reporting(E_ALL);.

Dazu noch display_errors einschalten, sonst sieht man selbst die nicht, die jetzt schon hätten zu sehen sein müssen (zum Beispiel wegen session_start()). Die Fehlermeldungen sind aber nur für den Entwicklungsprozess als Bildschirmausgabe sinnvoll. In einem echten produktiv eingesetzten System müsste man sich über das Logging der Meldungen noch ein paar andere Gedanken machen.

Das sind meist ganz vernünftige Meldungen, die du beheben kannst, aber das scheint nicht wirklich dein Problem zu sein. Dein Problem ist eher ein strukturelles. Ich denke nicht, dass du weißt, was das Skript genau macht. Ich empfehle dir ein Konzept aufzumalen, _was_ dein Skript tun soll und _wann_ es es tun soll. Erst danach ist es sinnvoll an die Implementierung zu gehen.

Und dabei die EVA-Affen berücksichtigen.

dedlfix.