Ist der Code sinnvoll?

Einklappen
X
 
  • Filter
  • Zeit
  • Anzeigen
Alles löschen
neue Beiträge

  • Ist der Code sinnvoll?

    Hallo

    Da ich momentan dabei bin, alle meine Skripte von einem projekt soweit ordnetlich zu machen, sprich nach unbenutzten Variablen zu suchen usw. oder einfach das manche Sachen anders sinnvoller sind würde ich euch gerne mal fragen ob diese Seite soweit in ordnung ist? Was würdet ihr anders machen, was ist sinnvoller usw.

    lasst einfach mal eure Meinung hören.

    Hier mein Script:

    PHP-Code:
    <?
    //Folgende Variablen bekommt diese Seite übergeben:
    //  $pass
    //  $id
    error_reporting(E_ALL);
    echo "<html>";
    echo "<head>";
    echo "<title>Cover Upload</title>";

    //CSS Stylesheet wird geladen.
    ?>
      <link rel="stylesheet" type="text/css" href="style.css" />
    <?

    echo "</head>";
    echo "<body>";

    //Variablen die ich nach einem Seitenaufruf wieder lade.
    $pass = isset($_GET['pass']) ? $_GET['pass'] : '';
    $modul = isset($_GET['modul']) ? $_GET['modul'] : '';
    $id = isset($_GET['id']) ? $_GET['id'] : '';
    $cover_pfad_neu = isset($_GET['cover_pfad_neu']) ? $_GET['cover_pfad_neu'] : '';


    //Array um später die Dateitypen bestimmen zu können,
    //getimagesize schreibt Zahlen für Dateitypen
    $BILDTYP=array
    (
    "1"=>"GIF",
    "2"=>"JPG",
    "3"=>"PNG",
    "4"=>"SWF"
    );


    //Wenn Button "hochladen" aktiviert
    switch ($modul) 
    {
    case 'edit':

        //Datenbank connect (server, benutzer, pw)
        //DB auswählen.
        $dbconnect =mysql_connect ("localhost","root",$pass);
        $select_db=mysql_select_db ("mp3",$dbconnect);
        
        //Hole mir den Pfad von den Covers aus der Datenbank.
        $sql_ad_co = "SELECT * FROM `administration` WHERE `typ` = 'covers'";
        $sqlquerry_ad_co = mysql_query($sql_ad_co,$dbconnect) or die ("Fehler in SQL : $sql");
        $row_ad_co = mysql_fetch_array($sqlquerry_ad_co, MYSQL_NUM);
        
        //selektiere den Datensatz mit allen Feldern welchen ich zuvor ausgewählt habe.
        $sql_mp_id = "SELECT * FROM `mp3s` WHERE `id` = '$id'";
        $sqlquerry_mp_id = mysql_query($sql_mp_id,$dbconnect) or die ("Fehler in SQL : $sql");
        while ($row_mp_id = mysql_fetch_array($sqlquerry_mp_id, MYSQL_NUM))
        {
            
            //Der Pfad von dem Cover, welches Momentan benutzt wird, wird zusammen gefügt.
            $cover_pfad_alt = $row_ad_co[2]."/".$row_mp_id[2]." - ".$row_mp_id[4];
            
          //Bildinformationen des ausgewählten Covers auslesen, [0]=x-Pixel
          //[1]=y-pixel, [2]=Dateityp(1=gif, 2=jpg, 3=png, 4=swf)
          $pixel = getimagesize($cover_pfad_neu);
          
          //Das Array BILDTYP wird solange durchlaufen wie es Daten gibt.
          foreach($BILDTYP as $zahl=>$typ)
          {
            //Wenn Dateityp von neuem Cover gleich einer der 4 Typen des Arrays,
            //Wandel die Zahl in den Dateinamen um.
            if ($pixel[2] == $zahl)
            {
                
                //Lösche das alte Cover. 
                //Der Pfad und die Dateiendung werden hier zusammengesetzt und mit einem 
                //Punkt getrennt.
              unlink($cover_pfad_alt.".".strtolower($typ));
              
              //ausgewähltes Cover in  mit richtigem Dateinamen speichern.
              //Der Pfad ergibt sich aus den zuvor geholten Daten aus der Datenbank
              rename($cover_pfad_neu, $cover_pfad_alt.".".strtolower($typ));
              
              //Updatebefehl um die neuen Infos in die Tabelle zu schreiben.
              $sql_co_pf = "UPDATE `covers` SET 
                              pfad = '/".$row_mp_id[2]." - ".$row_mp_id[4]."', 
                              dateityp = '".strtolower($typ)."', 
                              pixelx = '".$pixel[0]."', 
                              pixely = '".$pixel[1]."' 
                            WHERE pfad = '$cover_pfad_alt'";
              mysql_query($sql_co_pf,$dbconnect);
            }
          }
        }
    break;

    //Formular wird erstellt für die Eingabe des Covers. Variablenname des neuen Covers: cover_pfad_neu

    case 0:
        ?>
        <form method="get" action="<?php echo $_SERVER['PHP_SELF']; ?>" enctype="multipart/form-data">
        <?
        echo "<table width= '348'>";
        echo "<tr>";
        echo "<td align='center'>";
        echo "<font face = 'Tahoma'>Datei auswählen </font>";
        echo "</td>";
        echo "</tr>";
          echo "<tr>";
            echo "<td align = 'center'>";
              echo "<table width= '348'>";
                echo "<tr>";
                  echo "<td align = 'center'>";
                   echo "<br><br>";
                    echo "<input type='hidden' name='modul' value='edit'>";
                    echo "<input type='hidden' name='pass' value='$pass'>";
                    echo "<input type='hidden' name='id' value='$id'>";
                    echo "<input type='file' size ='10' class='file' name='cover_pfad_neu'>";
                    echo "<br>";
                  echo "</td>";
                echo "</tr>";
              echo "</table>";
            echo "</td>";
          echo "</tr>";
          echo "<tr>";
            echo "<td>";
              echo "<table width= '348'>";
                echo "<tr>";
                  echo "<td width='145'>";
                  echo "</td>";
                  echo "<td>";
                  
                    //Button zum hochladen. beim klicken wird die Seite die diese geöffnet hat neu geladen
                    //sprich die Hauptseite und diese automatisch geschlossen
                    echo "<input type='submit' name='submit' value='Hochladen' 
                          onclick='opener.location.reload();self.close()' >";
                  echo "</td>";
                echo "</tr>";
              echo "</table>";
            echo "</td>";
          echo "</tr>";
        echo "</table>";
        echo "</form>";
        echo "</body>";
        echo "</html>";
    break;
    }
    ?>
    Danke euch schon mal.

    LG
    Jache
    Zuletzt geändert von Jache84; 14.11.2006, 16:25.

  • #2
    Re: Ist der Code sinnvoll?

    Erst mal solltest du dich für einen "Stil" beim Ausgeben von reinem HTML-Code entscheiden - <html>, <head>, <title>, <body> und weiter unten die Tabelle werden per echo ausgegeben, <link> und <form> stehen dann aber wieder außerhalb des PHP-Bereiches - da ist noch überhaupt keine konsistente Linie erkennbar.

    (Und die echos bei der Tabelle so einzurücken, ist auch mal was neues.)
    I don't believe in rebirth. Actually, I never did in my whole lives.

    Kommentar


    • #3
      Code:
      Globale Variable $sql wird vor ihrer Deklaration benutzt (Zeile 48)
      PHP-Code:
      <?php
      [...]
      $sql_mp_id "SELECT * FROM `mp3s` WHERE `id` = '$id'"/* <-- Injection-Gefahr, weil du einfach nur $_GET nach $id geschrieben hast
       ... ggf. $id = intval($_GET['id']) machen, für die restlichen Vars gilt das Gleiche */

      case 0/* Bitte? Default bitte mal in der Manual angucken! */

      /*
      Die ganzen folgenden Echos kann man auch weglassen
      */
      ?>
      hier HTML <?php echo $var;?> weiteres HTML
      <?php
      /*
      weiter mit php

      Finde ich übersichtlicher als 3 Dutzen echo-Aufrufe*/
      weiter guck ich jetzt nicht ^^,

      Ein netter Guide zum übersichtlichen Schreiben von PHP/MySQL-Code!

      bei Klammersetzung bevorzuge ich jedoch die JavaCoding-Standards
      Wie man Fragen richtig stellt

      Kommentar


      • #4
        okay habe es jetzt soweit abgeändert.

        Was gibts noch zu meckern, immer her damit. das mit diesem intval, in der manual steh das php aus dem wert einen integer wert macht. den wenn ich das vor meine Variablen schreibe geht das script nicht mehr.

        PHP-Code:
        <?
        //Folgende Variablen bekommt diese Seite übergeben:
        //  $pass
        //  $id
        error_reporting(E_ALL);
        ?>
        <html>
          <head>
            <title>Cover Upload</title>

            <!--CSS Stylesheet wird geladen.-->
            <link rel="stylesheet" type="text/css" href="style.css" />
          </head>
          <body>
            <?
            //Variablen die ich nach einem Seitenaufruf wieder lade.
            $pass = isset($_GET['pass']) ? $_GET['pass'] : '';
            $modul = isset($_GET['modul']) ? $_GET['modul'] : '';
            $id = isset($_GET['id']) ? $_GET['id'] : '';
            $cover_pfad_neu = isset($_GET['cover_pfad_neu']) ? $_GET['cover_pfad_neu'] : '';

            //Array um später die Dateitypen bestimmen zu können,
            //getimagesize schreibt Zahlen für Dateitypen
            $BILDTYP=array
            (
            "1"=>"GIF",
            "2"=>"JPG",
            "3"=>"PNG",
            "4"=>"SWF"
            );

            //Wenn Button "hochladen" aktiviert
            switch ($modul)
            {
              case 'edit':

              //Datenbank connect (server, benutzer, pw)
              //DB auswählen.
              $dbconnect =mysql_connect ("localhost","root",$pass);
              $select_db=mysql_select_db ("mp3",$dbconnect);

              //Hole mir den Pfad von den Covers aus der Datenbank.
              $sql_ad_co = "SELECT * FROM `administration` WHERE `typ` = 'covers'";
              $sqlquerry_ad_co = mysql_query($sql_ad_co,$dbconnect) or die ("Fehler in SQL : $sql");
              $row_ad_co = mysql_fetch_array($sqlquerry_ad_co, MYSQL_NUM);

              //selektiere den Datensatz mit allen Feldern welchen ich zuvor ausgewählt habe.
              $sql_mp_id = "SELECT * FROM `mp3s` WHERE `id` = '$id'";
              $sqlquerry_mp_id = mysql_query($sql_mp_id,$dbconnect) or die ("Fehler in SQL : $sql");
              while ($row_mp_id = mysql_fetch_array($sqlquerry_mp_id, MYSQL_NUM))
              {

                //Der Pfad von dem Cover, welches Momentan benutzt wird, wird zusammen gefügt.
                $cover_pfad_alt = $row_ad_co[2]."/".$row_mp_id[2]." - ".$row_mp_id[4];

                //Bildinformationen des ausgewählten Covers auslesen, [0]=x-Pixel
                //[1]=y-pixel, [2]=Dateityp(1=gif, 2=jpg, 3=png, 4=swf)
                $pixel = getimagesize($cover_pfad_neu);

                //Das Array BILDTYP wird solange durchlaufen wie es Daten gibt.
                foreach($BILDTYP as $zahl=>$typ)
                {

                  //Wenn Dateityp von neuem Cover gleich einer der 4 Typen des Arrays,
                  //Wandel die Zahl in den Dateinamen um.
                  if ($pixel[2] == $zahl)
                  {

                    //Lösche das alte Cover.
                    //Der Pfad und die Dateiendung werden hier zusammengesetzt und mit einem
                    //Punkt getrennt.
                    unlink($cover_pfad_alt.".".strtolower($typ));

                    //ausgewähltes Cover in  mit richtigem Dateinamen speichern.
                    //Der Pfad ergibt sich aus den zuvor geholten Daten aus der Datenbank
                    rename($cover_pfad_neu, $cover_pfad_alt.".".strtolower($typ));

                    //Updatebefehl um die neuen Infos in die Tabelle zu schreiben.
                    $sql_co_pf = "UPDATE `covers` SET
                    pfad = '/".$row_mp_id[2]." - ".$row_mp_id[4]."',
                    dateityp = '".strtolower($typ)."',
                    pixelx = '".$pixel[0]."',
                    pixely = '".$pixel[1]."'
                    WHERE pfad = '$cover_pfad_alt'";
                    mysql_query($sql_co_pf,$dbconnect);
                  }
                }
              }
              break;

              //Formular wird erstellt für die Eingabe des Covers.
              //Variablenname des neuen Covers: cover_pfad_neu

              default:
              ?>
              <form method="get" action="<?php echo $_SERVER['PHP_SELF']; ?>" enctype="multipart/form-data">
                <table width= "348">
                  <tr>
                    <td align="center">
                      <font face = "Tahoma">Datei auswählen </font>
                    </td>
                  </tr>
                  <tr>
                    <td align = "center">
                      <table width= "348">
                        <tr>
                          <td align = "center">
                            <br><br>
                            <input type="hidden" name="modul" value="edit">
                            <input type="hidden" name="pass" value="<? echo $pass; ?>">
                            <input type="hidden" name="id" value="<? echo $id; ?>">
                            <input type="file" size ="10" class="file" name="cover_pfad_neu">
                            <br>
                          </td>
                        </tr>
                      </table>
                    </td>
                  </tr>
                  <tr>
                    <td>
                      <table width= "348">
                        <tr>
                          <td width="145">
                          </td>
                          <td>
                            <!--Button zum hochladen. beim klicken wird die Seite die diese geöffnet hat neu geladen
                            sprich die Hauptseite und diese automatisch geschlossen-->
                            <input type="submit" name="submit" value="Hochladen"
                                   onclick="opener.location.reload();self.close()" >
                          </td>
                        </tr>
                      </table>
                    </td>
                  </tr>
                </table>
              </form>
              <?
              break;
            }
            ?>
          </body>
        </html>

        Kommentar


        • #5
          Hab ja schon jede Menge unsicheren Code gesehen, aber deiner übertrifft alles: Wie kommst du auf die Idee, das MySQL-Passwort per GET zu übergeben?

          Kommentar


          • #6
            das ist eine Seite die NUR und AUSCHLIEßLICH auf meinem Rechner läuft. Allerdings wird das gaze GET noch auf POST umgestellt. Aber ich würde ganz gerne hier noch ein paar Tipps hören wie ich Code übersichtlicher mache und wie ich vieleicht die iene oder andere Funktion besser machen könnte. Ist das Case hier sinnvoll oder doch liebe ein if, else?

            Deswegen frage ich ja hier nach. Aber du hast natürlich recht PW per GET ist mehr als unsicher.
            Danke

            Kommentar


            • #7
              wie bekomme ich im Firefox den kompletten Pfad von einem input = file feld ausgegeben?

              Wenn ich folgendes Inpufeld habe:

              PHP-Code:
              <form method="get" action="<?php echo $_SERVER['PHP_SELF']; ?>">
              <input type="file" size ="10" class="file" name="cover_pfad_neu">
              <input type="submit" name="submit" value="Hochladen">
              </form>
              und es mit folgender zeile wieder lade:

              PHP-Code:
              $cover_pfad_neu = isset($_GET['cover_pfad_neu']) ? $_GET['cover_pfad_neu'] : ''
              bekomme ich im IE den kompletten Pfad von der ausgewählten Datei und im Firefox nur den Dateinamen.

              Wie bekomme ich im Firefox auch den kompletten Pfad?

              Hoffe mir kann jemand helfen?

              Kommentar


              • #8
                Ein File-Input in einer GET-Form ist Käse. Lies im PHP-Manual den Abschnitt zu file uploads!

                Kommentar


                • #9
                  Original geschrieben von Jache84
                  Wie bekomme ich im Firefox auch den kompletten Pfad?
                  Gar nicht [1], geht dich einen feuchten an :-)


                  [1] Oder höchstens clientseitig.
                  I don't believe in rebirth. Actually, I never did in my whole lives.

                  Kommentar


                  • #10
                    ich denke clientseitig reicht mir, denn ich brauche es nur, das der Firefox mir keinen Fehler in folgender funktion bringt:

                    PHP-Code:
                    $pixel getimagesize($cover_pfad_neu); 
                    da bringt mir der Firefox den Fehler das er es nicht ausführen kann da nur der Filename und nicht der relative Pfad drin steht.
                    Im IE geht es ohne Probleme.

                    Und warum bringt mir "Zend Developer" (ja habe ich mri zu testzwecken wegen dem Debugger mal runtergeladen einen Fehler in dem Script:

                    PHP-Code:
                    while ($row_mp_id mysql_fetch_array($sqlquerry_mp_idMYSQL_NUM))
                          { 
                    er sagt mir
                    Fehler: zuweisung in bedingung.
                    In der Fehlerbeschreibung steht das ich ein doppeltes gleich nehmen soll, aber das geht doch in dem Fall gar nicht, oder?

                    Und wieso bringt mir der Zend Editor nen fehler wegen meinem

                    PHP-Code:
                     or die ("Fehler in SQL : $sql"); 
                    er sagt mir Globale Variable $sql wird vor Ihrer Dekleration benutzt und Ergebnis des Ausdrucks wird nie verwendet.

                    okay wegen dem ergebnis ist klar, ist halt nie ein Fehler in einem SQl Statement, oder gibt es einen anderen Grund? aber wieso wird $sql schon mal benutzt, ich finde nicht wo.

                    Danke schon mal im Voraus.

                    @wahsaga, du kannst ja auch freundlich sein
                    Zuletzt geändert von Jache84; 14.11.2006, 21:36.

                    Kommentar


                    • #11
                      Original geschrieben von Jache84
                      ich denke clientseitig reicht mir, denn ich brauche es nur, das der Firefox mir keinen Fehler in folgender funktion bringt:
                      PHP-Code:
                      $pixel getimagesize($cover_pfad_neu); 
                      Das ist PHP, das führst du serverseitig aus - also reicht clientseitig dir nicht.
                      I don't believe in rebirth. Actually, I never did in my whole lives.

                      Kommentar


                      • #12
                        okay gut, Danke.

                        Was ist mit den zwei anderen Fehlern, ist das normal?

                        Kommentar


                        • #13
                          Ja das ist normal. Zend hat auf seinen Developer-Seiten selbst Code wie diesen
                          PHP-Code:
                          if ($row mysql_fetch_assoc(...)) 
                          Ist auch prinzipiell nichts daran auszusetzen, wenn man sich des Unterschieds zwischen = und == bewußt ist.
                          Der Zend-Analyzer geht davon aus, dass du den Unterschied nicht kennst, wohl um Einsteiger vor typischen Fehlern zu bewahren.
                          Ähnliches gilt auch für Ausdrücke mit or wie in deinem Fall or die().

                          @wahsaga: Welche Möglichkeiten gibt es denn clientseitig? Imho kann Javascript nicht auf den Inhalt von Fileinputs zugreifen, weder auf den darin angezeigten Pfad noch auf die Datei selbst.

                          Kommentar


                          • #14
                            ja Danke, das ist doch mal ne Aussage.

                            Ja unter schied zwischen = und == weiß ich. okay jetzt wegen dem 'or' raff ich es auch ich habe in diesem 'or' nicht den 'Operator' gesehen.

                            danke
                            Zuletzt geändert von Jache84; 14.11.2006, 23:58.

                            Kommentar


                            • #15
                              das ist eine Seite die NUR und AUSCHLIEßLICH auf meinem Rechner läuft. Allerdings wird das gaze GET noch auf POST umgestellt
                              Und wie kommst du drauf, dass POST sicherer wäre ? Und wieso überhaupt das DB PW übertragen ? Belege im Script mit der DB Verbindung doch einfach zwei Vars mit den entsprechenden Werten
                              PHP-Code:
                              $db_user 'root';
                              $db_pw 'Ich bin ToTal GEheim';
                              $db mysql_connect('127.0.0.1',$db_user,$db_pw); 
                              und verwende diese dann für die Verbindung. Der Client muss doch nichts von den DB Pasworten und Usern wissen.

                              Gruss

                              tobi
                              Gutes Tutorial | PHP Manual | MySql Manual | PHP FAQ | Apache | Suchfunktion für eigene Seiten

                              [color=red]"An error does not become truth by reason of multiplied propagation, nor does truth become error because nobody sees it."[/color]
                              Mohandas Karamchand Gandhi (Mahatma Gandhi) (Source)

                              Kommentar

                              Lädt...
                              X