PHP-Neuling: php header("location ") oder window.close ohne funktion

Hi zusammen :)

Jetzt bin ich schon wieder hier, und zermadere mir mein Kleinhirn.

Das Problem ist so simpel, dass ich wahrscheinlich deswegen die Kurve nicht kriege. Oder irgendwie so ...

Ich habe für eine Datenbankübersicht ein einfaches delete script geschrieben.

Es ist ein Script, welches für mehrere Tabellen gültig ist. Das Script per se funktioniert auch. Aber in diesem speziellen Fall werde ich das script nicht wieder los.

Ich hänge an das Ende des Scriptes üblicherweise ein

Header("Location: index.php");

um nach erfolgter Löschung oder Speicherung direkt wieder eine aktuelle version der Seite zu ziehen.

Das funktioniert auch immer, eigentlich.

Bei diesem kurzen DELETE script funktioniert das überhaupt nicht. Der PHP Code wird ausgeführt, anschließend bleibt die Seite weiß.

<?php
header('Content-Type: text/html; charset=utf-8');
header("Expires: on, 01 Jan 1970 00:00:00 GMT");
header("Last-Modified: " . gmdate("D, d M Y H:i:s") . " GMT");
header("Cache-Control: no-store, no-cache, must-revalidate");
header("Cache-Control: post-check=0, pre-check=0", false);
header("Pragma: no-cache");
ini_set("display_errors", true);
require '../include/db.php';
IF(isset($_GET['NEWS'])) { $ID1 = $_GET['NEWS'] ; $TYPE = "br_news";} else {
IF(isset($_GET['TERM'])) { $ID1 = $_GET['TERM'] ; $TYPE = "br_termine";} else {
IF(isset($_GET['MTGL'])) { $ID1 = $_GET['MTGL'] ; $TYPE = "br_mitglieder";}}}

	$Entfernen = $db->query("DELETE FROM " .$TYPE. " WHERE ID = $ID1 ");
	$Entfernen->execute() or die($db->error);
	Header("Location: index.php");
}
?>

Was soll also passieren:

Über die index.php wird einem link über del.php? der TYPE mitgegeben. Bspw.

<a href="del.php?NEWS=2">click</a>

Die 2 ist die ID des DB Eintrags. NEWS/MTGL/TERM brauche ich zum wählen der korrekten DB.

das query läuft durch. Die entsprechenden Einträge werden also korrekt gelöscht. Aber die Seite bleibt weiß.

Testweise habe ich auch javascript zugefügt um die Seite zu schließen/umzuleiten. Oder einfach nur einen HTML Teil angefügt.

Sobald das Script gelaufen ist wird nichts mehr geladen. weder PHP code noch HTML. Auch ein

echo 'Raketenfürst'; direkt nach dem Execute wird nicht mehr ausgeführt.

Wo habe ich denn den Hänger ?

Vg

  1. Hallo PHP-Neuling,

    den ini_set wird er wohl nicht ausführen, weil er das Script gar nicht erst startet. Dem } am Ende fehlt das {-Gegenstück.

    Deine IF-Kette ist übrigens verbesserungsfähig.

    • Konsistenz beim Schreiben der Schlüsselwörter. if und else oder IF und ELSE
    • Zeilenumbrüche. Es gibt natürlich die unterschiedlichsten Stile beim Setzen von { und } und beim Einrücken von bedingtem Code, aber "all in one line" macht man höchstens dann, wenn es einen IF und ein kurzes Statement im then-Teil gibt. Aber gut ist auch das nicht. Für
    if (isset($_GET['NEWS'])) {
       $ID1 = $_GET['NEWS'];
       $TYPE = "br_news";
    }
    

    sollte immer Zeit sein. Vertikaler Platz muss nicht extra bezahlt werden 😉

    • Ein Ketten-If kann man eleganter schreiben als mit dreifach geschachtelten Klammern.
    if (isset($_GET['NEWS'])) {
       $ID1 = $_GET['NEWS'];
       $TYPE = "br_news";
    }
    else if (isset($_GET['TERM'])) {
       $ID1 = $_GET['TERM'];
       $TYPE = "br_termine";
    }
    else if (isset($_GET['MTGL'])) {
       $ID1 = $_GET['MTGL'];
       $TYPE = "br_mitglieder";
    }
    
    • Und schließlich mal wieder ein Klassiker.
    $Entfernen = $db->query("DELETE FROM " .$TYPE. " WHERE ID = $ID1 ");
    

    Ich würde diesen Code ja gerne dunkelrot und blinkend machen. Sicherlich haben wir Dir schon was vom Kontextwechsel erzählt? Dein Code erlaubt Injektion von SQL, das deine DB zerstört.

    Referenzbeispiel dafür: https://xkcd.com/327/

    Rolf

    --
    sumpsi - posui - obstruxi
    1. Hi Rolf. und wieder mal danke für deine Mühe

      die letzte "}" war ein copypaste fehler. Zwischenzeitlich hatte ich noch ein submit drumrum gebaut

      Die Hinweise zur Ordnung und Sicherung (Kontextwechsel) habe ich verstanden Da war ich jetzt einfach etwas vorschnell

      Hier nochmal überarbeitet (noch ohne Kontextwechsel) und mit Schaltfläche. Header greift dennoch nicht

      if(isset($_GET['NEWS'])) { 
      $ID1 = $_GET['NEWS']; 
      $TYPE = "br_news";
      } 
      else if (isset($_GET['TERM'])) { 
      $ID1 = $_GET['TERM']; 
      $TYPE = "br_termine";
      } else if (isset($_GET['MTGL'])) { 
      $ID1 = $_GET['MTGL']; 
      $TYPE = "br_mitglieder";
      }
      
      if (isset($_POST['DELETE']) and $_POST['DELETE']== 'loeschen' ) {
      
      	$Entfernen = $db->query("DELETE FROM " .$TYPE. " WHERE ID = $ID1 ");
      	$Entfernen->execute() or die($db->error);
      	Header("Location: close.php");
      }
      
      1. Hallo,

        die letzte "}" war ein copypaste fehler. Zwischenzeitlich hatte ich noch ein submit drumrum gebaut

        wie bitte??

        Hier nochmal überarbeitet (noch ohne Kontextwechsel) und mit Schaltfläche. Header greift dennoch nicht

        Wo ist da eine Schaltfläche?

        if(isset($_GET['NEWS'])) { 
        $ID1 = $_GET['NEWS']; 
        $TYPE = "br_news";
        } 
        else if (isset($_GET['TERM'])) { 
        $ID1 = $_GET['TERM']; 
        $TYPE = "br_termine";
        } else if (isset($_GET['MTGL'])) { 
        $ID1 = $_GET['MTGL']; 
        $TYPE = "br_mitglieder";
        }
        
        if (isset($_POST['DELETE']) and $_POST['DELETE']== 'loeschen' ) {
        
        	$Entfernen = $db->query("DELETE FROM " .$TYPE. " WHERE ID = $ID1 ");
        	$Entfernen->execute() or die($db->error);
        	Header("Location: close.php");
        }
        

        Ich habe mal die Code-Auszeichnung korrigiert: Nicht HTML, sondern PHP.
        Außerdem scheinst du jetzt zweigleisig zu fahren: Einige Parameter erwartest du per GET (also als URL-Parameter), einige per POST. Ernsthaft? - Okay, das ist nicht verboten, aber dann muss man beim Request auch sauber darauf achten, dass man alle Parameter richtig bestückt.

        Live long and pros healthy,
         Martin

        --
        Wer respektiert werden will, sollte zunächst damit anfangen, andere zu respektieren.
        1. Hi

          Die Schaltfläche befindet sich im HTML kontext. Die hatte ich jetzt just nicht mit herauskopiert

          <form method="post">
          <input type="hidden" name="DELETE" value="loeschen"><input id="" type="submit" value="Wirklich löschen ?" onclick="submit">
          </form>
          

          Daher auch ein POST. Das ist eigentlich überhaupt nicht nötig, und auch gar nicht gewollt. Das Script soll direkt löschen.

          Ich wollte lediglich die geschweifte Klammer erklären am Ende des PHP.

          Egal ... Summasummarum. leitet dennoch nicht weiter. Weder so, noch so, noch ohne alles, auch nicht im Kontextwechsel

          1. okay, ich habs :)

            Verstehen tu ich's nicht. Aber nachdem ich mein Statement umgebaut habe funktioniert die Weiterleitung einwandfrei

            Vielen Dank euch

            Vg

          2. Hallo PHP-Neuling,

            auch wenn Du das jetzt nicht mehr verwendest - das geht eleganter mit dem Form.

            1. Dein onclick ist wirkungslos. Die Submit-Aktion wird durch type="submit" ausgelöst, und wenn Du bei Click eine Funktion namens submit aufrufen willst, müsstest Du onclick="submit()" schreiben. So, wie es jetzt ist, ist es ein Syntaxfehler und wird ignoriert.

            2. Ein Submit-Button braucht nur dann einen Click-Handler, wenn du explizit den Submit durch diesen Klick überwachen willst (es gibt auch Submit durch ENTER Taste oder ggf. andere Buttons). Willst Du generell den Submit des Form überwachen, registriere Dich auf das submit-Event des Forms.

            3. <input type="submit"> ist nachteilig, weil Text und Value nicht trennbar sind. Man nimmt dafür besser das Button-Element. Dem gibst Du einen name und einen value, und die bekommst Du dann bei Klick auf den Button oder ENTER Taste im Form auch gepostet.

            <form method="post" >
            <button type="submit" name="DELETE" value="loeschen">Wirklich löschen?</button>
            </form>
            

            type="submit" kann man auch weglassen, das ist der Defaut. Und den Button mit einem Fragezeichen zu beschriften ist irreführend, wenn es die Bestätigung ist... Da gehört ein ‼️ hin.

            Rolf

            --
            sumpsi - posui - obstruxi
            1. Hallo Rolf,

              1. Dein onclick ist wirkungslos.

              das stimmt soweit. Aber ...

              müsstest Du onclick="submit()" schreiben. So, wie es jetzt ist, ist es ein Syntaxfehler und wird ignoriert.

              ... nicht deshalb. Es ist kein Syntaxfehler, sondern ein korrekter, gültiger Ausdruck. Er ermittelt eine Referenz auf die submit-Methode und tut dann einfach nichts damit. Auch das

              42;
              

              ist eine gültige Javascript-Anweisung. Nutzlos, aber korrekt.

              Und den Button mit einem Fragezeichen zu beschriften ist irreführend, wenn es die Bestätigung ist... Da gehört ein ‼️ hin.

              Yesss!!

              Live long and pros healthy,
               Martin

              --
              Wer respektiert werden will, sollte zunächst damit anfangen, andere zu respektieren.
              1. Hallo Martin,

                whoa - was passiert denn da in onclick? Was ist der Kontext, in dem Code gesucht wird?

                Ich habe mal ein bisschen gespielt - es macht den Eindruck, als würde er eine Funktion zuerst am Button-Element suchen, dann am Form und dann global.

                Also

                <button onclick="hugo()">huhu</button>
                

                ruft eine hugo-Methode auf, wenn ich sie an diesen Button klebe. Oder an HTMLButtonElement.prototype. Oder ich klebe sie ans Form. Oder an HTMLFormElement.prototype. Oder an window. WTUF? Ist das irgendwo dokumentiert?

                Rolf

                --
                sumpsi - posui - obstruxi
  2. Hallo,

    Bei diesem kurzen DELETE script funktioniert das überhaupt nicht. Der PHP Code wird ausgeführt, anschließend bleibt die Seite weiß.

    Wie ist display_errors eingestellt? Eine weiße Seite deutet darauf hin dass das Script aufgrund eines Fehlers abgebrochen wurde.

    $Entfernen = $db->query("DELETE FROM " .$TYPE. " WHERE ID = $ID1 ");
    

    Wie Rolf schon schrieb: das ist ein gefährliches Sicherheitsscheunentor (eine Lücke ist das schon nicht mehr)! Das zu beheben wäre ganz einfach: statt $ID1 ein »?« (oder einen benannten Parameter bei PDO) in den Query schreiben, prepare() statt query() verwenden und $ID1 in einem Array an execute() übergeben, fertig.

    $Entfernen->execute() or die($db->error);
    

    Mal abgesehen davon dass ein execute() wenig sinnvoll ist wenn du query() verwendest (das braucht man nur bei prepare()): Was verwendest du denn um auf die Datenbank zuzugreifen? Wenn du mysqli verwendest, enthält $Entfernen an dem Punkt ein true. Boolsche Werte haben aber natürlich keine Methode execute() und damit schlägt die Zeile mit einem Fatal Error fehl – das Ergebnis (bei display_errors=0) ist eben eine leere Seite.

    <a href="del.php?NEWS=2">click</a>
    

    Aktionen die Daten verändern (anlegen, ändern oder löschen) sollten per POST-Request erfolgen, nicht per (GET-)Link.

    Gruß,
    Tobias

    1. Vielen lieben Dank euch allen

      ich hatte da tatsächlich einen syntax-fehler. Die Didaktik bei prepared Statements ist wohl noch nicht ganz in Fleisch und Blut übergegangen...

      Das ist auch immer ein Problem wenn man zum Erlernen und Fehlersuchen verschiedene Quellen nimmt. Denn verschiedene Quellen benutzen verschiedene Lösungswege (die auch manchmal gar nicht funktionieren), die untereinander nicht zwangsweise kompatibel sein müssen.

      Mein Delete Statement sieht jetzt so aus

      if($db->query("DELETE FROM $TYPE  WHERE ID = $ID1")){
          printf("%d bearbeitete Zeilen.\n", $db->affected_rows);
      	}
      	$db->close;	
      

      den print habe ich zum testen drin. Im normalfall folgt der header("location") nach dem ->close

      Ist manchmal schwer zu sehen, welche Syntax jetzt zusammen passt oder nicht.

      Prozeduale oder Objektive Schreibweise

      Mysqli oder mysql oder pdo

      Da renne ich immer mal wieder in Fallen ... Genau so war es jetzt hier mit dem Execute()

      Den Code für den Button habe ich, so wie er ist, tatsächlich aus einem Lehrbuch (!), und habe den bislang immer für alle Knöpfe übernommen

      Ich probier aber mal rum ob man das auch einkürzen kann

      Ich bin euch sehr dankbar :)

      1. Hallo,

        Mein Delete Statement sieht jetzt so aus

        if($db->query("DELETE FROM $TYPE  WHERE ID = $ID1")){
            printf("%d bearbeitete Zeilen.\n", $db->affected_rows);
        	}
        	$db->close;	
        

        Das ist leider immer noch genauso falsch und gefährlich wie vorher! Wie ich schon schrieb lag dein Problem an der execute()-Zeile, die bei dem von dir verwendeten mysqli einen Fatal-Error ausgelöst. Bei der Entwicklung solltest du die Ausgabe von Fehlermeldungen einschalten um solche Fehler selbst direkt zu sehen auch ohne ins errorlog schauen zu müssen.

        Die letzte Zeile scheint mir aber auch falsch zu sein - du möchtest die Methode close() aufrufen (was allerdings nicht notwendig ist), eine Eigenschaft close (also ohne die Klammern dahinter) gibt es nicht.

        Ist manchmal schwer zu sehen, welche Syntax jetzt zusammen passt oder nicht.

        Prozeduale oder Objektive Schreibweise

        Mysqli oder mysql oder pdo

        Die mysql-Erweiterung gibt es nicht mehr, es bleiben nur noch mysqli und PDO. Ich würde zu PDO (und damit objektorientiert) raten, das unterstützt benannte Parameter in prepared Statements, und das Zuweisen der Parameter ist einfacher. Dein Lösch-Query sähe dann so aus:

        $stm = $pdo->prepare("DELETE FROM ".$TYPE." WHERE ID = :id");
        $stm->execute(['id' => $ID1]);
        

        wenn du bei mysqli bleiben willst, musst du statt „:id“ ein Fragezeichen verwenden und vor dem execute() noch die Werte über bind_param() zuweisen (das was ich gestern geschrieben hatte war nicht ganz korrekt, das mysqli-execute kann keine Parameter entgegennehmen, das muss vorher gemacht werden).

        Da renne ich immer mal wieder in Fallen ... Genau so war es jetzt hier mit dem Execute()

        Gewöhn dir ab besten an immer prepared Statements zu verwenden und verwende query() nur dann wenn du einen statischen Query ausführen willst der keinerlei Parameter enthält. Und denk daran die Löschaktion über einen POST-Request anzustoßen, nicht über einen Link.

        Den Code für den Button habe ich, so wie er ist, tatsächlich aus einem Lehrbuch (!), und habe den bislang immer für alle Knöpfe übernommen

        Von wann ist das Lehrbuch? Bücher neigen dazu relativ schnell zu veralten, auch Internetseiten sind oft veraltet (bei denen ist es aber oft nicht erkennbar von wann die Seite stammt). Leider ist es (v.a. für Anfänger) meist schwer zu erkennen welche Seite/welches Buch was taugt und welche(s) nicht.

        Gruß,
        Tobias

        1. uff. escapen wie o.g. ist also nicht ausreichend

          okay schei** drauf. Ich münze jetzt alles auf PDO um. Ist langfristig scheinbar das einzig vernünftige, und ich falle vielleicht nicht mehr so schnell auf die Nase mit solch Kleinigkeiten

          ...

          die Frage die hier stand war natürlich total dämlich ...

          Ich nutze ab jetzt überall pdo

          vielen vielen Dank

          1. Hallo PHP-Neuling,

            escapen wie o.g. ist also nicht ausreichend

            Wo escapest Du denn was? Wenn Du es tun würdest, dann könnte es auch ausreichen. Den TYPE generierst Du selbst, denn muss man nicht escapen. Das Problem ist die ID. Wenn sie numerisch ist, könnte auch ein intval() ausreichen, um nur eine Zahl durchzulassen. Andernfalls gibt's in mysqli die Methode real_escape_string (die heißt aus historischen Gründen so) und in PDO gibt's quote.

            Die Variable mit dem DB-Handle kannst Du nennen wie Du willst. Meinetwegen auch $oracle.

            Rolf

            --
            sumpsi - posui - obstruxi
            1. Hallo Rolf,

              ergänzend:

              Andernfalls gibt's in mysqli die Methode real_escape_string (die heißt aus historischen Gründen so) und in PDO gibt's quote.

              … was in dem Fall hier aber nicht ausreichend würde, dafür müssten im Query noch Anführungszeichen um $ID1 stehen, sonst könnten mit der Übergabe von 1 OR 1 immernoch alle Datensätze gelöscht werden.

              Aber wie schon gesagt: besser immer prepared Statements verwenden, dann spart man sich das escapen.

              Gruß,
              Tobias

              1. $stmt = $pdo->prepare("DELETE FROM ".$TYPE." WHERE ID = :id"); $stmt->execute(['id' => $ID1]);

                ich lerne nun mit PDO umzugehen. zukünftig werde ich auch nichts anderes mehr nehmen, und bestehende mysqli anwendungen umarbeiten

                Am Ende ist es sogar "einfacher" mit PDO stmt's, wenn man's erstmal raus hat. Sah aber immer kompliziert komisch aus .. :-D

                Danke Euch

              2. Hallo tk,

                Anführungszeichen um $ID1

                latürnich. Das hatte ich übersehen. Ich muss nur Strings escapen, und Strings muss ich in Anführungszeichen setzen.

                Zahlen muss ich zu Zahlen machen und ohne Anführungszeichen einsetzen, weswegen ich auf intval verwies.

                Einen String an einen Query Parameter zu übergeben, der eine Zahl erwartet, dürfte zu SQL Fehlern führen...

                Kontextwechsel ist nichts für einfache Gemüter 😉

                Rolf

                --
                sumpsi - posui - obstruxi