2
if ($_GET['action'] == "like")
{
mysql_query("UPDATE blog SET like=like+1 WHERE id=".$_GET['id']."");
header('Location: blog.php?id='.$_GET['id'].'');
}
else if ($_GET['action'] == "dislike")
{
mysql_query("UPDATE blog SET dislike = dislike+1 WHERE id = ".$_GET['id']."");
header('Location: blog.php?id='.$_GET['id'].'');
}

The "dislike" action works great! But the "like" one doesn't. It's close to be the same thing?

Can someone help me???

Frederick Marcoux
  • 2,195
  • 1
  • 26
  • 57
  • What's the sense of this ."" after your query? – Aurelio De Rosa Dec 06 '11 at 23:09
  • add basic error checking with mysql_error() –  Dec 06 '11 at 23:10
  • 1
    Welcome to Stack Overflow! You are not doing any error checking in your queries, so it's no wonder you're unable to see what happens when a query fails. How to show errors properly is outlined in the [manual on `mysql_query()`](http://php.net/mysql_query) or in this [reference question.](http://stackoverflow.com/questions/6198104/reference-what-is-a-perfect-code-sample-using-the-mysql-extension) – Pekka Dec 06 '11 at 23:10
  • 1
    Also, the code you show is vulnerable to [SQL injection](http://php.net/manual/en/security.database.sql-injection.php). Use the proper sanitation method of your library (like `mysql_real_escape_string()` for the classic mysql library), or switch to PDO and prepared statements. – Pekka Dec 06 '11 at 23:11
  • I hope that you do realize that accepting user input without escaping it for MySQL poses a severe security risk... But on topic: what does MySQL say if you paste the query in (e.g.) phpmyadmin (or echo `mysql_error()` like @Dagon suggests)? – Bart Dec 06 '11 at 23:12

2 Answers2

6

LIKE is a keyword. Use backticks : 

UPDATE blog SET `like`=`like`+1 ...

In general, it's much better not to name column after keywords (LIKE,CASE,SELECT,WHERE, etc).

Example 

mysql_query("UPDATE blog SET `like`=`like`+1 WHERE id='".
       mysql_real_escape_string($_GET['id'])."'");

Or if your id is integer, you can do in this particular case just .... WHERE id=".(int)$_GET['id']

a1ex07
  • 36,826
  • 12
  • 90
  • 103
1

A good plan would be to check the return value from mysql_error() to get the actual error from MySQL (You can check this by simply echo'ing mysql_error()).

Other than that you really want to throw in an exit() after the call to header() to actually terminate the execution of the script right there, and you also want to add mysql_real_escape_string to escape the GET-arguments you're passing in to MySQL. You do not want to use user supplied data like that unescaped or unfiltered.

MatsLindh
  • 49,529
  • 4
  • 53
  • 84
  • All true and good points, but `mysql_real_escape_string` would not protect from vulnerability in this specific case. Look closely – Pekka Dec 06 '11 at 23:12
  • It's from a link and the user doesn't see the URL, I redirect between two pages... – Frederick Marcoux Dec 06 '11 at 23:12
  • @Frederick you still need to escape the SQL. It's trivial to see the URL for anybody with a little bit of technical knowledge – Pekka Dec 06 '11 at 23:14