0

Basically I am trying to make it check if the user has their ID in the database and if they do have it in there it updates it putting $page as 'lastpage' in the database. It also checks to make sure that the value it has set inside there now is less than $page, if it isn't than it does nothing.

If the user doesn't have there ID in there than it should add it with whatever $page is set to.

The problem is that it isn't updating in the database at all.

This is the code I got so far, anyone got ideas?

session_start();
if(!isset($_SESSION['id'])) {
header("Location: ../../index.php");
} else {

}
    include '../../connect.php';
    include '../../users_func.php';
    $id = $_SESSION['id'];
    $page = 3;

    $sql_chk = " select * from html where id = '$id' and lastpage = '$page' ";
    $rs_chk = mysql_query($sql_chk);
    $num_chk = mysql_num_rows($rs_chk);

    if ($num_chk == 0) {
        mysql_query("INSERT INTO `html` (`id`, `lastpage`) VALUES ('$id', '$page') ");
    } else {
       $sql = "UPDATE html SET lastpage='$page' WHERE id='$id' and lastpage < $page";
       mysql_query($sql) or die("MYSQL Query Failed : " . mysql_error());
    }
Kevin Harrison
  • 335
  • 1
  • 10
  • 1
    Your code is vulnerable to some pretty nasty SQL Injection. You also should not use the mysql extension in PHP as it's deprecated. Look into PDO or MySQLi, and parameterized queries to protect yourself from attacks. – jdp Oct 08 '13 at 20:55
  • I know how to convert it to Mysqli, is that all I should do to make it less vulnerable? Also will that fix the problem I'm having? – Kevin Harrison Oct 08 '13 at 20:56
  • 1
    Won't answer your question, but it's still very important. Here's a good explanation of how to prevent SQL Injection using either PDO or MySQLi: http://stackoverflow.com/a/60496/324307 – jdp Oct 08 '13 at 21:00

2 Answers2

1

Your code is overly verbose, and those 3 queries would be replaced with a single

INSERT INTO html (id, lastpage) VALUES ($id, $page)
ON DUPLICATE KEY UPDATE lastpage = IF(lastpage < VALUES(lastpage), VALUES(lastpage), lastpage)
Marc B
  • 356,200
  • 43
  • 426
  • 500
  • So I should replace my queries with this code? mysqli_query($link, "INSERT INTO html (id, lastpage) VALUES ($id, $page) ON DUPLICATE KEY UPDATE lastpage = IF(lastpage < VALUES(lastpage), VALUES(lastpage), lastpage)") – Kevin Harrison Oct 08 '13 at 21:00
  • Because that doesnt work at all and if there were a database connection error it would say on the page. There most be something wrong. What about where it says if(lastpage < values(lastpage) shouldn't it be if(page < values(lastpage) ? – Kevin Harrison Oct 08 '13 at 21:07
1

I know you are going to change the code to mysqli or PDO and all, but your existing code should check for errors

$result = mysql_query($sql);
if (!$result) {
    die('Invalid query: ' . mysql_error());
}

In your code, it's quite possible mysql_num_rows($rs_chk) is checking on an invalid resource $rs_chk.

crafter
  • 6,246
  • 1
  • 34
  • 46