The last time I wrote any serious code was the 90’s, so a lot has changed and I am finding I am having to relearn pretty much everything.
I have leaned a lot from a previous post and this is the result. So as the title states I need a code review to help me identify where I am making mistakes by today’s standards. To thwart any PDO comments, I have decided against it for now.
One note, the ValidateForms() function is just one big catch all at the moment, later I will break it down and put the parts and pieces into their corresponding functions. And yes, there are errors in that function as I am fixing them write the corresponding page/function. But the registration page and the corresponding functions do successfully complete their tasks.
It’s a lot, I know! ;D
The User Registration Page:
[php]
include(‘pg_top.php’);
if(isset($_POST[‘submitbtn’])){
$errors = ValidateForms();
if(empty($errors)){
$errors = UserRegistration();
}
}
if($errors[0] == “You must activate your account before logging in. Please check your email.”){
$success = $errors[0];
echo $success;
}else {
?>
User Registration
<?php
if(!empty($errors)){
foreach($errors as $error){
echo '
<?php
}
include('pg_bot.php');
[/php]
Validating user input from registration for:
[php]function ValidateForms() {
array_walk_recursive($_POST, 'trim');
array_walk_recursive($_GET, 'trim');
$errormsg = array();
foreach($_POST as $key => $value){
if($key == 'auction_desc'){ continue; }
if($key == 'imageUploads'){ continue; }
if($value != strip_tags($value)) {
$errormsg[] = 'HTML Tags are not allowed';
}
}
if(in_array($_POST['userName'] || $_POST['pswd'] || $_POST['activationCode'] || $_POST['licenseNum'] || $_POST['street'],$_POST)){
$toCheck = array($_POST['userName'],$_POST['pswd'],$_POST['activationCode'],$_POST['licenseNum'], $_POST['street']);
foreach($toCheck as $key => $var) {
if(empty($var)){ continue; }
if(!ctype_alnum($var)){
$errormsg[] = $var.' is not alphnumeric';
}
}
}
if(in_array($_POST['firstName']||$_POST['ampm']||$_POST['role'],$_POST)){
$toCheck = array($_POST['firstName'],$_POST['ampm'],$_POST['role']);
foreach($toCheck as $key => $var) {
if(empty($var)){ continue; }
if(!ctype_alpha($var)){
$errormsg[] = $var.' must only contain letters.';
}
}
}
if(in_array($_POST['zip']||$_POST['month']||$_POST['day']||$_POST['year']||$_POST['hour']||$_POST['min']||$_POST['sort'],$_POST)){
$toCheck = array($_POST['zip'],$_POST['month'],$_POST['day'],$_POST['year'],$_POST['hour'],$_POST['min'],$_POST['sort']);
foreach($toCheck as $key => $var) {
if(empty($var)){ continue; }
if(!ctype_digit($var)){
$errormsg[] = 'You have an error in either zip code or auction date/time.';
}
}
}
if(isset($_POST['city'])){
if (!preg_match("/^[A-Za-z\ \']+$/",$_POST['city'])) {
$errormsg[] = 'City name may only contain letters, single quote';
}
}
if(isset($_POST['state'])){
if (!preg_match("/^[A-Za-z\ \']+$/",$_POST['state'])) {
$errormsg[] = 'City name may only contain letters, single quote';
}
}
if(isset($_POST['lastName'])){
if (!preg_match("/^[A-Za-z\\- \']+$/",$_POST['lastName'])) {
$errormsg[] = 'Last name may only contain letters,a hyphen or a single quote';
}
}
if(isset($_POST['busName'])){
if (!preg_match("/^[0-9A-Za-z\\- \&\.\\']+$/",$_POST['busName'])) {
$errormsg[] = "Business name may only contain letters, numbers and . & ' -";
}
}
if(isset($_POST['title'])){
if (!preg_match("/^[0-9A-Za-z\\- \&\.\\']+$/",$_POST['title'])) {
$errormsg[] = "Business name may only contain letters, numbers and . & ' -";
}
}
if(isset($_POST['bio'])){
if (!preg_match("/^[0-9A-Za-z\\- \&\.\,\(\)\$\?\%\\']+$/",$_POST['bio'])) {
$errormsg[] = "Bio name may only contain letters, numbers and ?$%&()-,.";
}
}
if(isset($_POST['question'])){
if (!preg_match("/^[0-9A-Za-z\\- \&\.\,\(\)\$\?\%\\']+$/",$_POST['question'])) {
$errormsg[] = "Question name may only contain letters, numbers and ?$%&()-,.";
}
}
if(isset($_POST['category_1'])){
if (!preg_match("/^[A-Za-z\/\&\.\']+$/",$_POST['category_1'])) {
$errormsg[] = "Bio name may only contain letters, numbers and ?$%&()-,.";
}
}
if(isset($_POST['category_2'])){
if (!preg_match("/^[A-Za-z\/\&\.\']+$/",$_POST['category_2'])) {
$errormsg[] = "Bio name may only contain letters, numbers and ?$%&()-,.";
}
}
if(isset($_POST['category_3'])){
if (!preg_match("/^[A-Za-z\/\&\.\']+$/",$_POST['category_3'])) {
$errormsg[] = "Bio name may only contain letters, numbers and ?$%&()-,.";
}
}
if(isset($_POST['phone'])){
if(!preg_match($toMatch,$_POST['phone'])){
$errormsg[] = 'Phone number must be (###)###-#### format';
}
}
if(isset($_POST['fax'])){
if(!preg_match($toMatch,$_POST['fax'])){
$errormsg[] = 'Fax number must be (###)###-#### format';
}
}
if (isset($_POST['email']) && !filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
$errormsg[] = "The email address entered is invalid.";
}
//does pswd contain 1 upper, 1 lower and 1 number_format
if(isset($_POST['pswd']) && !preg_match('/(?=.{8,})(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).*/', $_POST['pswd'])){
$errormsg[] = 'Password must contain one uppercase and lowercase letter and one number.';
}
//Is state length longer than 2 chars
if(isset($_POST['state']) && strlen($_POST['state'])!=2){
$errormsg[] = 'State must be abbreviated to two characters';
}
//is zipcode longer than 5 chars
if(isset($_POST['zip']) && strlen($_POST['zip']) !=5){
$errormsg[] = 'Zipcode must be 5 digits.';
}
//month length
if(isset($_POST['month']) && strlen($_POST['month']) !=2){
$errormsg[] = 'Something is wrong with the month field.';
}
//day length
if(isset($_POST['day'])){
if(strlen($_POST['day']) >2 || strlen($_POST['day'])< 1){
$errormsg[] = 'Something is wrong with the day field.';
}
}
//year length
if(isset($_POST['year']) && strlen($_POST['year']) !=4){
$errormsg[] = 'Something is wrong with the year field.';
}
//hour length
if(isset($_POST['hour'])){
if(strlen($_POST['hour']) >2 || strlen($_POST['hour'])< 1){
$errormsg[] = 'Something is wrong with the hour field.';
}
}
if(isset($_POST['min']) && strlen($_POST['min']) !=2){
$errormsg[] = 'Something is wrong with the minute field.';
}
if(isset($_POST['ampm']) && strlen($_POST['ampm']) !=2){
$errormsg[] = 'Something is wrong with the am/pm field.';
}
if (isset($_POST['wsaddr']) && filter_var($_POST['wsaddr'], FILTER_VALIDATE_URL) === false) {
$errormsg[] = 'The websiite address you entered is invalid.';
}
if(isset($_POST['bio']) && strlen($_POST['bio'])>255){
$errormsg[] = 'Bio can not be longer than 255 characters';
}
//is title longer than 75 chars.
if(isset($_POST['title'])){
if(strlen($_POST['title'])>75){
$errormsg[] = 'Title can not be longer than 75 characters';
}
}
if(isset($_POST['auction_desc'])){
if(strlen($_POST['auction_desc'])>5000){
$errormsg[] = 'Auction description is limited to 3000 characters';
}
}
if(isset($_POST['sort'])){
if(strlen($_POST['sort']) >1){
$errormesg[] = 'Something has gone wrong with the sort feature, please try again.';
}
}
if(isset($_POST['role'])){
if(strlen($_POST['role']) > 10 || strlen($_POST['role']) < 5){
$errormsg[] = 'Something has gone wrong please try again.';
}
}
return $errormsg;
}[/php]
Input data comparison and storage:
[php] function UserRegistration(){
$errormsg = array();
if(!isset($_POST['role'])){
$errormsg[] = 'You must chose if you are buyer or an auctioneer.';
}
if(!isset($_POST['email'])){
$errormsg[] = 'You must enter a valid email address.';
}
if(!isset($_POST['pswd'])){
$errormsg[] = 'You must enter a password.';
}
if(!isset($_POST['retypepswd'])){
$errormsg[] = 'You must enter a password.';
}
if($_POST['pswd'] !== $_POST['retypepswd']){
$errormsg[] = 'Your passwords do not match.';
}
if(!empty($errormsg)){
return $errormsg;
}else {
$token = md5(uniqid(mt_rand(), true));
$hash = password_hash($_POST['pswd'], PASSWORD_DEFAULT);
$a = 0;
require('includes/db_connect.php');
$sql = "SELECT username FROM members WHERE username=?";
if(!($stmt = $conn->prepare($sql))){
$errormsg[] = "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
}
if(!$stmt->bind_param("s",$_POST['userName'])){
$errormsg[] = "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
}
if(!$stmt->execute()) {
$errormsg[] = "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
}
$stmt->bind_result($username);
$stmt->fetch();
$stmt->close();
if($_POST['userName'] == $username){
$errormsg[] = "This username taken, please choose another.";
}else {
$sql = "INSERT INTO members (username, email, password, active, code, role, setup_complete) VALUES (?, ?, ?, ?, ?, ?, ?)";
if(!($stmt = $conn->prepare($sql))){
$errormsg[] = "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
}
if(!$stmt->bind_param("sssissi",$_POST['userName'],$_POST['email'],$hash,$a,$token,$_POST['role'],$a)){
$errormsg[] = "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
}
if(!$stmt->execute()) {
$errormsg[] = "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
}
if(empty($errormsg)){
$rows = $stmt->affected_rows;
}
$stmt->close;
$conn->close();
if ($rows == 1) {
SendActivationEmail($to,$token);
}else {
$errormsg[] = 'There was a database error Please contact support';
}
if(@mail($email, $subject, $message, $headers)){
$errormsg[] = "You must activate your account before logging in. Please check your email.";
}else {
$errormsg[] = 'There was an error sending your activation email.';
}
}
return $errormsg;
}
}[/php]
- '.$error.'
';
}
}
?>
I am a buyer!
I am a auctioneer!
Username
Email
Password
Retype Password