nicht angemeldet
echo $begrüßung;
Die DB enthält folgende Felder:
id
buchstabe (zur Selektion nach Anfangsbuchstabe)
Dieses Feld ist im Grunde genommen überflüssig, da sich dessen Inhalt beim Abfragen leicht ermitteln lässt. Doppelte Datenhaltung sollte man vermeiden, wenn es nicht triftige Gründe dafür gibt. Du wirst ja wohl kaum einen Ansturm erwarten, bei dem selbst diese kleine Rechenoperation unvertretbar viel Zeit verbraucht. Mein Vorschlag: Nutze von den MySQL-Stringfunktionen SUBSTRING()/SUBSTR().
begriff (Ausgabe des jew. Begriffs in der Tabelle usw.)
url
definition
wort (entspricht im Grunde dem Feld "Begriff" allerdings hab ich hier Umlaute o.ä. durch ae, ue usw. ersetzt und überlange Worte abgekürzt, damit es keine Probleme in der URL bzw. beim Verlinken gibt; weiß nicht ob das sein muss, erschien mir aber sinnvoll).
Gibt es nicht im Prinzip nicht, wenn man beachtet, dass Daten immer kontextspezifisch kodiert werden, wenn man sie in einen bestimmten Kontext bringen möchte. In dem Fall bietet sich an, eine URL-Kodierung zu verwenden.
<?php
#######
#Schutz vor SQL-Injections
#######if(get_magic_quotes_gpc() == false)
{
$buchstb = addslashes($_GET['buchstb']);
$wort = addslashes($_GET['wort']);
}
else
{
$buchstb = $_GET['buchstb'];
$wort = $_POST['wort'];
}
Magic Quotes und addslashes() soll vor SQL-Injection schützen, wie ja auch dein Kommentar aussagt. Doch ist es weder sinnvoll, generell addslashes() anzuwenden, noch für den konkreten Fall MySQL ausreichend, weil einfach zu wenig Zeichen berücksichtigt werden. Wie bereits erwähnt, sollen Daten, wenn man sie in einen bestimmten Kontext bringen möchte, kontextspezifisch kodiert werden. Dies soll aber erst kurz bevor die Daten in diesen Kontext übergeben werden geschehen, und intern sollte möglichst immer mit Rohdaten gearbeitet werden. Angenommen du möchtest die eingegebenen Daten am Bildschirm anzeigen und Magic Quotes hat zugeschlagen, zeigt es statt O'Conner nun O'Conner an. Was macht der unerfahrene Programmierer? Er wendet an der Stelle stripslashes() an (und vergisst auch gern noch htmlspecialchars()) und macht die ganze Sache unübersichtlicher. Das bewährte EVA-Prinzip (1. Eingabe, 2. Verarbeitung, 3. Ausgabe) gilt auch für Daten.
- Entgegennehmen und Transportsicherung entfernen
- Verarbeitung
- Ausgeben, dabei Transportsicherung hinzufügen
Mein Vorschlag: Magic Quotes deaktivieren (wird sowieso mit PHP6 gestorben sein) oder, falls nicht möglich, deren Auswirkungen am Scriptanfang rückgängig machen. Die MySQL-Maskierung übernimmt mysql_real_escape_string() und zwar erst dann, wenn der SQL-Befehl zusammengebaut wird oder unmittelbar davor. Dabei sollte aber nicht der Inhalt der Variablen geändert werden, mit denen man später noch weiter rechnen möchte.
Noch ein Punkt ist, dass du einfach auf Inhalte von $_GET/$_POST zugreifst, ohne zu prüfen, ob diese überhaupt vorhanden sind.
Mein Vorschlag: Prüfe mit isset() auf Vorhanden sein und initialisiere die Variable ansonsten mit einem Default-Wert. Verwende außerdem ein auf E_ALL gestelltes error_reporting beim Entwickeln von PHP-Scripten, da dir damit Hinweismeldungen auf Zugriffe auf nicht initialisierte Variablen nicht vorenthalten werden. Prüfe deine Scripte auf Schwachstellen, indem du auch mal gezielt völlig unerwartete Werte übergibst (z.B. HTML-Code in Benutzereingaben, ' und "-Zeichen). Auch vom Script selbst erzeugte Ausgaben, wie Links mit Parameteranhang sind nicht gegen Missbrauch sicher.
echo "<h1>Lexikon</h1>
<b>$buchstb</b>";
Da haben wir es schon, das vergessene htmlspecialchars() zum Entschärfen bzw. Transportsichern von Daten, die in den HTML-Kontext gebracht werden sollen. Das ist eine Einfallstelle für Code-Injection. Wenn jemand statt eines Buchstaben HTML- und/oder clientseitigen Scriptcode übergibt, gibts du das einfach so aus. Außerdem hast du doch nur einen einzigen Buchstaben erwartet. Du solltest prüfen, dass du auch nur einen Buchstaben übergeben bekommst. Das geht auch indem du einfach nur den ersten Buchstaben z.B. mit substr() ausschneidest. Diese Eingabedatenprüfung sollte gemäß EVA gleich nach Entfernen der Transportsicherung erfolgen und nicht quer übers Script verstreut.
Mein Vorschlag: Gleich nach der Magic-Quotes-Rück-Behandlung prüfen, dass Buchstabe nur ein Zeichen enthält oder das erste ausschneiden.
Es ist beim Ausschneiden besser, substr() statt eines direkten Zeichenzugriffs zu verwenden, weil $string{0} bzw. $string[0] bei einem Leerstring ins Leere greift und eine Hinweismeldung ergibt.
$query = "SELECT begriff, url FROM lexikon WHERE buchstabe = '$buchstb'" or die(mysql_error());
Das die() wird nie aufgerufen werden, da die Zuweisung in diesem Fall stets true ergibt. Und es gehört dort auch gar nicht hin, sondern, wenn überhaupt, hinter die mysql_*-Funktionen.
$result = mysql_query($query);
Also hier dran. Allerdings ist das Sterbenlassen eines Scripts mit Ausgabe einer MySQL-Fehlermeldung zwar für das Entwickeln hilfreich, jedoch für den Endanwender sehr benutzerunfreundlich. Es ist ja nicht sein Verschulden, dass der Fehler auftritt, deswegen sollte man ihm auch nicht die Fehlermeldung als Reaktion auf sein Handeln präsentieren.
Mein Vorschlag: Fehlermeldungen in einem Logmechanismus eintragen (Email an den Admin senden ist der einfachste Fall), dem Benutzer eine Tröstmeldung anzeigen und das Script ordnungsgemäß beenden. Natürlich müssen dann die Code-Teile, die auf die fehlgeschlagenen Operation aufbauen, umgangen werden (Stichwort: if-then-else).
Übrigens geben (fast) alle mysql_*-Funktionen im Fehlerfall false zurück statt eines Ressourcen-Handles, wie es von den meisten nachfolgenden mysql_*-Funktionen benötigt wird. Fängt man das nicht ab, beschwert sich die PHP mit einer Warnung.
Den Datenbank-Connect hast du in einer Include-Datei versteckt. Zum Thema Fehlerbehandlung gilt da auch das eben gesagte.
Prüf doch mal, wie dein Script reagiert, wenn du falsche Zugangsdaten verwendest oder auch eine falsche/nicht vorhandene Datenbank auswählst und überlege aus Anwendersicht, ob dein Script angemessen reagiert.
Positiv hervorzuheben ist, dass du dein Script kommentierst. Achte dabei darauf, den Sinn der Codestelle zu kommentieren, nicht nur was sie macht. Letzteres sollte sich aus dem Code ergeben.
Beispiel:
$i++; // hier wird $i um eins erhöht
So ein Kommentar ist sinnlos, weil das offensichtlich ist. Kommentiert werden sollte, warum hier $i erhöht wird.
Ebenfalls lobend erwähnenswert ist, dass du dich bemühst, deinen Code ordentlich zu formatieren, auch wenn dir das an einigen Stellen nicht gelungen ist. Aber vielleicht war das auch nur ein Copy'n'Paste-Fehler.
Was mir selbst besonders missfällt (ohne überhaupt zu wissen, wie gut/schlecht das Skript ist), ist der letzte Teil ab der Ausgabe der Begriffserklärung. Ist es überhaupt sinnvoll, dass alles in eine Skript zu stecken?
Das Script ist doch noch recht übersichtlich. Es scheint mir nicht sinnvoll, hier noch weitere Code-Dateien zu erzeugen, das erhöht nur den Pflegeaufwand.
Wenn du Code-Teile auslagerst, achte darauf, dass sich diese Dateien auch zu Fuß aufrufen lassen, wenn sie nicht gerade außerhalb des Documentroots des Webservers abgelegt sind. Zugriffsbeschränkende Maßnahmen in der Webserverkonfiguration können hilfreich sein. Ebenfalls nützlich ist es, wenn die ausgelagerten Teile (Include-Dateien) bei Direktaufruf keinerlei Ausgabe vornehmen. Ausgaben und andere Rückgabewerte sollten von Funktionen erzeugt werden, die man im Hauptprogramm aufruft. Weniger bekannt ist, dass man die Include-Datei auch quasi als eine Funktion behandeln kann. Man kann mit return etwas zurückgeben, was man im Hauptprogramm entgegennehmen kann.
include.php:
<?php
$foo = <<<HTMLTEXT
<div id="footer">...</div>
HTMLTEXT;
return $foo;
// das abschließende ?> kann auch weggelassen werden. Das verhindert ungewollte Whitespace-Zeichen, die sich gern nach ihm einschmuggeln.
in der main.php:
echo include 'include.php';
oder:
$bla = include 'include.php';
Teile, die keinen PHP-Code enthalten, bindet man auch besser mit readfile() statt include ein. Der Dateiinhalt wird von readfile() nicht geparst. Das ist vor allem dann von Vorteil, wenn es jemandem gelungen sein sollte, einen ungewünschten Dateinamen/URL unterzuschieben, kann doch dann der darin enthaltene PHP-Code auf dem Server keinen Schaden anrichten.
echo "$verabschiedung $name";
MudGuard