Marcel: Denkfehler im PHP-Script

SELF-Forum

Denkfehler im PHP-Script

Marcel Homepage des Autors
– Informationen zu den Bewertungsregeln

Hallo zusammen!

ich habe ein php-Script geschrieben, das eine Linkliste erzeugt. Diese Linkliste soll nach den beliebtesten Links sortiert werden. Jeder Link hat einen eigenen Wert, der aussagt wie oft er schon angeklickt wurde (mit IP-Sperre). Der Link der am häufigsten angeklickt wurde erscheint ganz oben in der Linkliste. Der mit den wnigsten Klicks ganz unten. Jetzt habe ich irgendwo in dem Script einen Denkfehler. Das Script ist bereits im Einsatz (siehe URL-Link). Der Fehler: Nach einer Weile sind immer neue Datensätze da, bei denen nur die Felder "id", "klicks" und "ip" gefüllt sind. Ich bin das Script immer wieder durchgegangen und konnte keinen Fehler finden und auch nie selbst reproduzieren das neue Datensätze erzeugt werden. Hat einer von euch eine Idee?

Danke und Gruß,
Marcel

Tabelle links:
id  url          name   text   klicks  ip
1   www.url1.de  name1  text1  10      297.145.45.158
2   www.url2.de  name2  text2  256     209.167.50.22
3   www.url3.de  name3  text3  5       62.155.239.248

<?PHP
$sql_database = "+++";
$sql_host = "+++";
$sql_user = "+++";
$sql_pass = "+++";

$db = mysql_connect($sql_host,$sql_user,$sql_pass) OR DIE ("<br>Keine Verbindung zur SQL-Datenbank!");
      mysql_select_db($sql_database,$db) OR DIE ("<br>Konnte Datenbank nicht finden!");

if(isset($id)) {
$count = mysql_query("SELECT * FROM links WHERE id = '$id'");
$wert = mysql_fetch_row($count);

$ip_alt = $wert[5];
$ip_neu = getenv("REMOTE_ADDR");
$link = $wert[1];

if ($ip_alt != $ip_neu)
{
  $hits = $wert[4]; $hits++;
  $update="REPLACE INTO links SET id = '$wert[0]', url = '$wert[1]', name = '$wert[2]', text = '$wert[3]', klicks = '$hits', ip = '$ip_neu'";
  mysql_query($update) OR DIE(" ".mysql_error());
}

header("Location: $link");
exit;
}

$seitentitel = "Treffpage &raquo; Start &raquo; Links";
$kategorie = "start";
include("/web/header.php");

echo "Die Links in unserer Linkliste sind nach ihrer Beliebtheit sortiert. Das heißt im Klartext: Je öfter ein Link von Besuchern angeklickt wurde, desto

weiter oben steht er in der Linkliste.";

$data = mysql_query("SELECT * FROM links ORDER BY klicks DESC");
$zaehler = "1";
while($zeile = mysql_fetch_row($data)) {

echo "<div style="margin-top: 15px;"><b>$zaehler. <a href="index.php?id=$zeile[0]" target="_blank">$zeile[2]</a></b> ($zeile[4] Aufrufe)<br>

\n$zeile[3]</div>\n";
$zaehler++;
}

include("/web/footer.php");
?>

Beitrag melden
–
Informationen zu den Bewertungsregeln

  1. Denkfehler im PHP-Script

    lulu Homepage des Autors
    – Informationen zu den Bewertungsregeln

    Huhu Marcel

    Hat einer von euch eine Idee?

    Ja, übergib einfach mal per URL einen beliebigen Wert für "id", z.B.

    http://www.treffpage.de/start/links/?id=hallo

    if(isset($id)) {

    Du benutzt noch "register globals=on", das solltest Du ändern.

    $count = mysql_query("SELECT * FROM links WHERE id = '$id'");

    Warum keine Fehlerbehandlung? Beim "connect" hast Du es ja gemacht, warum also hier nicht?

    if ($ip_alt != $ip_neu)

    Hier steckt der Fehler, diese Bedingung erfüllt nur zufällig aber nicht _zuverlässig_ das was Du meinst.

    Viele Grüße

    lulu

    --
    bythewaythewebsuxgoofflineandenjoytheday
    Beitrag melden
    –
    Informationen zu den Bewertungsregeln

    1. Denkfehler im PHP-Script

      Marcel
      – Informationen zu den Bewertungsregeln

      Hallo lulu!

      if ($ip_alt != $ip_neu)
      Hier steckt der Fehler, diese Bedingung erfüllt nur zufällig aber nicht _zuverlässig_ das was Du meinst.

      if ($ip_alt != $ip_neu and $wert[0] == $id)
      so scheint es zu funktionieren.

      Danke für deinen Tipp und alle anderen Antworten.
      Ich wünsche euch allen ein schönes und erholsames Wochenede.

      Liebe Grüße,
      Marcel

      Beitrag melden
      –
      Informationen zu den Bewertungsregeln

  2. Denkfehler im PHP-Script

    rodney rehm
    – Informationen zu den Bewertungsregeln

    was passiert wenn eine ID übergeben wird die es nicht gibt?

    if(isset($id)) {
    $count = mysql_query("SELECT * FROM links WHERE id = '$id'");
    $wert = mysql_fetch_row($count);

    übergeben wurde $id = 0;
    raus kommt $wert = false;

    $ip_alt = $wert[5];

    da nicht geprüft wurde ob $wert false oder ein array ist,
    ist $ip_alt = null;
    (oder was weiss ich was...)

    $ip_neu = getenv("REMOTE_ADDR");
    $link = $wert[1];

    if ($ip_alt != $ip_neu)
    {
      $hits = $wert[4]; $hits++;
      $update="REPLACE INTO links SET id = '$wert[0]', url = '$wert[1]', name = '$wert[2]', text = '$wert[3]', klicks = '$hits', ip = '$ip_neu'";

    der replace ist dran schuld, dass du neue datensätze bekommst... da kein alter überschrieben werden kann weil der primary_key (in deinem fall wohl ID) nicht existiert, wird ein neuer datensatz angelegt..
    vielleicht solltest du hier lieber UPDATE statt REPLACE INTO benutzen...

    mysql_query($update) OR DIE(" ".mysql_error());
    }

    header("Location: $link");
    exit;
    }

    Tabelle links:
    id  url          name   text   klicks  ip
    1   www.url1.de  name1  text1  10      297.145.45.158
    2   www.url2.de  name2  text2  256     209.167.50.22
    3   www.url3.de  name3  text3  5       62.155.239.248

    nächstes mal bitte angeben was dein primary_key ist... da dieser für das verhalten von REPLACE INTO ausschlaggebend ist!

    abgesehen davon verstehe ich dein IP-handling nicht so ganz...
    was passiert wenn...
    User 1 (IP: 1.1.1.1) klickt link_1 an.. IP wird auf 1.1.1.1 gesetzt.
    User 1 kann den link nicht erneut anklicken...
    klickt nun User 2 (IP: 2.2.2.2) auf link_1 wird die IP auf 2.2.2.2 gesetzt...
    jetzt kann doch User 1 wieder den link anklicken, da er eine andere IP hat als die, die in der datenbank gespeichert wurde...
    (und ja... das kann (und wird) innerhalb von sekunden passieren...)

    eben genanntes problem kannst du aber nur über eine weitere tabelle lösen... IP und SERVERZEIT speichern... und schauen ob diese IP innerhalb der letzten 4h gespeichert wurde... wenn ja wird der klick nicht gezählt... (datensätze die älter as 4h sind, können gelöscht werden... (vielleicht ab und an nen OPTIMZE TABLE laufen lassen ;)))

    HTH,
    Rod

    Beitrag melden
    –
    Informationen zu den Bewertungsregeln

  3. Denkfehler im PHP-Script

    Sven Rautenberg Homepage des Autors
    – Informationen zu den Bewertungsregeln

    Moin!

    Kommentare stehen im Code.

    Ursache für deinen beobachteten Effekt dürfte aber die Abwesenheit von jeglichem prüfenden Code sein, ob die übergebenen Parameter auch stimmen.

      

> <?PHP  
> $sql_database = "+++";  
> $sql_host = "+++";  
> $sql_user = "+++";  
> $sql_pass = "+++";  

// Ich halte es für sehr empfehlenswert, wenn man solche Angaben, die sich im gesamten Skript nie mehr ändern werden und auch nicht sollen, als Konstanten definiert.  

>   
> $db = mysql_connect($sql_host,$sql_user,$sql_pass) OR DIE ("<br>Keine Verbindung zur SQL-Datenbank!");  
>       mysql_select_db($sql_database,$db) OR DIE ("<br>Konnte Datenbank nicht finden!");  
>   
> if(isset($id)) {  

// Über die negativen Auswirkungen von register_globals=on wurde schon etwas gesagt. $_GET['id'] wäre hier der richtige Wert, auf den man zugreifen muß.  

>  $count = mysql_query("SELECT * FROM links WHERE id = '$id'");  

// "SELECT *" ist immer schlecht. Schreibe explizit die Spalten auf, welche du abfragen willst. Damit definierst du gleichzeitig auch die Reihenfolge, in der die Spalten im Ergebnis erscheinen, denn...  

>  $wert = mysql_fetch_row($count);  
>   
>  $ip_alt = $wert[5];  

// ...an dieser Stelle vertraust du darauf, dass die fünfte Spalte deines "SELECT *" auch tatsächlich die IP-Adresse ist - es könnte aber auch was ganz anderes sein, ohne dass du es bemerken würdest. Deshalb:  
// 1. Nie "SELECT *". Immer "SELECT id, ip, url".  
// 2. Nie mysql_fetch_row(), immer mysql_fetch_array() bzw. mysql_fetch_assoc().  
// 3. Der Zugriff auf die Ergebniszeile geht dann mit $wert['ip'] - da sieht man sofort, welche _Bedeutung_ die Spalte hat. Das erleichtert auch die Fehlersuche.  

>  $ip_neu = getenv("REMOTE_ADDR");  
>  $link = $wert[1];  

// Hier genau dasselbe. $wert['link'] wäre schlauer, das Umkopieren in eine neue Variable kann man sich sparen.  

>   
>  if ($ip_alt != $ip_neu)  
>  {  
>   $hits = $wert[4]; $hits++;  

// Umständlich. Einfacher ist $hits = $wert['hits'] + 1; Oder gar $wert['hits']++;  

>   $update="REPLACE INTO links SET id = '$wert[0]', url = '$wert[1]', name = '$wert[2]', text = '$wert[3]', klicks = '$hits', ip = '$ip_neu'";  

// Nun ja, wenn hier mal keine Variablen durcheinander kommen.  

>   mysql_query($update) OR DIE(" ".mysql_error());  
>  }  
>   
>  header("Location: $link");  

// Ist der Link eigentlich eine vollwertige HTTP-URL? Die Tabelle oben hatte keine Protokollangabe.  

>  exit;  
> }  
>   
> $seitentitel = "Treffpage &raquo; Start &raquo; Links";  
> $kategorie = "start";  
> include("/web/header.php");  
>   
> echo "Die Links in unserer Linkliste sind nach ihrer Beliebtheit sortiert. Das heißt im Klartext: Je öfter ein Link von Besuchern angeklickt wurde, desto weiter oben steht er in der Linkliste.";  
>   
> $data = mysql_query("SELECT * FROM links ORDER BY klicks DESC");  

// "SELECT *" schon wieder...  

> $zaehler = "1";  
> while($zeile = mysql_fetch_row($data)) {  

// mysql_fetch_row() schon wieder...  

>   
>  echo "<div style=\"margin-top: 15px;\"><b>$zaehler. <a href=\"index.php?id=$zeile[0]\" target=\"_blank\">$zeile[2]</a></b> ($zeile[4] Aufrufe)<br>  
>   
> \n$zeile[3]</div>\n";  
>  $zaehler++;  
> }  
>   
> include("/web/footer.php");  
> ?>
    • Sven Rautenberg
    Beitrag melden
    –
    Informationen zu den Bewertungsregeln

    1. Denkfehler im PHP-Script

      Ilja
      – Informationen zu den Bewertungsregeln

      yo Moin,

      $count = mysql_query("SELECT * FROM links WHERE id = '$id'");
      // "SELECT *" ist immer schlecht. Schreibe explizit die Spalten auf, welche du abfragen willst. Damit definierst du gleichzeitig auch die Reihenfolge, in der die Spalten im Ergebnis erscheinen, denn...

      es wird ein wenig zur manie, das * zu verurteilen und geht meiner meinnung zuweit, wenn man betont, dass es "immer" schlecht sei. es gibt sehr wohl sinnvolle anwendungen von dem * nämlich dann, wenn man alle spalten anzeigen will, auch wenn sich das datendesign der entsprechenden tabelle verändert hat oder einem die spaltennamen beim schreiben der programme gar nicht bekannt sind. scripte in form von phpmyadmin können davon profitieren.

      Ilja

      Beitrag melden
      –
      Informationen zu den Bewertungsregeln