PHP ip logger error

Question

i have this code :

   <?php
    echo $_POST['id-input'];
    $time = time();
    $ip=$_SERVER['REMOTE_ADDR'];
    $userid_c = $_POST['id-input'];
    $con = mysql_connect("localhost","username","password");
    if (!$con)
    {
        die('Could not connect: ' . mysql_error());
    }

     mysql_select_db("kritya20_data", $con);
     $sql = "SELECT * FROM users_online WHERE user_id='$userid_c'";
     $query = mysql_query($sql);
     $rows_only_c = mysql_num_rows($query);
     if($rows_only_c == 0)
     {
        $sql1= "INSERT INTO users_online (user_id , online_since , lastup , lobbieid)
        VALUES ('$userid_c' , '$time' , '$time' , '1')";
        $query1 = mysql_query($sql1);
     }
     elseif($rows_only_c==1)
     {
        $sql2 = "UPDATE users_online
        SET lastup='$time'
        WHERE user_id='$userid_c' ";
        $query2 = mysql_query($sql2);
     }
     $sql3 = "SELECT * FROM iptable WHERE user_id='$userid_c'";
     $query3 = mysql_query($sql3);
     $row_ip = mysql_num_rows($query3);
     if($row_ip==0)
     {
          $sql4 = "INSERT INTO iptable (user_id , ip , added)
          VALUES ('$userid_c' , '$ip' , '$time')";
          $query4= mysql_query($sql4);
    }
    elseif($row_ip>0)
    {
        $mt=0;
        $mf=0;
        $sql4 = "SELECT * FROM iptable WHERE user_id='$userid_c'";
        $query4 = mysql_query($sql4);
        while($row=$query4)
        {
          if($ip==$row[2])
          {
              $mt++;
          }
          else
          {
              $mf++;
          }
        }
        if($mf!=0)
        {
            $sql5 = "INSERT INTO iptable (user_id , ip , added)
            VALUES ('$userid_c' , '$ip' , '$time')";
            $query5= mysql_query($sql5);
        }
    }
?>

where have been i going wrong ? :O because either it only adds for first time and then whenever the ip changes it doesnt.

I don't think the code is that terrible (although it has [SQL injection vulnerabilities](http://php.net/manual/en/security.database.sql-injection.php) that need to be fixed), but you really need to do some debugging first, try to isolate where the problem is, and add that more detailed information. We can't do those steps for you — Pekka, Jun 05 '11 at 09:25

score 1 · Accepted Answer · answered Jun 05 '11 at 09:36

I have modified the code below for improved layout, descriptive variable names, added debugging, suggested logic improvements which simplify the code and fixed your bug.

Please read and try to use some of this in your future code, it will help you immensely with debugging or just writing code people can understand. I not trying to sound like an condescending ass, we all started somewhere, and I still make silly mistakes!

If there is something I have done you do not understand (either the code, or why I did it that way) please ask.

<?php
    $debug_on = true;

    #Variable section
    $time = time();
    $ip=$_SERVER['REMOTE_ADDR'];
    $userid_c = $_POST['id-input'];

    if($debug_on) {
        echo "<p>time: '$time'</p>";
        echo "<p>ip: '$ip'</p>";
        echo "<p>userid_c: '$userid_c'</p>";
    }

    #Connect to database
    $con = mysql_connect("localhost","username","password");
    if (!$con) {
        die('Could not connect: ' . mysql_error());
    }
    mysql_select_db("kritya20_data", $con) or die('Could not select database');

    #Insert or update users_online
    $user_exits_sql = "SELECT * FROM users_online WHERE user_id='$userid_c'";
    $user_exits_query = mysql_query($user_exits_sql);
    $user_exists = mysql_num_rows($user_exits_query);
    if($user_exists == 0) {
        $insert_user_sql = "INSERT INTO users_online (user_id , online_since , lastup , lobbieid) VALUES ('$userid_c' , '$time' , '$time' , '1')";
        if($debug_on) {
            echo "<p>No record found in `users_online` - Inserting new record</p>";
            echo "<p>$insert_user_sql</p>";
        }
        mysql_query($insert_user_sql);
    }
    else { # no need for 'elseif($rows_only_c==1)' as this is the only other option
        $update_user_sql = "UPDATE users_online
                            SET lastup='$time'
                            WHERE user_id='$userid_c' ";
        if($debug_on) {
            echo "<p>Record found in `users_online` - Updating existing record</p>";
            echo "<p>$update_user_sql</p>";
        }
        mysql_query($update_user_sql);
    }

    #Insert or update iptable
    $user_ip_exists_sql = "SELECT * FROM iptable WHERE user_id='$userid_c'";
    $user_ip_exists_query = mysql_query($user_ip_exists_sql);
    $user_ip_exists = mysql_num_rows($user_ip_exists_query);
    if($user_ip_exists==0) {
        $user_ip_insert_sql = "INSERT INTO iptable (user_id , ip , added)
                               VALUES ('$userid_c' , '$ip' , '$time')";
        if($debug_on) {
            echo "<p>No record found in `iptable` - Inserting new record</p>";
            echo "<p>$user_ip_insert_sql</p>";
        }
        mysql_query($user_ip_insert_sql);
    }
    else { # 'elseif($row_ip>0)'
        $mt = 0;
        $mf = 0;
        $iptable_sql = "SELECT * FROM iptable WHERE user_id='$userid_c'";
        $iptable_query = mysql_query($iptable_sql);
        #The next line is your bug,
        #should read while($row = $iptable_query->mysql_fetch_array())
        while($row = $iptable_query) {
          if($ip==$row[2]) {
              $mt++;
          }
          else {
              $mf++;
          }
        }
        # For bonus points you could use $iptable_query->mysql_fetch_row(), then $row->ip rather than $row[2]. 
        # This makes it obvious what variable you are trying to access.
        if($mf != 0) {
            $iptable_insert_sql = "INSERT INTO iptable (user_id , ip , added)
                                   VALUES ('$userid_c' , '$ip' , '$time')";
            if($debug_on) {
                echo "<p>No record found in `iptable` for user with this ip address - Inserting new record</p>";
                echo "<p>$iptable_insert_sql</p>";
            }
            mysql_query($iptable_insert_sql);
        }

        # This whole section seems to be:
        # If user doesnt exist, add them,
        # If user does exist, but not for this ip, add them
        # If this is right this could be made much simpler by changing $user_ip_exists_sql to be where user_id='$userid_c' and ip='$ip'
        # then you could remove all the $mt, $mf stuff
    }
?>

Good effort to read all this long code, but it is still vulnerable to sql injections. assuming the $user_id is an integer, you can add `(int)$_POST['id-input'];` to make sure nothing else can be entered — Ibu, Jun 05 '11 at 09:43
Good point, pear allows parameterized statements which handle injections, as does mysqli and pdo in php5, take a look at http://en.wikibooks.org/wiki/PHP_Programming/SQL_Injection — mattumotu, Jun 05 '11 at 09:53
or take a look at http://stackoverflow.com/questions/6198104/reference-what-is-a-perfect-code-sample-using-the-mysql-extension/6198584#6198584 for advise on using mysql_query and the like — mattumotu, Jun 05 '11 at 10:07

Sabeen Malik · Answer 2 · 2011-06-05T09:30:27.870

0

There could be some improvement to the code. However try changing while($row=$query4) to while($row=mysql_fetch_array($query4))

Also as you are running $sql3 = "SELECT * FROM iptable WHERE user_id='$userid_c'"; twice, why not just use the resultset from the first query instead of sending another query to the database.

edited Jun 05 '11 at 09:30

answered Jun 05 '11 at 09:24

Sabeen Malik

10,816
4
33
50

PHP ip logger error

2 Answers2