How to fix this Warning being shown on my website

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

 

Change

$check2=mysql_num_rows($check_exec2);

PHP:

To

$check2=mysql_num_rows($check_exec2) or die(mysql_error());

PHP:

 

Add after $delete_exec2=mysql_query($delete_query2); another IF statement

if (mysql_num_rows($delete_query2)) { ... then the while { } and then }

 

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!

 

It's for debugging. You can change it afterwards. Life will go on.

 

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.

 

Not even for debugging, there are better methods to do debugging.

 

Thanks, I will try and implement your fix tonight. Would be good if you pasted the actual code for me...

 

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?

 

this has now been fixed by repairing a table using phpmyadmin

 

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, 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.

 

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"

 

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.

 

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.

 

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...