Benutzereingaben richtig überprüft?

[valenterry]

Hi,
Ich habe mir zu Lernzwecken ein kleines Gästebuch gemacht und wollte mal von euch wissen, ob die Validierung der Benutzereingaben richtig/vollständig ist.
Hier der Code:

if(!isset($name) || !isset($titel) || !isset($eintrag))
{
	die("Deine Eingaben sind unvollständig.");
}

if(!is_string($name) || !is_string($titel) || !is_string($eintrag))
{
	die("Deine Eingaben sind unvollständig.");
}

if(trim($name)=="" || trim($titel)=="" || trim($eintrag)=="")
{
	die("Es müssen alle Felder ausgefüllt werden.");
}


//Besondere HTML-Zeichen ersetzen.
$titel = htmlentities($titel);
$name = htmlentities($name);
$eintrag = htmlentities($eintrag);


//Zeilenumbrüche richtig darstellen.
$titel = nl2br($titel);
$name = nl2br($name);
$eintrag = nl2br($eintrag);


//Leerzeichen richtig darstellen.
$titel = str_replace(' ', '   ', $titel);
$name = str_replace(' ', '   ', $name);
$eintrag = str_replace(' ', '   ', $eintrag);


//Bei zu langen Wörtern einen Zeilenumbruch erzwingen, um das Layout zu bewahren.
$titel = wordwrap( $titel, 80, "\n", 1 );
$name = wordwrap( $name, 80, "\n", 1 );
$eintrag = wordwrap( $eintrag, 80, "\n", 1 );


//Vor SQL-injektion schützen.
$titel = mysql_escape_string($titel);
$name = mysql_escape_string($name);
$eintrag = mysql_escape_string($eintrag);

 

[LX]

Erstmal als allgemeiner Hinweis: Da du mit allen 3 Eingaben dasselbe machst, kannst du das auch in eine Funktion packen, der du dann eine der 3 Felder jeweils als Parameter übergibst. Fehlerausgaben solltest du nach Möglichkeit nu auch net mit die() machen, sondern einen Fehlerstring ausgeben an entsprechender Stelle. Ob is_string() nun nötig ist, darüber kann man auch streiten. Achja, und deine Auswertung sieht ein bisschen aus, als ob register_globals aktiviert sein muss, das würde ich auch ausbessern.

Etwas aufbereitet mit meinen Vorschlägen sähe das dann etwa so aus (ungetestet):

<?php

function prepare_input ( $string )
{
    if ( empty ( trim ( $string ) ) )
        return false;

    $string = htmlentities ( $string );
    $string = nl2br ( $string );
    $string = wordwrap ( $string, 80, ' ', 1 );
    $string = mysql_escape_string ( $string );

    return $string;
}

$name    = prepare_input ( $_POST [ 'name'    ] );
$titel   = prepare_input ( $_POST [ 'titel'   ] );
$eintrag = prepare_input ( $_POST [ 'eintrag' ] );

if ( $name === false || $titel === false || $eintrag === false )
{
    // Fehlerausgabe, aber bitte net mit die() ;)
}
else
{
    // in die Datenbank damit
}

?>

Vorteil dabei: die Funktion kannst du auslagern und von beliebiger Stelle aus aufrufen, um alle möglichen Benutzereingaben aufzubereiten... vielleicht willst du ja noch andere Formulare auf deiner Website auswerten. In puncto Sicherheit ist dabei aber an alles gedacht.

 

[valenterry]

Danke, das ist ne gute Idee.
Kann ich eigentlich die Funktion auch in eine Datei schreiben und diese dann mit include einbinden? Gibt es da etwas zu beachten?
Du meintest ja, ich soll kein die() benutzen, aber wieso nicht?
Wie soll ich denn stattdessen das Script abbrechen?
Und noch zu dem register_globals. Das ist bei mir (natürlich) nicht aktiviert. Verlangt das etwa eine von den verwendeten Funktionen?

 

[LX]

Original von valenterry
Kann ich eigentlich die Funktion auch in eine Datei schreiben und diese dann mit include einbinden?

Das meinte ich mit "auslagern".

Du meintest ja, ich soll kein die() benutzen, aber wieso nicht?
Wie soll ich denn stattdessen das Script abbrechen?

Abbrechen gar nicht. Gute Software bricht nicht einfach ab, wenn irgendwas passiert, womit sie nicht klarkommt, sondern arbeitet weiter und gibt an passender Stelle eine schön formatierte Fehlermeldung aus. die() beendet rabiat jegliche Ausgaben, allerdings wäre es natürlich praktischer, du würdest deine Seite ausgeben wie immer, eventuell sogar nochmal das Formular anzeigen und einfach nur eine Fehlermeldung dabei stehen haben, in der dem Nutzer dann erklärt wird, was er falsch gemacht hat.

Und noch zu dem register_globals. Das ist bei mir (natürlich) nicht aktiviert. Verlangt das etwa eine von den verwendeten Funktionen?

Nein, aber deine unmittelbare Abfrage auf isset($name) statt z.B. isset($_POST['name']) suggeriert, dass du mit register_globals=on rechnest.