Problem
I have a contact form that will perform validation on the server. Right now the form checks if the fields are empty or not. My goal is to expand it following:
Check the fields for particular format, if it is a proper email format, phone number format, etc
Set the field as required or not
My form is as below, I appreciate any suggestions on how to clean it up and make it easier to adjust and overall cleaner.
<?php
ini_set("log_errors", 1);
ini_set("error_log", "php-error.log");
error_log( "Errors" );
include "recaptcha.php";
include 'email.php';
use PHPMailerPHPMailerPHPMailer;
use PHPMailerPHPMailerException;
require 'Exception.php';
require 'PHPMailer.php';
require 'SMTP.php';
$errors = array(); // array to hold validation errors
$data = array(); // array to pass back data
// validate the variables ======================================================
// if any of these variables don't exist, add an error to our $errors array
if (empty($_POST['firstName']))
$errors['firstName'] = 'First Name is required.';
if (empty($_POST['lastName']))
$errors['lastName'] = 'Last Name is required.';
if (empty($_POST['companyName']))
$errors['companyName'] = 'Company Name is required.';
if (empty($_POST['companyAddress']))
$errors['companyAddress'] = 'Company Address is required.';
if (empty($_POST['city']))
$errors['city'] = 'City is required.';
if (empty($_POST['state']))
$errors['state'] = 'State is required.';
if (empty($_POST['emailAddress']))
$errors['emailAddress'] = 'Email Address is required.';
if (empty($_POST['comment']))
$errors['comment'] = 'Comment is required.';
if (empty($_POST['g-recaptcha-response']))
$errors['recaptcha'] = 'Captcha is required.';
// return a response ===========================================================
// Check ReCaptcha Validation
if(!validateRecaptcha($secret, $_POST['g-recaptcha-response'], $_SERVER["REMOTE_ADDR"]))
{
$errors['recaptcha'] = 'Captcha is required.';
}
if ( ! empty($errors)) {
// if there are items in our errors array, return those errors
$data['success'] = false;
$data['errors'] = $errors;
} else {
// if there are no errors process our form, then return a message
// DO ALL YOUR FORM PROCESSING HERE
// THIS CAN BE WHATEVER YOU WANT TO DO (LOGIN, SAVE, UPDATE, WHATEVER)
// show a message of success and provide a true success variable
$data['success'] = true;
$data['message'] = 'Success!';
}
// return all our data to an AJAX call
echo json_encode($data);
//Begin of send email
$mail = new PHPMailer;
//Enable SMTP debugging.
$mail->SMTPDebug = 3;
//Set PHPMailer to use SMTP.
$mail->isSMTP();
//Set SMTP host name
$mail->Host = "smtp.server.com";
//Set this to true if SMTP host requires authentication to send email
$mail->SMTPAuth = true;
//Provide username and password
$mail->Username = "user@server.com";
$mail->Password = "super_secret_password";
//If SMTP requires TLS encryption then set it
$mail->SMTPSecure = "tls";
//Set TCP port to connect to
$mail->Port = 587;
$mail->From = $emailAddress;
$mail->addAddress("name@server.com", "Recepient Name");
$mail->isHTML(true);
$mail->Subject = "Subject Text";
$mail->Body = $emailBody;
if(!$mail->send())
{
$data['success'] = false;
$data['errors'] = "Mailer Error: " . $mail->ErrorInfo;
}
else
{
$data['message'] = 'Success!';
}
?>
Solution
General inconsistency
Basically your code is borderline off topic. Although it could work, it is not a working code in the sense a programmer could take it; but rather a set of copy-pasted code blocks loosely (and wrongly) connected to each other.
The main problem is inconsistent code structure. You have $data['success']
set at the end of the script but… it goes nowhere.
So it basically should be
if ($errors) {
$data['success'] = false;
$data['errors'] = $errors;
} else {
$mail = new PHPMailer;
// the rest or phpmailer code goes on
if(!$mail->send())
{
$data['success'] = false;
$data['errors'] = ["Mailer Error: " . $mail->ErrorInfo];
}
else
{
$data['success'] = true;
$data['message'] = 'Success!';
}
}
// return all our data to an AJAX call
echo json_encode($data);
Note that another error has been fixed. $data[‘errors’] should be an array, but in case of phpmailer error you were sending a string.
Configuration options
error_log( "Errors" );
Always use the absolute filesystem path to a log file or you’ll end up with log files in the every directory.
Also, it’s better to configure such settings on the server level, for some errors may occur before the file gets parsed completely. Or at least put these settings into a configuration file and include it in all your php scripts.
Autoload
Instead of requiring files manually, consider using PSR-4 autoload and Composer. It’s really easy, you just have to install Composer, then run two commands in console
php composer.phar require google/recaptcha
php composer.phar require phpmailer/phpmailer
and then add just a single require line into your code
require __DIR__ . '/vendor/autoload.php';
and all classes will be loaded automatically!
Validation and sanitization
You may want to verify whether the email address is valid.
if (empty($_POST['emailAddress'])) {
$errors['emailAddress'] = 'Email Address is required.';
} else {
$emailAddress = filter_var($errors['emailAddress'], FILTER_SANITIZE_EMAIL);
if (!filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) {
$errors['emailAddress'] = 'Email Address is invalid.';
}
}
and also sanitize the email body,
$emailBody = htmlspecialchars($_POST['comment']);