3

I have a comics website where I'd like to allow users to vote once per comic and once per piece of artwork.

There seems to be two problems with my code:

1) I only want one user voting once per image... so I want to capture their information and store it in a database. I have a ON DUPLICATE KEY UPDATE, but it gives me the following syntax error even though I haven't found ANYTHING wrong with it:

Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'table = VALUES(table), imgid = VALUES(imgid)' at line 7 

An example of it allowing multiple entries into the database for the same IP:

enter image description here

2) It's still allowing one user to vote multiple times.

    $sql = "SELECT ip FROM votes WHERE ip = \"".$_SERVER['REMOTE_ADDR']."\" AND table_name = $table AND imgid = $imgid";

$result = $mysqli->query($sql);
var_dump($result);

Full code:

<?php 
include 'dbconnect.php';
$site = $_GET['_site'];
$imgid = intval($_GET['_id']);
$input = $_GET['_choice'];


if ($site == "artwork") {
   $table = "artwork";
}
else {
   $table = "comics";
}

$result = $mysqli->query("SELECT like_count, dislike_count FROM $table WHERE id = $imgid");

list($likes, $dislikes) = $result->fetch_array(MYSQLI_NUM);

$sql = "INSERT INTO 
            votes (ip, table_name, imgid) 
        VALUES 
            (\"".$_SERVER['REMOTE_ADDR']."\", \"$table\", $imgid)
        ON DUPLICATE KEY UPDATE
            ip = VALUES(ip),
            table = VALUES(table),
            imgid = VALUES(imgid)";

$mysqli->query($sql);
echo $mysqli->error;
echo "<br/>";

$sql = "SELECT ip FROM votes WHERE ip = '".$_SERVER['REMOTE_ADDR']."' AND table_name = '$table' AND imgid = $imgid";

$result = $mysqli->query($sql);
echo $mysqli->error;

if ($result->num_rows == 0) { 
    if ($input == "like") {
        $sql = "UPDATE $table SET like_count = like_count + 1 WHERE id = $imgid";
        $mysqli->query($sql);           
        $likes++;
    }
    else if ($input == "dislike") {
        $sql = "UPDATE $table SET dislike_count = dislike_count + 1 WHERE id = $imgid";
        $mysqli->query($sql);
        $dislikes++;        
    }
echo "Likes: " . $likes . ", Dislikes: " . $dislikes;
}
else {
    echo "You have already voted";
}
mysqli_close($mysqli);

?>

Echoing out sql:

echo "sql: ". $sql;

Produces:

sql: INSERT INTO votes (ip, table_name, imgid) VALUES ("127.0.0.1", "comics", 34) ON DUPLICATE KEY UPDATE ip = VALUES(ip), table = VALUES(table), imgid = VALUES(imgid)

Any help would be greatly appreciated!

user3871
  • 12,432
  • 33
  • 128
  • 268
  • You know that IPs are not unique by user, right? – markus Nov 24 '12 at 00:39
  • @markus-tharkun I couldn't think of another way to ensure that only a user could only vote once per comic. – user3871 Nov 24 '12 at 00:40
  • Users need to have an account for it to really work! Just use some openID and a simple sign on. – markus Nov 24 '12 at 00:41
  • @markus-tharkun It's supposed to be an open-browsing comics website. The last thing I wanted to do is make users log in. Is getting their IPs not enough of a catch? – user3871 Nov 24 '12 at 00:43
  • @markus-tharkun hey markus, any other thoughts why those two errors may be happening? – user3871 Nov 24 '12 at 01:01
  • For some reason you're not using [prepared statements](http://php.net/manual/en/mysqli.prepare.php) with `mysqli` could be creating all kinds of [SQL injection bugs](http://bobby-tables.com/php). Using string interpolation to construct SQL queries is not a very safe technique. Placeholders will always properly handle string values so you don't end up stuck on silly problems like this. – tadman Nov 24 '12 at 02:59
  • @tadman Thanks Tadman. I am looking to go live with this website tomorrow though... I was going to rework everything into prepared statements after. Do you have any advice about this specific problem? It's my final issue before I launch my comics site – user3871 Nov 24 '12 at 03:04

2 Answers2

2

What you're seeing is table is one of the MySQL reserved words but you're trying to use it as a column name. Your column is actually called table_name based on your question, though.

A query with placeholders looks like:

INSERT INTO votes (ip, table_name, imgid) 
  VALUES (?, ?, ?)
  ON DUPLICATE KEY UPDATE
    ip=VALUES(ip),
    table_name= VALUES(table_name),
    imgid=VALUES(imgid)

Remember with mysqli you can execute this query by doing this:

$sth = $mysqli->prepare("...");
$sth->bind_param("sss", $_SERVER['REMOTE_ADDR'], $table, $imgid);
$sth->execute();

The documentation describes this process in more detail, but the "sss" thing refers to three strings, and the three values are passed in as parameters.

You should probably be using PDO as it's a lot less fussy to use than mysqli. Even better would be to use a database framework like Doctrine to do a lot of the SQL dirty work for you. Even better still would be to use a framework like CodeIgnighter, CakePHP or FuelPHP to give you a foundation to build on. Constructing applications by hand from the ground up is extremely time-consuming and significantly more error prone.

Another thing to note is you should try and use consistent naming in your code. You refer to $table as a value for table_name, so it should presumably be $table_name to start with.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • Thank you for advice on other frameworks! I've promised my fans i'll release the site soon, so I'll change to cake php after I release it. Also, I'm never using "table", I'm always using it as a variable "$table", which should be okay, right? My column for "votes" table is indeed called "table_name", but I first need to get the like/dislike count from the "comics" or "artwork" tables. – user3871 Nov 24 '12 at 16:04
  • If a variable correlates with the `table_name` column, I'm saying it should probably be called `$table_name` to avoid confusion. – tadman Nov 24 '12 at 19:55
1

You query fails but you do not check that and try to use $result but it is not an object bug false. And it fails, because you should be like this:

$sql = "SELECT ip FROM votes WHERE ip = '".$_SERVER['REMOTE_ADDR']."' AND table_name = $table AND imgid = $imgid";

(string values are quoted with single quote ')

Marcin Orlowski
  • 72,056
  • 11
  • 123
  • 141
  • I tried that before. This doesn't seem to be working, and produces the same error :/ – user3871 Nov 24 '12 at 00:42
  • sure. `echo $mysqli->error;` after your failing query. or `echo $sql;` and manual attempt in console or phpmyadmin – Marcin Orlowski Nov 24 '12 at 01:17
  • Error: Unknown column 'comics' in 'where clause'. But I don't see what the issue is.. – user3871 Nov 24 '12 at 01:23
  • 3
    ain't message clear enough? ;) check how is your column named **in db** - maybe you got typo, maybe you got trailing space? if none dump table struct and add to question – Marcin Orlowski Nov 24 '12 at 01:40
  • Lol. K I found it... needed tick marks around '$table' because it's looking for a string. Also, Do you happen to know why ON DUPLICATE KEY UPDATE is allowing multiple entries of the same ip? The code is above – user3871 Nov 24 '12 at 02:06
  • Have you set the IP and table_name as primary key? – take Nov 24 '12 at 02:13
  • @take IP is a Primary Key, yes. – user3871 Nov 24 '12 at 02:43
  • @WebnetMobile.com I've updated my question. I am trying to use ON DUPLICATE KEY UPDATE (following this: http://stackoverflow.com/questions/450682/insert-into-on-duplicate-key-update-for-multiple-items/450695#450695) but have had NO luck :( – user3871 Nov 24 '12 at 02:44
  • @WebnetMobile.com any other ideas? – user3871 Nov 24 '12 at 04:38
  • @WebnetMobile.com I'm not sure what you mean by "example query". The code above is up to date, along with the errors I am getting. – user3871 Nov 24 '12 at 16:11
  • there's no schema. "example query" is the query you try to execute. just `echo $sql;` and paste the output. – Marcin Orlowski Nov 24 '12 at 16:14
  • @WebnetMobile.com (ALSO ADDED ABOVE) sql: INSERT INTO votes (ip, table_name, imgid) VALUES ("127.0.0.1", "comics", 34) ON DUPLICATE KEY UPDATE ip = VALUES(ip), table = VALUES(table), imgid = VALUES(imgid) – user3871 Nov 24 '12 at 16:26
  • @WebnetMobile.com Any ideas on my response? – user3871 Nov 26 '12 at 15:56