Cleaner - professional code

Hi Guys,

I have self taught myself PHP / MYSQL over the past year, learning just enough and what I need to for each job that comes along.

Can I please get some constructive criticism on the following  code ??

Am I doing things correctly / securely enough, the least time consuming way

// check if the form was submitted

if(isset($_POST['submitted']))
   

{


// Show errors, if any
  

ini_set ('display_errors', 1);  
  

error_reporting (E_ALL & ~E_NOTICE);

include 'db-connect.php';

// filter posted variables to guard against header injection

function cleaner($data)
    {
        if(is_array($data))
        {
            $ret = array();
            foreach($data as $key=>$value)
            {
                $ret[$key] = cleaner($value);
            }
            return $ret;
        }
        else
        {
            if(!is_numeric($data))
            {
                if(get_magic_quotes_gpc())
                {
                    $data = stripslashes($data);
                }
                $data = mysql_real_escape_string($data);
            }
            return $data;
        }
    }    
    

$clean = cleaner($_POST);

// we then need to access each POSTED variable like this:  $clean['name'] 


// declare the variables that werent required (by the form) and or declare integars that didnt need to be cleaned

$security = $_POST['security'];

$job_description = $clean['job_description'];

$vet_reg_num = $clean['vet_reg_num'];

$fax_number = (int) $_POST['fax_number'];

$type = $clean['type'];

// format the checkbox data - seperate with commas
foreach($clean['services'] as &$value) {
    

$value="".$value."";

} 

$services=implode(", ", $clean['services']);



// Initialize error array.

$errors = array();


if(empty($clean['username'])){

$errors[] = 'A username is required and must be alpha-numeric.';

}


// validate the username

$username = $clean['username'];

if (!preg_match('/^[a-z\d_]{4,28}$/i', $username)) {

$errors[] = 'A username is required and must be alpha-numeric.';

}



// Make sure the username is available.

include 'db-connect.php';

$query = "SELECT username FROM Members WHERE username='$username'";

$result= mysql_query($query) or die(mysql_error());

if (mysql_num_rows($result)) {

$errors[] = 'Sorry, that username is already in use.';

}

mysql_close($db);

// Check for a password and match against the confirmed password.

if (eregi ("^[[:alnum:]]{4,20}$", stripslashes(trim($clean['password'])))) {

if ($clean['password'] == $clean['password_confirm']) {

$password = $clean['password'];

} else {

$errors[] = 'Your password did not match the confirmed password!';

}

} else {

$errors[] = 'Your password must be alpha-numeric.';

}


if(empty($clean['password_confirm'])){

$errors[] = 'Please confirm your password.';

} else{ 

$password_confirm = $clean['password_confirm'];

}


if(empty($clean['title'])){

$errors[] = 'Please enter your title.';

} else{ 

$title = $clean['title'];

}

if(empty($clean['first_name'])){

$errors[] = 'Please enter your first name.';

} else{ 

$first_name = $clean['first_name'];

}


if(empty($clean['last_name'])){

$errors[] = 'Please enter your last name.';

} else{ 

$last_name = $clean['last_name'];

}


if(empty($clean['occupation'])){

$errors[] = 'Please enter your occupation.';

} else{ 

$occupation = $clean['occupation'];

}

if(empty($clean['prac_name'])){

$errors[] = 'Please enter your practice name.';

} else{ 

$prac_name = $clean['prac_name'];

}

if(empty($clean['prac_type'])){

$errors[] = 'Please enter your practice type.';

} else{ 

$prac_type = $clean['prac_type'];

}

// Check for an email address & make sure there are no errors.

$email_address = $clean['email_address'];

if (empty($email_address))

{

if (!eregi("^.+@.+\\..+$", $email_address))

{

$errors[] = 'Your email address contains errors.';

}

} 


// Make sure the email address is available.

include 'db-connect.php';

$query = "SELECT email FROM Members WHERE email='$email_address'";

$result= mysql_query($query) or die(mysql_error());

if (mysql_num_rows($result)) {

$errors[] = 'Sorry, that email address is already in use.';

}

mysql_close($db);



if(empty($clean['phone_number']) && ($clean['mobile_number'])){

$errors[] = 'Please enter a phone number';

} else{ 

$phone_number = $clean['phone_number'];

$mobile_number = $clean['mobile_number'];

}


if(empty($clean['street'])){

$errors[] = 'Please enter a street name.';

} else{ 

$street = $clean['street'];

}

if(empty($clean['city'])){

$errors[] = 'Please enter a city.';

} else{ 

$city = $clean['city'];

}

if(empty($clean['state'])){

$errors[] = 'Please select a state.';

} else{ 

$state = $clean['state'];

}

if(empty($clean['postcode'])){

$errors[] = 'Please enter your postcode.';

} else{ 

$postcode = $clean['postcode'];

}

if(empty($clean['country'])){

$errors[] = 'Please enter your country.';

} else{ 

$country = $clean['country'];

}


// Check the security field has been answered correctly

if(($security) !== "2"){

$errors[] = "You answered the security question wrongly. 1 + 1 = 2)";

}

// If everything went okay and there were no errors, continue.

if (empty($errors)) { 

// format the date & time

$now = time();

$thisYear = date("Y-m-d H:i:s", $now);

// Create the activation code

$a = md5(uniqid(rand(), true));



require 'db-connect.php';


   $sql = "INSERT into Members (

username,

pass,

title,

first_name,

last_name,

occupation,

job_description,

vet_reg_num,

email,

prac_name,

street,

city,

state,

postcode,

country,

phone,

mobile,

fax,

type,

services,

activation_code,

date_reg) 
   VALUES (

'$username',

SHA('$password'), 

'$title',

'$first_name',

'$last_name',

'$occupation',

'$job_description',

'$vet_reg_num',

'$email_address',

'$prac_name',

'$street',

'$city',

'$state',

'$postcode',

'$country',

'$phone_number',

'$mobile_number',

'$fax_number',

'$prac_type',

'$services',

'$a',

'$thisYear') ";

       mysql_query($sql) or die(mysql_error());

echo 'data added!';

mysql_close($db);

exit();


// if errors array contains a value

} else {

echo '<table align="center"><tr><td><b>The following error(s) occurred:</b><br /><blockquote>';

foreach ($errors as $msg) {

echo "* $msg<br />\n";

}

echo '</font></blockquote></td></tr></table><br /><br />';

}

// if the form wasn't submitted - display the reg form

}

?>

[Join us @ facebook]

Your code looks ok, but maybe it is too much. I mean do you really need to use a recursive function to create an SQL injection safe version of your whole data, instead of cleaning the data that you are using to your queries?

An optimization you can use, is limiting the mysql results where possible. For instance this:

$query = "SELECT username FROM Members WHERE username='$username'";?>

could be :

$query = "SELECT username FROM Members WHERE username='$username' LIMIT 1";?>

[Guest]

[62.886]

Googlebot, deccanbazaar