zwerg: Datenbank updaten

Glück auf!

Erstmal möchte ich mich schonmal für den langen Code, der jetzt kommt entschuldigen. Leider weiß ich den gerade nicht sinnvoll zu kürzen, ohne möglicherweise relevante Informationen vorzuenthalten.

Ich habe eine Datenbank mit zwei Tabellen (blog und guestbook). Ich habe nun für die Tabelle Blog ein Script, welches je nach Inhalt der Variablen "$status" unterschiedliche Aktionen durchführen soll.

  
<?php  
if ($status == "Vorschau") {  
 echo "<h3>", $datum, "</h3><h4>Eintrag:</h4><p>", format($eintrag), "</p><h4>Kurzeintrag:</h4><p>", format($kurzeintrag), "</p>";  
}  
  
elseif ($status == "Eintragen") {  
 $query = "INSERT INTO blog SET  
         datum = now(),  
         eintrag = '".addslashes($eintrag)."',  
         kurzeintrag = '".addslashes($kurzeintrag)."'  
         ";  
 $sql = mysql_query($query) or die(mysql_error());  
 unset($status,$eintrag,$kurzeintrag);  
}  
  
elseif ($status == "Bearbeiten") {  
 $query = "UPDATE blog SET  
         eintrag = '$eintrag', kurzeintrag = '$kurzeintrag' WHERE id = $id";  
 $sql = mysql_query($query);  
 unset($id, $datum, $eintrag, $kurzeintrag);  
}  
  
elseif ($status == "edit") {  
 $query = "SELECT * FROM blog WHERE id = $id";  
 $sql = mysql_query($query) or die (mysql_error());  
 $ds = mysql_fetch_object($sql);  
  $id = $ds -> id;  
  $datum = $ds -> datum;  
  $eintrag = $ds -> eintrag;  
  $kurzeintrag = $ds -> kurzeintrag;  
}  
  
elseif ($status == "delete") {  
 $query = "DELETE FROM blog WHERE id = $id";  
 $sql = mysql_query($query);  
 unset($id, $datum, $eintrag, $kurzeintrag);  
}  
  
?>  

Das Problem tritt nun auf, wenn ich die Blogeinträge/-kurzeinträge bearbeiten will. Das klappt ab und an, aber meist nicht. Wann dieser "ab und an"-Fall eintrifft, kann ich mir/euch nicht erklären.

Wenn ich einen Eintrag bearbeiten will, gehe ich wie folgt vor:

1. Ich lasse mir alle Einträge anzeigen und klicke dann beim entsprechenden Eintrag auf ein Formularbutton, der den Status "edit" übergibt.

  
<form action="blog_admin.php" method="post">  
 <input type="image" src="../bilder/layout/edit.gif" name="status" value="edit" />  
 <input type="image" src="../bilder/layout/delete.gif" name="status" value="delete" />  
 <input type="hidden" name="id" value="<?php echo $id ?>" />  
</form>  

2. Dann wird mir der betroffene Datensatz in dem nachfolgendem Formular angezeigt:

  
<form action="blog_admin.php" method="post">  
 <input type="hidden" name="id" value="<?php echo $id ?>" />  
 <input type="hidden" name="datum" value="<?php echo $datum ?>" />  
 <h4>Eintrag:</h4>  
 <p><textarea cols="75" rows="10" name="eintrag"><?php echo htmlentities($eintrag) ?></textarea></p>  
 <h4>Kurzeintrag:</h4>  
 <p><textarea cols="75" rows="5" name="kurzeintrag"><?php echo htmlentities($kurzeintrag) ?></textarea></p>  
 <p><input type="submit" name="status" value="Vorschau" />  
<?php if (!empty($id)) { ?>  
 <input type="submit" name="status" value="Bearbeiten" />  
<?php } else { ?>  
 <input type="submit" name="status" value="Eintragen" />  
<?php } ?>  
 </p>  
</form>  

3. Bis 2. klappt alles wie gewollt. Klicke ich nun jedoch auf "Bearbeiten", werden die Änderungen mal übernommen, mal nicht.

Ich weiß, dass das o. g. Script eine Zumutung ist. Das macht es mir wohl auch so schwer den Fehler zu finden. Aber das war irgendwann mal nen ganz süßes kleines Script, was ich nach und nach angepasst habe, bis das da oben draus geworden ist. Wäre wirklich sehr nett, wenn sich trotzdem jmd. darin versucht, dort durchzublicken.

An alle die es versuchen oder überhaupt erst bis hier gelesen haben schonmal ein Dankeschön.

Freundliche Grüße

zwerg Alex

P.S.
Ich hoffe ihr versteht, dass ich das obige Script aus Sicherheitsgründen nicht verlinken kann. Ich hoffe aber alle relevanten Angaben gemacht zu haben. Ansonsten reiche ich die natürlich gerne nach.

  1. Hi,

    Ich weiß, dass das o. g. Script eine Zumutung ist.

    es ist ein bisschen rätselhaft. Erst mal wäre es wichtig zu wissen, wann und wie die Variablen wie z.B. $status initialisiert werden. Dann scheinst Du beim INSERT noch zu wissen[1], wie man Daten in ein SQL-Statement übergibt, beim UPDATE jedoch hast Du dieses Wissen schon wieder verlernt. An keiner Stelle scheinst Du Dir über Sicherheit Gedanken zu machen, SQL-Injection ist offenbar ein Fremdwort.

    Das macht es mir wohl auch so schwer den Fehler zu finden.

    Und was genau hat es Dir so schwer gemacht, ihn zu beschreiben?

    Cheatah

    [1] Nun ja, oder zu ahnen. addslashes() ist keine Datenbank-bezügliche Kodierung und damit die falsche Funktion, um Werte in einen Datenbank-Kontext zu bringen - sie macht allenfalls zufällig das Richtige.

    --
    X-Self-Code: sh:( fo:} ch:~ rl:° br:> n4:& ie:% mo:) va:) de:] zu:) fl:{ ss:) ls:~ js:|
    X-Self-Code-Url: http://emmanuel.dammerer.at/selfcode.html
    X-Will-Answer-Email: No
    X-Please-Search-Archive-First: Absolutely Yes
    1. Glück auf Cheatah!

      Erstmal danke für deine Antwort.

      es ist ein bisschen rätselhaft. Erst mal wäre es wichtig zu wissen, wann und wie die Variablen wie z.B. $status initialisiert werden.

      Entschuldige. Das habe ich in der Tat vergessen anzugeben:

        
       $datum = PostVar('datum');  
       $eintrag = PostVar('eintrag');  
       $kurzeintrag = PostVar ('kurzeintrag');  
       $status = PostVar('status');  
              $id = PostVar('id');  
      
      

      Dann scheinst Du beim INSERT noch zu wissen[1], wie man Daten in ein SQL-Statement übergibt, beim UPDATE jedoch hast Du dieses Wissen schon wieder verlernt. An keiner Stelle scheinst Du Dir über Sicherheit Gedanken zu machen, SQL-Injection ist offenbar ein Fremdwort.

      Das Script liegt in einem mit .htaccess geschützten Ordner, auf welchem nur ich Zugriff habe. Deswegen habe ich die Sicherheitsaspekte etwas vernachlässigt.

      Das macht es mir wohl auch so schwer den Fehler zu finden.

      Und was genau hat es Dir so schwer gemacht, ihn zu beschreiben?

      Das keine Fehlermeldung kommt und ich nicht weiß, wann er das Update ausführt und wann nicht.

      [1] Nun ja, oder zu ahnen. addslashes() ist keine Datenbank-bezügliche Kodierung und damit die falsche Funktion, um Werte in einen Datenbank-Kontext zu bringen - sie macht allenfalls zufällig das Richtige.

      Das werde ich abändern. Danke für den Hinweis.

      Freundliche Grüße

      zwerg Alex

      1. Hallo Alex,

        es ist ein bisschen rätselhaft. Erst mal wäre es wichtig zu wissen, wann und wie die Variablen wie z.B. $status initialisiert werden.

        Entschuldige. Das habe ich in der Tat vergessen anzugeben:

        $datum = PostVar('datum');
        $eintrag = PostVar('eintrag');
        $kurzeintrag = PostVar ('kurzeintrag');
        $status = PostVar('status');
                $id = PostVar('id');

          
          
        und woher kommt die Variable PostVar, die offensichtlich ein Array ist?  
          
        
        > Das Script liegt in einem mit .htaccess geschützten Ordner, auf welchem nur ich Zugriff habe. Deswegen habe ich die Sicherheitsaspekte etwas vernachlässigt.  
          
        das ist noch lange kein Grund für schlampigen Code.  
          
          
        Freundliche Grüße  
          
        Vinzenz
        
        1. Glück auf!

          Hallo Alex,

          es ist ein bisschen rätselhaft. Erst mal wäre es wichtig zu wissen, wann und wie die Variablen wie z.B. $status initialisiert werden.

          Entschuldige. Das habe ich in der Tat vergessen anzugeben:

          $datum = PostVar('datum');
          $eintrag = PostVar('eintrag');
          $kurzeintrag = PostVar ('kurzeintrag');
          $status = PostVar('status');
                  $id = PostVar('id');

          
          >   
          >   
          > und woher kommt die Variable PostVar, die offensichtlich ein Array ist?  
            
          function PostVar($variablen\_name) {  
           $ergebnis = $\_POST[$variablen\_name];  
           if (get\_magic\_quotes\_gpc()) $ergebnis = stripslashes($ergebnis);  
           return trim($ergebnis);  
          }  
            
          
          > > Das Script liegt in einem mit .htaccess geschützten Ordner, auf welchem nur ich Zugriff habe. Deswegen habe ich die Sicherheitsaspekte etwas vernachlässigt.  
          >   
          > das ist noch lange kein Grund für schlampigen Code.  
            
          Für den Code hab ich mich doch schon entschuldigt. Das war doch nur auf die Sicherheit bezogen.  
            
          Freundliche Grüße  
            
          zwerg Alex
          
          1. Hi,

            Für den Code hab ich mich doch schon entschuldigt.

            Du brauchst Dich nicht zu entschuldigen, sondern solltest die Ratschläge beherzigen: Sicherheit sollte ein Thema sein, dass Dich *jederzeit* als integraler Bestandteil begleitet.

            Was die Fehlerbeschreibung betrifft: Diese beinhaltet auch immer Deine Analyse.

            Cheatah

            --
            X-Self-Code: sh:( fo:} ch:~ rl:° br:> n4:& ie:% mo:) va:) de:] zu:) fl:{ ss:) ls:~ js:|
            X-Self-Code-Url: http://emmanuel.dammerer.at/selfcode.html
            X-Will-Answer-Email: No
            X-Please-Search-Archive-First: Absolutely Yes
            1. Lieber Cheatah, lieber Vinzenz,

              wie ich euch vom mitlesen kenne, habt ihr mir mit Sicherheit schon einen Hinweis gegeben, wo das Problem liegt. Nur sprecht ihr in Rätseln für mich. Ich fühl mich einfach nur geprügelt für den schlechten Code ohne in euren Antworten eine Lösung für mein Problem zu sehen.

              Vlt. bin ich für heute aber auch zu betriebsblind und auf zu vielen Baustellen unterwegs (arbeite gerade noch an ner Bildergalerie). Ich werd für heute erstmal aufgeben.

              Freundliche Grüße
              Zwerg Alex

              1. Hallo Alex,

                wie ich euch vom mitlesen kenne, habt ihr mir mit Sicherheit schon einen Hinweis gegeben, wo das Problem liegt. Nur sprecht ihr in Rätseln für mich. Ich fühl mich einfach nur geprügelt für den schlechten Code ohne in euren Antworten eine Lösung für mein Problem zu sehen.

                Dein relevanter Codeabschnitt für Bearbeiten ist der folgende:

                  
                elseif ($status == "Bearbeiten") {  
                 $query = "UPDATE blog SET  
                         eintrag = '$eintrag', kurzeintrag = '$kurzeintrag' WHERE id = $id";  
                 $sql = mysql_query($query);  
                 unset($id, $datum, $eintrag, $kurzeintrag);  
                }  
                
                

                a) Du behandeltst keinen einzigen der Eingabewerte mit
                   mysql_real_escape_string().

                Was passiert, wenn ein problematisches Zeichen, wie z.B. ein einfaches
                   Anführungszeichen in $_POST['eintrag'] oder $_POST['kurzeintrag'] steht?

                Logge daher den Inhalt von $sql in einer Datei mit

                b) Du überprüfst nicht den Rückgabewert von mysql_query().
                   Wie willst Du feststellen, ob die Datenbankoperation erfolgreich war.
                   Prüfe und logge daher den Rückgabewert von mysql_query() buw. mysql_error().

                Bei unverändertem Code (außer dem Log-Mechanismus) solltest Du nun feststellen
                können, wann das Update fehlschlägt und im dazugehörigen SQL-Statement ggf. auch den Fehler finden können. Das ist eine normale Debug-Strategie.

                Leicht abgeänderter Code:

                  
                elseif ($_POST['status'] == "Bearbeiten") {  
                    $query = "UPDATE blog SET  
                                  eintrag = '" . mysql_real_escape($_POST['eintrag']) . "',  
                                  kurzeintrag = ' . mysql_real_escape($_POST['kurzeintrag']) . "'  
                              WHERE id = ' . mysql_real_escape($_POST['id']) . "'";  
                  
                    // [link:http://www.php.net/manual/de/function.sprintf.php@title=sprintf()] ist eine lesbarere Alternative zu solchen  
                    // zusammengebauten Zeichenketten:  
                    $query = sprintf(  
                        "UPDATE blog SET  
                            eintrag = '%s',  
                            kurzeintrag = '%s'  
                        WHERE  
                            id = '%s'",  
                        mysql_real_escape($_POST['eintrag']),  
                        mysql_real_escape($_POST['kurzeintrag']),  
                        mysql_real_escape($_POST['id'])  
                    );  
                    // Beachte, dass Du im SQL-Dialekt von MySQL auch Zahlen in einfache  
                    // Anführungszeichen setzen kannst - und bei Benutzereingaben auch solltest.  
                  
                    if ($success = mysql_query($query)) === false) {  
                        // Fehler aufgetreten  
                        // Logge geeignet $query und mysql_error();  
                        // Gebe Dir eine Rückmeldung, dass ein Fehler aufgetreten ist.  
                    }  
                    else {  
                        // Update erfolgreich durchgeführt  
                        // Überflüssig, da nicht umkopiert:  
                        // unset($id, $datum, $eintrag, $kurzeintrag);  
                        // Du solltest allerdings dem Benutzer (also Dir selbst) eine  
                        // Rückmeldung geben  
                    }  
                }  
                
                

                Du siehst hier sofort, dass die Daten in der Abfrage aus potentiell gefährlicher Quelle stammen. Die Daten wurden ordnungsgemäß für die SQL-Schnittstelle maskiert, es kann nichts Schlimmes passieren.

                Hübscher wäre es mit der verbesserten MySQL-Erweiterung mysqli, die entgegen der Warnung auf der deutschen Handbuchseite nicht mehr experimentell ist, und Prepared Statements, siehe mysqli_prepare(). Beachte, dass die Parameter getrennt vom Statement übermittelt werden und sich MySQL selbst um die fachgerechte Aufbereitung der Parameter kümmert.

                Freundliche Grüße

                Vinzenz

                1. Hallo nochmal Vinzenz, hallo Cheatah,

                  Ich habe gerade entgegen meinem Vorsatz doch nochmal über eure Hinweise gebrütet und habe den vermeintlichen Fehler gefunden. Tatsächlich lag es an dem unbehandelten Formularwerten.

                  Das hat sich jetzt mit der Antwort von Vinzenz überschnitten. Aber ich bin mir sicher, dass ich dort noch was zum optimieren finde. Ich versuche kurzfristig eure Hinweise nochmal durchzugehen und so gut wie möglich umzusetzen.

                  Jedenfalls vielen Dank an euch Beide.

                  Freundliche Grüße und noch einen frohen Ostermontag

                  zwerg Alex

  2. Hallo Zwerg Alex,

    ich zerpflücke ein bißchen Dein Posting und knüpfe dort an, wo Du aufgehört hast.

    Ich weiß, dass das o. g. Script eine Zumutung ist.

    ist es in der Tat. Du solltest es schnellstens aus dem Verkehr ziehen, um- bzw. neu schreiben und danach wieder in den Verkehr bringen.

    P.S.
    Ich hoffe ihr versteht, dass ich das obige Script aus Sicherheitsgründen nicht verlinken kann.

    Ja.

    Das macht es mir wohl auch so schwer den Fehler zu finden. Aber das war irgendwann mal nen ganz süßes kleines Script, was ich nach und nach angepasst habe, bis das da oben draus geworden ist.

    Ich habe eine Datenbank mit zwei Tabellen (blog und guestbook). Ich habe nun für die Tabelle Blog ein Script, welches je nach Inhalt der Variablen "$status" unterschiedliche Aktionen durchführen soll.

    <?php

    $status? Bitte $_POST['status']. Das gilt gleichermaßen für alle Parameter, die aus Deinem Formular kommen.

    Statt if - elseif - elseif ... solltest Du switch ... case verwenden.

    if ($status == "Vorschau") {
    echo "<h3>", $datum, "</h3><h4>Eintrag:</h4><p>", format($eintrag), "</p><h4>Kurzeintrag:</h4><p>", format($kurzeintrag), "</p>";
    }

    elseif ($status == "Eintragen") {
    $query = "INSERT INTO blog SET
             datum = now(),

    Nein, kein addslashes, sondern mysql_real_escape_string. Oder prepared Statements.

    eintrag = '".addslashes($eintrag)."',
             kurzeintrag = '".addslashes($kurzeintrag)."'
             ";

    die() ist keine Fehlerbehandlung.

    $sql = mysql_query($query) or die(mysql_error());
    unset($status,$eintrag,$kurzeintrag);
    }

    elseif ($status == "Bearbeiten") {
    $query = "UPDATE blog SET

    Aua, $id kommt ebenfalls über $_POST. Hier ohne jegliche Behandlung :-(

    eintrag = '$eintrag', kurzeintrag = '$kurzeintrag' WHERE id = $id";
    $sql = mysql_query($query);

    Wo ist die Überprüfung, ob der Vorgang erfolgreich war?
    Wie willst Du hier einen Fehler bei "Bearbeiten" finden?

    [...]

      
    
    > Das Problem tritt nun auf, wenn ich die Blogeinträge/-kurzeinträge bearbeiten will. Das klappt ab und an, aber meist nicht. Wann dieser "ab und an"-Fall eintrifft, kann ich mir/euch nicht erklären.  
      
    Hmm, das ist kein Wunder, wenn Du es nicht überprüfst :-)  
      
      
    Vinzenz
    
    1. Glück auf Vinzenz!

      Ich weiß, dass das o. g. Script eine Zumutung ist.

      ist es in der Tat. Du solltest es schnellstens aus dem Verkehr ziehen, um- bzw. neu schreiben und danach wieder in den Verkehr bringen.

      Danke, dass du dich trotzdem damit beschäftigt hast. Es ist wohl in der Tat das Beste, wenn ich aus dem ganzen Script 4 Dateien mache für jeden Status. Aber mein Problem ist dann wohl immer noch nicht gelöst :-/

      Statt if - elseif - elseif ... solltest Du switch ... case verwenden.

      Muss ich mir wohl mal angucken, welche Vorteile ich davon hätte. Mein Problem löst das aber nicht, oder?

      if ($status == "Vorschau") {
      echo "<h3>", $datum, "</h3><h4>Eintrag:</h4><p>", format($eintrag), "</p><h4>Kurzeintrag:</h4><p>", format($kurzeintrag), "</p>";
      }

      elseif ($status == "Eintragen") {
      $query = "INSERT INTO blog SET
               datum = now(),

      Nein, kein addslashes, sondern mysql_real_escape_string. Oder prepared Statements.

      Jo. Weiß nicht, warum ich "addslashes" dadrin habe. Werde ich ändern.

      eintrag = '".addslashes($eintrag)."',
               kurzeintrag = '".addslashes($kurzeintrag)."'
               ";

      die() ist keine Fehlerbehandlung.

      Hhhm. Änder ich.

      $sql = mysql_query($query) or die(mysql_error());
      unset($status,$eintrag,$kurzeintrag);
      }

      elseif ($status == "Bearbeiten") {
      $query = "UPDATE blog SET

      Aua, $id kommt ebenfalls über $_POST. Hier ohne jegliche Behandlung :-(

      Auf das Script kann ja nur ich zugreifen (.htaccess-Schutz) von daher ok, oder?

      eintrag = '$eintrag', kurzeintrag = '$kurzeintrag' WHERE id = $id";
      $sql = mysql_query($query);

      Wo ist die Überprüfung, ob der Vorgang erfolgreich war?
      Wie willst Du hier einen Fehler bei "Bearbeiten" finden?

      [...]

      [/code]

      Das Problem tritt nun auf, wenn ich die Blogeinträge/-kurzeinträge bearbeiten will. Das klappt ab und an, aber meist nicht. Wann dieser "ab und an"-Fall eintrifft, kann ich mir/euch nicht erklären.

      Hmm, das ist kein Wunder, wenn Du es nicht überprüfst :-)

      Ich hab das mehrfach überprüft. Dachte erst, das geht nur, wenn ich zunächst über Vorschau gehe, aber auch da klappt es nicht immer. Aber mit "überprüfen" meinst du warscheinlich nicht dieses "testen", aber was meinst du?

      Freundliche Grüße

      zwerg Alex

      1. Hallo Alex,

        Danke, dass du dich trotzdem damit beschäftigt hast. Es ist wohl in der Tat das Beste, wenn ich aus dem ganzen Script 4 Dateien mache für jeden Status.

        nein, das ist nicht erforderlich. Es erhöht die Gesantkomplexität.

        Statt if - elseif - elseif ... solltest Du switch ... case verwenden.

        Muss ich mir wohl mal angucken, welche Vorteile ich davon hätte. Mein Problem löst das aber nicht, oder?

        Sauberer strukturierten Code - und nein, Dein Problem wird dadurch nicht gelöst.

        if ($status == "Vorschau") {
        echo "<h3>", $datum, "</h3><h4>Eintrag:</h4><p>", format($eintrag), "</p><h4>Kurzeintrag:</h4><p>", format($kurzeintrag), "</p>";

        ach so, das hatte ich völlig vergessen:
        Trenne Datenverarbeitung von Ausgabe.
        Du könntest hier hübsche kleine Funktiönchen verwenden :-)

        Nein, kein addslashes, sondern mysql_real_escape_string. Oder prepared Statements.
        Jo. Weiß nicht, warum ich "addslashes" dadrin habe. Werde ich ändern.

        Ein guter Anfang.

        elseif ($status == "Bearbeiten") {
        $query = "UPDATE blog SET

        Aua, $id kommt ebenfalls über $_POST. Hier ohne jegliche Behandlung :-(
        Auf das Script kann ja nur ich zugreifen (.htaccess-Schutz) von daher ok, oder?

        Nein, ist es nicht. Bitte escape _alle_ Eingaben, selbst wenn sie von geschützten Admin-Skripten kommen. Es ist viel einfacher, _immer_ die angemessene Behandlung zu verwenden. Es spart Überlegung - und Du bist immer auf der sicheren Seite.

        eintrag = '$eintrag', kurzeintrag = '$kurzeintrag' WHERE id = $id";
        $sql = mysql_query($query);

        Wo ist die Überprüfung, ob der Vorgang erfolgreich war?
        Wie willst Du hier einen Fehler bei "Bearbeiten" finden?

        Das Problem tritt nun auf, wenn ich die Blogeinträge/-kurzeinträge bearbeiten will. Das klappt ab und an, aber meist nicht. Wann dieser "ab und an"-Fall eintrifft, kann ich mir/euch nicht erklären.

        Hmm, das ist kein Wunder, wenn Du es nicht überprüfst :-)

        Ich hab das mehrfach überprüft. Dachte erst, das geht nur, wenn ich zunächst über Vorschau gehe, aber auch da klappt es nicht immer. Aber mit "überprüfen" meinst du warscheinlich nicht dieses "testen", aber was meinst du?

        Ich sehe keine Überprüfung des Rückgabewertes von mysql_query. Diese Überprüfung sollte übrigens mit dem Vergleichsoperator auf Identität vorgenommen werden, da die Rückgabe 0 eine legale Rückgabe eines erfolgreichen Statements sein kann. Es wurde in diesem Fall kein Datensatz geändert.

        Freundliche Grüße

        Vinzenz