0

I got this query that is not working, and I don't know what it is. I'm writing a email active scripts. This the URL;

http://www.example.com/activate.php?id=1&h=secretkey1

This is what activate.php looks like;

<?php
require "inc/connect.php";

$id = (int)$_GET['id'];
$hash = mysql_real_escape_string($_GET['h']);

$sql = mysql_query("SELECT id, h, active FROM tablename WHERE id=$id AND key=$hash") or die(mysql_error());  
$row = mysql_fetch_array($sql);

if ($row['active'] == "1") {
    echo "error!";
}
elseif ($row['active'] == "0") {
    mysql_query("UPDATE tablename SET active=1 WHERE id='$id' AND key='$hash'");
}
?>

This is the error that I get;

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 'key=secretkey1' at line 1

In my database I have a test field in tablename with;

id /    key    / active
------------------
1 / secretkey1 / 0
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Linkjuice57
  • 3,473
  • 6
  • 24
  • 23
  • Is the different use of quotation marks in the two queries intentional? WHERE id=$id AND key=$hash vs WHERE id='$id' AND key='$hash' – Josien Jul 27 '12 at 13:26

2 Answers2

3

Two reasons:

1) Because you need to enclose secretkey1 (inside $hash) in single quotes, since it is a string and strings must be single-quoted. $id doesn't need to be quoted since you have cast it to an integer.

2) KEY is a MySQL reserved keyword, and must be enclosed in backquotes to be used as a table or column name:

$sql = mysql_query("SELECT id, h, active FROM tablename WHERE id=$id AND `key`='$hash'") or die(mysql_error());

I note also that you are selecting id, h, active in your query but your example table has a column named key, not h (unless we don't see that column).

Michael Berkowski
  • 267,341
  • 46
  • 444
  • 390
  • Thanks for the fast reply. I did have single quotes before, but tried removing them to see if it helped. Also it should be `key` in the query but even when using `*` I still get an error. With both single quotes and `select *` I now get this: `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 'key='secretkey1'' at line 1` – Linkjuice57 Jul 27 '12 at 13:32
  • 1
    @Linkjuice57 See change above. KEY is a reserved keyword. – Michael Berkowski Jul 27 '12 at 13:34
  • Aha! Never would have figured that about by myself :) – Linkjuice57 Jul 27 '12 at 13:36
2

You have this:

 key=$hash

That should be:

 key='$hash'

By the way, as much as possible, use parameterized query to prevent sql injection attack.

Use PDO:

How can I prevent SQL injection in PHP?

Are PDO prepared statements sufficient to prevent SQL injection?

Community
  • 1
  • 1
Michael Buen
  • 38,643
  • 9
  • 94
  • 118
  • I thought that `mysql_real_escape_string` was good enough, thanks for the suggestion I'll check it out! – Linkjuice57 Jul 27 '12 at 13:37
  • there are times it isn't enough http://johnroach.info/2011/02/17/why-mysql_real_escape_string-isnt-enough-to-stop-sql-injection-attacks/ – Michael Buen Jul 27 '12 at 14:42