adammc - 11:34 pm on Jul 27, 2008 (gmt 0)

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 ?

[php]
// 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
}

[/php]