pc-world: Links am Ende werden nicht angezeigt

Hi, bei mir werden Links am Ende nicht angezeigt (im ausgegebenen HTML-Code auch nicht zu finden).

Mein Code (der Bug ist ganz unten zu finden):

<?php  
session_start();  
  
include "mysqlconfig.inc.php";  
  
echo '<link rel="stylesheet" type="text/css" href="../style.css">';  
  
//Var-Sicherheit  
if(isset($_GET['fid'])) { $fid = mysql_real_escape_string(htmlentities($_GET['fid'],ENT_QUOTES)); } else { echo 'Keine Forums-ID empfangen.'; exit; }  
if(isset($_GET['tid'])) { $tid = mysql_real_escape_string(htmlentities($_GET['tid'],ENT_QUOTES)); } else { echo 'Keine Thread-ID empfangen.'; exit; }  
  
//Lesen aus DB  
echo '<a href="newanswer_form.php?fid='.$fid.'&tid='.$tid.'">Neue Antwort</a><br><br>';  
  
 //Antworten  
$res = mysql_query("select * from ".$tableanswers." where fid=".$fid." AND tid=".$tid);  
  
//ausgeben  
while($row = mysql_fetch_array($res) or die(mysql_error())) {  
 $text = nl2br($row["text"]); //Zeilenumbrüche  
 echo '<fieldset style="background: gray; color: white;">  
        <legend><font color="chartreuse"><b>'.$row["user"].'</b> schrieb am '.$row["createdToShow"].':</font></legend>'.  
        $text.'</fieldset>';  
}  
  
echo '<a href="newanswer_form.php?fid='.$fid.'&tid='.$tid.'">Neue Antwort</a>  
      <br><a href="javascript:history.back();">Zurück</a>  
      <br><a href="showthreads.php?tid='.$tid.'">Zurück zu Threads</a>  
      <br><a href="..">Zur Startseite</a>'; //Diese Links werden nicht angezeigt  
?>
  1. Mahlzeit pc-world,

    Hi, bei mir werden Links am Ende nicht angezeigt (im ausgegebenen HTML-Code auch nicht zu finden).

    Kein Wunder:

    //ausgeben
    while($row = mysql_fetch_array($res) or die(mysql_error())) {

    Wenn mich meine PHP-Kenntnisse nicht trüben, wird diese Schleife ausgeführt, solange noch Arrays im Ergebnis enthalten sind (und solange die erste Bedingung wahr ist, wird die zweite gar nicht ausgewertet). Anschließend wird diese Bedingung falsch. Das wiederum bedeutet, dass PHP versucht, die zweite Bedingung auf Wahrheit zu überprüfen und dabei "stirbt". Da aber kein MySQL-Fehler vorliegt, bekommst Du auch keine Meldung.

    Damit bewahrheitet sich wieder einmal Zitat 1282: "die()" ist ebenso sehr eine Fehlerbehandlung, wie Suizid eine Heilmethode ist.

    MfG,
    EKKi

    --
    sh:( fo:| ch:? rl:( br:> n4:~ ie:% mo:} va:) de:] zu:) fl:{ ss:) ls:& js:|
    1. Danke, hab die() rausgelöscht - jetzt geht alles.

    2. echo $begrüßung;

      while($row = mysql_fetch_array($res) or die(mysql_error())) {
      Damit bewahrheitet sich wieder einmal Zitat 1282: "die()" ist ebenso sehr eine Fehlerbehandlung, wie Suizid eine Heilmethode ist.

      Das ist zwar richtig, aber unpassend, weil in dem Fall einfach kein Fehler vorliegt, den es zu behandeln gäbe. Das von mysql_fetch_array() zurückgegebene false ist ein ganz normaler und gewollter Zustand.

      Die eigentlich notwendig Fehlerbehandlung müsste am mysql_query() ansetzen und da glänzt sie durch völlige Abwesenheit.

      echo "$verabschiedung $name";

  2. echo $begrüßung;

    //Var-Sicherheit
    if(isset($_GET['fid'])) { $fid = mysql_real_escape_string(htmlentities($_GET['fid'],ENT_QUOTES)); } else { echo 'Keine Forums-ID empfangen.'; exit; }
    if(isset($_GET['tid'])) { $tid = mysql_real_escape_string(htmlentities($_GET['tid'],ENT_QUOTES)); } else { echo 'Keine Thread-ID empfangen.'; exit; }

    Was ist denn das für ein Unfug mit den geschachtelten mysql_real_escape_string(htmlentities())? Du behandelst hier in einem Aufwasch Werte für zwei völlig verschiedenen Kontexte.

    //Lesen aus DB
    echo '<a href="newanswer_form.php?fid='.$fid.'&tid='.$tid.'">Neue Antwort</a><br><br>';

    Abgesehen vom für diese Stelle unpassenden Kommentar, hast du hier einen der beiden Kontexte, nämlich HTML. Für den ist eine Behandlung gemäß MySQL-Strings-Kontext wenig sinnvoll.

    //Antworten
    $res = mysql_query("select * from ".$tableanswers." where fid=".$fid." AND tid=".$tid);

    Und hier ist der zweite Kontext. Für den wirkt sich die HTML-Behandlung im Allgemeinen eher störend aus, lassen sich doch keine sinnvollen Stringbearbeitungen mehr durchführen, wenn solcherart behandelten Werte in den Tabellen stehen.

    Wenn fid und tid Zahlen sein sollen, dann wäre eher eine Überprüfung auf Zahlen sinnvoll, beziehungsweise mit intval() eine erzwungene Konvertierung zu einer Zahl.

    Außerdem schützt dich deine obige Behandlung im Zusammenhang mit diesem Statement nicht die Bohne vor SQL-Injection. Angenommen der Wert von $tid wäre »42 OR 1«. Weder mysql_real_escape_string() noch htmlentities() erwirkt eine Änderung. mysql_real_escape_string() ist für Strings gedacht, also Werte, die im Statement zwischen "" oder '' zu stehen kommen sollen. Es ergibt sich damit ein Statement

    ... where fid=23 AND tid=42 OR 1

    Gemäß Operator-Rangfolge findet zuerst die AND-Verkknüpfung statt, dann die mit OR. Zur Verdeutlichung mal mit Klammern geschrieben ergäbe das

    ... where (fid=23 AND tid=42) OR 1

    Die beiden ersten Einschränkungen werden durch die Verknüpfung mit dem OR außer Kraft gesetzt.

    Vielleicht ist diese Stelle unkritisch, ergibt das doch "nur" eine Ausgabe aller Antworten, doch wenn du das auch so notiert hast, wenn es um beispielsweise Administrator-Funktionen geht, ...

    Glücklicherweise schützt sich MySQL selbst vor mehrfachen Statements, so dass sich kein

    ; DELETE FROM ...

    unterschieben lässt, doch auch ein SELECT-Statement lässt sich zur Abfrage sensibler Daten erweitern

    SELECT user, text, createdToShow FROM answers WHERE fid = 23 AND tid=42
      UNION SELECT user, passwort AS text, 0 AS createdToShow FROM user

    Damit hängt es den Inhalt der Tabelle user an die Ergebnismenge und so gelangen alle Passwörter (oder was auch immer davon in der Tabelle) gelandet ist zur Anzeige.

    Deine SQL-Injection-Lücke in diesem Fall kannst du schließen indem du die Eingabewerte für fid und tid mittels intval() garantiert in Zahlen umwandelst. Im Fehlerfall (also bei Nicht-Zahlen-Strings) gibt die Funktion 0 zurück. Alternativ müsstest du den mit mysql_real_escape_string() behandelten Stringwert auch in String-Begrenzer einrahmen ("" oder '').

    //ausgeben
    while($row = mysql_fetch_array($res) or die(mysql_error())) {
    $text = nl2br($row["text"]); //Zeilenumbrüche
    echo '<fieldset style="background: gray; color: white;">
            <legend><font color="chartreuse"><b>'.$row["user"].'</b> schrieb am '.$row["createdToShow"].':</font></legend>'.
            $text.'</fieldset>';
    }

    An dieser Stelle übergibst du wieder Werte in den HTML-Kontext. Erst dabei ist eine Behandlung gemäß der HTML-Regeln sinnvoll, nicht (wie ich vermute) bereits beim Eintragen in die DB.

    Die Sache mit dem die() ist ja bereits geklärt. Es fehlt noch eine Auswertung des Rückgabewertes von mysql_query() inklusive angemesser Reaktion. Das Script sterben zu lassen und dem Anwender den mysql_error()-Text zu präsentieren ist meist nicht die feine englische Art, geschweige denn, dass die dabei veröffentlichten Interna jemand anderes zu sehen bekommen sollte als der Entwickler. Auch der am Anfang zu sehende Abbruch mit »exit;« ist nicht besser.

    Vielleicht solltest du dir mal grundlegende Gedanken machen, wie du Scripte besser strukturieren kannst, so dass Eingabe, Verarbeitung und Ausgabe getrennte Bestandteile werden (EVA-Prinzip). exit und die() sind dann "verbotene" Dinge. Ein Script sollte nach einer vollständig ausgegebenen Seite enden. Fehlerzustände, so sie dem Anwender mitgeteilt werden sollen, finden darin ihren Platz und zerhackstücken nicht das Layout durch plötzlichen Abbruch. Außerdem sollte der Text einer sein, mit dem der Anwender etwas anfangen kann. Wenn beispielsweise die Datenbank defekt ist, kann er nichts dafür. Die genaue Fehlerursache ist nicht für seine Augen bestimmt, nur für die des Administrators.

    echo "$verabschiedung $name";