// in the query we convert it to an integer to prevent any injection
if(mysql_query("delete from rota where personid='".(int)$_GET['personid']."'")){
$output = '<b>One member has been removed from the cleaning rota.</b><br/><br/>';
}else{
$output = '<b>An Error Occurred: ' . mysql_error() . '</b><br><br>';
}
// show the list
$output .= ShowList();

return $output;
}

function ManageItem(){
if(isset($_GET['personid'])){
// if we're editing we need to grab the stuff from the database

// convert to integer (if its not a number it'll become zero
$personid= (int)$_GET['personid'];

Well you still aren't passing in the $userid variable as a parameter to your SaveItem function. You need to change the function declaration like so:

PHP Code:

function SaveItem($userid) {// ...

Then when you call the function, supply the user id:

PHP Code:

$content = SaveItem($userid);

A couple of other things, you need to escape all user input. Currently I could manually set a cookie value which would inject SQL into your queries. Data from cookies should be treated with the same suspicion as any other user-supplied data. You already have a function for cleaning data so you just need to call it.

Also you will probably find it easier to write cleaner code if you indent statements. If nothing else it makes it easier to read.

08-04-2009, 07:09 PM

djstrictlylegit

Thank's for all of your feedback Mindzai, it's really useful. I think I'm sorted now, just in the process of implementing all of the changes that you suggested! :)