1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

How to fix this Warning being shown on my website

Discussion in 'PHP' started by mistermix, Nov 21, 2013.

  1. #1
    hi folks

    A couple of days ago one of my websites started displaying the following error message at the top of every page:

    The code on line 355 of functions.php is (line 3):

    $delete_query2 = "SELECT ipaddress,usertime FROM userlog";
        $delete_exec2 = mysql_query ($delete_query2);
        while ($delete_result2 = mysql_fetch_array ($delete_exec2))
        {
            $delete_time2 = $delete_result2["usertime"];
            if ($delete_time2 < ($time - 86400))
    PHP:
    The code on line 388 of functions.php is (line 3):

    $check_query2 = "SELECT ipaddress FROM userlog WHERE ipaddress='$myip'";
        $check_exec2 = mysql_query ($check_query2);
        $check2 = mysql_num_rows ($check_exec2);
        if ($check2 == 0)
    PHP:
    I basically have no idea how to fix this, any help is appreciated. Initially i'd like to know how to atleast prevent visitors from seeing the error message. The site is actually functioning as normal, apart from error shown at the top of every page.

    If you need more info, please do ask.

    Thanks
     
    mistermix, Nov 21, 2013 IP
  2. CoreyPeerFly

    CoreyPeerFly Notable Member Affiliate Manager

    Messages:
    394
    Likes Received:
    24
    Best Answers:
    5
    Trophy Points:
    240
    #2
    Change
    $check2=mysql_num_rows($check_exec2);
    PHP:
    To

    $check2=mysql_num_rows($check_exec2) or die(mysql_error());
    PHP:
     
    CoreyPeerFly, Nov 21, 2013 IP
  3. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #3
    Add after $delete_exec2=mysql_query($delete_query2); another IF statement

    if (mysql_num_rows($delete_query2)) { ... then the while { } and then }
     
    EricBruggema, Nov 21, 2013 IP
  4. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #4
    OMG!~don't do that, never use DIE! its so wrong and the whole damn script stops when you DIE! lol

    Use my example, much better... oh and avoid using die after queries... if you need DIE? you need to start learning more PHP for real!
     
    EricBruggema, Nov 21, 2013 IP
  5. CoreyPeerFly

    CoreyPeerFly Notable Member Affiliate Manager

    Messages:
    394
    Likes Received:
    24
    Best Answers:
    5
    Trophy Points:
    240
    #5
    It's for debugging. You can change it afterwards. Life will go on. :)
     
    CoreyPeerFly, Nov 21, 2013 IP
  6. PoPSiCLe

    PoPSiCLe Illustrious Member

    Messages:
    4,623
    Likes Received:
    725
    Best Answers:
    152
    Trophy Points:
    470
    #6
    And for the love of god, proper naming of the queries, please. $delete_query2? It doesn't delete anything, it selects. Just... no. Just no.
     
    PoPSiCLe, Nov 21, 2013 IP
  7. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #7
    Not even for debugging, there are better methods to do debugging.
     
    EricBruggema, Nov 21, 2013 IP
  8. mistermix

    mistermix Active Member

    Messages:
    2,326
    Likes Received:
    85
    Best Answers:
    0
    Trophy Points:
    90
    #8
    Thanks, I will try and implement your fix tonight. Would be good if you pasted the actual code for me...
     
    mistermix, Nov 22, 2013 IP
  9. mistermix

    mistermix Active Member

    Messages:
    2,326
    Likes Received:
    85
    Best Answers:
    0
    Trophy Points:
    90
    #9
    Ok tried to implement this but it either gave more errors or totally took the site down. its probably because the site is written with bad/old code.

    Here is a wider snippet of the code giving the error (lines 29 and 62):

    function log_user()
    {
        $myid = $_SESSION['loggedinuserid'];
        if (!$myid)
        $myid = "";
        if ($_SERVER['HTTP_X_FORWARD_FOR'])
        {
            $myip = $_SERVER['HTTP_X_FORWARD_FOR'];
        }
        else
        {
            $myip = $_SERVER['REMOTE_ADDR'];
        }
        $time = time();
        $delete_query = "SELECT userip,usertime FROM online_users";
        $delete_exec = mysql_query ($delete_query);
        while ($delete_result = mysql_fetch_array ($delete_exec))
        {
            $delete_time = $delete_result["usertime"];
            if ($delete_time < ($time - 300))
            {
                $delete_ip = $delete_result["userip"];
                $query = "DELETE FROM online_users WHERE userip='$delete_ip'";
                $exec = mysql_query ($query);
            }
        }
        $delete_query2 = "SELECT ipaddress,usertime FROM userlog";
        $delete_exec2 = mysql_query ($delete_query2);   
        while ($delete_result2 = mysql_fetch_array ($delete_exec2))
        {
            $delete_time2 = $delete_result2["usertime"];
            if ($delete_time2 < ($time - 86400))
            {
                $delete_ip = $delete_result2["ipaddress"];
                $query2 = "DELETE FROM userlog WHERE ipaddress='$delete_ip'";
                $exec2 = mysql_query ($query2);
            }
        }
        $check_query = "SELECT userip FROM online_users WHERE userip='$myip'";
        $check_exec = mysql_query ($check_query);
        $check = mysql_num_rows ($check_exec);
        if ($check == 0)
        {
            $insert_query = "INSERT INTO online_users (userid,userip,usertime) VALUES ('$myid','$myip','$time')";
            $insert_exec = mysql_query ($insert_query);
        }
        else
        {
            if ($myid != 0)
            {
                $update_query = "UPDATE online_users SET usertime=$time, userid='$myid' WHERE userip='$myip'";
                $update_exec = mysql_query ($update_query);
            }
            else
            {
                $update_query = "UPDATE online_users SET usertime=$time, userid='0' WHERE userip='$myip'";
                $update_exec = mysql_query ($update_query);
            }
        }
        $check_query2 = "SELECT ipaddress FROM userlog WHERE ipaddress='$myip'";
        $check_exec2 = mysql_query ($check_query2);
        $check2 = mysql_num_rows ($check_exec2);
        if ($check2 == 0)
        {
            $insert_query2 = "INSERT INTO userlog (userid,ipaddress,usertime) VALUES ('$myid','$myip','$time')";
            $insert_exec2 = mysql_query ($insert_query2);
        }
        else
        {
            if ($myid != 0)
            {
                $update_query2 = "UPDATE userlog SET usertime=$time, userid='$myid' WHERE ipaddress='$ipaddress'";
                $update_exec2 = mysql_query ($update_query2);
            }
            else
            {
                $update_query2 = "UPDATE userlog SET usertime=$time, userid='0' WHERE ipaddress='$myip'";
                $update_exec2 = mysql_query ($update_query2);
            }
    
        }
    }
    
    PHP:
    does this make it eaier?
     
    mistermix, Nov 22, 2013 IP
  10. mistermix

    mistermix Active Member

    Messages:
    2,326
    Likes Received:
    85
    Best Answers:
    0
    Trophy Points:
    90
    #10
    this has now been fixed by repairing a table using phpmyadmin
     
    mistermix, Nov 22, 2013 IP
  11. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #11
    I'd also point out that it's 2013, not 1997 -- lose the mysql_ functions for mysqli or PDO. I'm SO looking forward to when those are actually removed from PHP and/or start hemorrhaging errors when used. They actually already should be throwing errors by now; why not? 'split' does!
     
    deathshadow, Nov 22, 2013 IP
  12. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #12
    deathshadow, normaly i agree, but when a good function site gives just a couple of problems, you won't start rescripting it all.. (except if you are willing to grow... and see future in that..) but when the site is just for the fun of it... you fix the 'old' scripting.
     
    EricBruggema, Nov 22, 2013 IP
  13. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #13
    Unless of course there's a looming expiration date if you want to keep installing new versions of the language it's built in... and the corresponding security patches to that language. We've basically been told for EIGHT YEARS to stop doing that, and at this point it's like the makers of PHP are going to have to bend every last developer over the table to force the issue with people spouting attitudes about it like you just did. Apparently 8 years of being told not to use them and giant red warning boxes in the manual just aren't enough.

    Of course were that browser makers were as dilligent -- I'd love to see all the tags HTML 4 STRICT deprecated FIFTEEN YEARS AGO just up and stop working overnight... just to see all the folks who still have their heads wedged up 1997's ass run around like chickens with their heads cut off -- of course no, instead you have HTML 5 telling people "go ahead and sleaze out everything 1997 style, who cares about the past decade and a half of progress"
     
    deathshadow, Nov 22, 2013 IP
  14. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #14
    HTTP_X_FORWARD_FOR is a user defined variable and needs to be sanitised. Plus, it can contain a LIST of comma separated IPs if you're using multiple proxies. So you might want to make sure the supplied data is actually a valid IP address before using it.

    You can also replace most queries (if not all, haven't checked your entire code) with a single query such as:
    
    "DELETE FROM online_users WHERE usertime < " . ($time - 300)
    
    PHP:
    That's one query instead of one for fetching all entries, and then god knows how many to delete each individual entry.

    And as for the last block, you can run one UPDATE query, then check the number of affected rows, and only insert a new one if the returned number is zero.
     
    nico_swd, Nov 22, 2013 IP
    ryan_uk likes this.
  15. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #15
    You might want to add this paragraph to your signature. It's suitable in almost all topics on here. This, and the one about the giant red boxes on all mysql_* manual pages. :)
     
    nico_swd, Nov 22, 2013 IP
    ryan_uk likes this.
  16. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #16
    Yup and you are right, but its time vs money.... and yes there will be a time that you can't just fix errors and need to rescript the script but until that time... the site runs... and maby when that time comes the site goes down... but i get ur point... and agree with it...
     
    EricBruggema, Nov 22, 2013 IP