tags:

views:

223

answers:

4
$thread_id = (isset($_GET['id']));
$thread_id = intval($_GET['id']);

$q = mysql_query("SELECT * FROM threads WHERE id = $thread_id");

Can't you use two functions on one variable?? As you can see I check if its set but it still gives me:

Notice: Undefined index: id in C:\wamp\www\bfhq\thread.php on line 7

+3  A: 

isset() returns a boolean value: TRUE / FALSE. If you want to determine whether $_GET["id"] is set, you need to do so in an IF-Statement, or via a ternary operator.

if (isset($_GET["id"])) {
  $thread_id = $_GET["id"]; // Bad idea taking values in from $_GET directly
} else {
  die("Thread id not set");
}
Jonathan Sampson
Calling intval() on a $_GET value is perfectly reasonable if you expect the variable to be an ID.
Paolo Bergantino
You're right, Paolo. Sorry for the hasty statement on my behalf - I'm just very sensitive when I see $_GET and a query that close ;)
Jonathan Sampson
Only keep in mind that a php integer may have a different min/max values than mysql's numeric values.
VolkerK
+3  A: 

Try this:

$thread_id = isset($_GET['id']) ? intval($_GET['id']) : 0;

Which reads as "if $_GET['id'] is set, set $thread_id to intval($_GET['id']), if not, set to 0"

Paolo Bergantino
A: 

This error is dependent on error/warning level set in php.ini. Its good practice to use...

array_key_exists('id', $_GET);

http://us2.php.net/manual/en/function.array-key-exists.php

also: make sure you sanitize that input from the $_GET before you do that. You're leaving yourself open to mysql injection.

nategood
can mysql injections bypass intval()? doesnt intval allow only numbers.cheers
ah, yes i suppose so. had to make sure intval didn't return false on an error, leaving you with "WHERE = ". however, the fact that it returns 0 on error could be a problem if you do have records with an id of 0.
nategood
A: 

You can get rid of all notices by simply using error_reporting(4);. Another, maybe crude but working, method to suppress the notice and sanitize the variable could be:

$thread_id = (int)@$_GET["id"];

If you do a lot of these things, you maybe want to take a look at filter_var() and put all the functionality into a separate function (eg. get_int("id")).

merkuro