PHP Session Issue

[dunlop03]

I've created a simple comment feature for my website. And I'm using sessions to determine who is in the admin group so they can delete comments.

Group ID 1 = Normal User
Group ID 2 = Admin

When I set myself as group 1 and use the delete code below it triggers the "else" which is what I want. But when I set myself as group 2 I still get the "else". I have echoed the $_SESSION['grp'] on the page to make sure the session is set correctly and it is. Anyone know why its not deleting the row correctly ?

include("inc/connect.php");

if(isset($_SESSION['grp']) == 2) {
    $row = mysql_real_escape_string($_GET['row']);
    $result2 = mysql_query("DELETE FROM messages WHERE mid=$row ") or die(mysql_error());
}else{
    header('location: error.php?x=8');
} 

[dunlop03]

Sorted it, I had to encase the group ID in double quotes ? I thought you didn't need to do that. Anyone know why ?

if(isset($_SESSION['grp']) == "2") {
//etc......
}

[MonkOFox]

Hmm, is ID varchar in the table? If not, are you accidently setting it to the string value of 1 or 2? I would also use $_POST instead of $_GET, a little more secure. Also, slap an intval() around that $row in that query.
Could also do: MySQL_query(sprintf("Delete From messages Where mid=%d",intval($row))) or die ("Query error"). Unless you plan on turning off errors and warnings, I wouldn't echo MySQL_error().

The intval() will return zero if an int conversion fails (returns 0) prevents someone from making $row = '2 OR mid > -1' or something along those lines.
Not echoing the error is just to keep table/field names unviewable by users. If you need the error, create log files and store them outside of root.

Justin

**Edit: the f on sprintf always seems to escape me.

[dunlop03]

No the table field is of type int(11). Thank you for your suggestions, i'll take note and implement. As for echoing the mysql error, you mean remove the "or die ("Query error")" from the query ?

[Nanotech]

You shouldn't show the user an error from PHP/SQL, instead, just show them a custom string error making it more user friendly.

Regards,

Ntech

[Nanotech]

Sorry for double post, I can't edit though.

If your ID field is integer value, then you should be checking if $_SESSION is integer 1 and not string '1'.

Can you post your code where you initially set the session value?

Regards,

Ntech.

[MonkOFox]

No the table field is of type int(11). Thank you for your suggestions, i'll take note and implement. As for echoing the mysql error, you mean remove the "or die ("Query error")" from the query ?

yes just do something like:

//...
$result2 = mysql_query("DELETE FROM messages WHERE mid=$row ")
if(!results){
header('location: error.php?x=differenterrorcode');
}
//...

Yeah, posting the code snippet that sets the $_SESSION['grp'] value would help : ).

Justin