My company is designing a new site, and because I'm currently on bench I've been tasked with writing any PHP required. Pretty simple stuff, just a contact form and a resume upload page, both using a form to email the details to management. No database, no SQL, no business logic. The designer had already built the bulk of the form, so most of the bootstrap was existing when I started.
After some research, I decided on using PHPMailer and Google Recaptcha. The code works as expected, however, as a junior C#/.net dev, PHP is somewhat new to me. I've trawled through many similar code-review posts, and made improvements where I could, but still have some questions (as would I appreciate an overall review!):
- Is it standard to have simple scripts such as this in the same file as the HTML? I've seen it both ways.
- I've read that using globals is usually not best practice. However, I'm unsure how to set the errors from within the validate_form function otherwise. Is there a better way?
- I stole the construct_error_html function from the old site, but I haven't seen similar examples of it. Is this an adequate way to build error messages? Or should I just print the error message within the HTML like I did with the recaptcha error? (
<?="<p class='text-danger'>$recaptcha_error</p>";?>
) These messages should rarely show, as I'm using HTML5 validation as well. - My code (especially the HTML/PHP mix) seems really messy. I tried to standardize formatting, but I'm still not completely happy. Any suggestions for making it look cleaner?
- Security - I've followed along with several security-minded posts, any glaring mistakes?
- I looked into AJAX, but it seems to be overkill for such a small form as this. Opinions?
I currently have all of the code in a single file, with the header and footer added through include statements.
contact.php
<?php
require 'assets/library/PHPMailer-master/PHPMailerAutoload.php';
require 'assets/library/recaptcha-master/src/autoload.php';
$overview_error = '';
$recaptcha_error = '';
$email_error = '';
$name_error = '';
$subject_error = '';
$message_error = '';
$email_sent = false;
$siteKey = 'SITE KEY'; // https://github.com/google/recaptcha/blob/master/examples/example-captcha.php
if (isset($_POST["submit"]))
{
if (!validate_form())
{
$overview_error = 'There are errors on the form.';
}
if (!isRecaptchaValid())
{
$overview_error = 'There are errors on the form.';
$recaptcha_error = 'Please submit recaptcha!';
}
else
{
$subject = strip_tags($_POST["subject"]);
$name = strip_tags($_POST["name"]);
$email = strip_tags($_POST["email"]);
$message = strip_tags($_POST["message"]);
$mail = new PHPMailer; // https://github.com/PHPMailer/PHPMailer
// Tell PHPMailer to use SMTP
// Commented out for now
//$mail->isSMTP();
// Enable SMTP debugging and HTML-friendly debug output
$mail->SMTPDebug = 2;
$mail->Debugoutput = 'html';
$mail->Host = "host";
$mail->Port = 587; // Likely 25, 465, or 587
$mail->SMTPAuth = true; // not sure what the auth details are for this
$mail->Username = "email username";
$mail->Password = "email password";
$mail->setFrom('email', 'Company Contact Us Form');
$mail->addReplyTo($email);
$mail->addAddress('send to email', 'Company Administration');
$mail->Subject = 'Contact us email from ' . $name . ' concerning ' . $subject;
$mail->Body = $name . " has contacted us from the website leaving the following information: \nSubject: " . $subject . "\nName: " . $name . "\nEmail: " . $email . "\n\n" . $message;
// Send the message, check for errors
if(!$mail->send())
{
echo ("Message could not be sent.");
echo ("Mailer Error: " . $mail->ErrorInfo);
exit;
}
$email_sent = true;
}
}
function validate_form()
{
$error_count = 0;
if (empty($_POST["email"])) {
global $email_error;
$email_error = "Please enter your email.";
$error_count ++;
}
elseif (!filter_var($_POST["email"], FILTER_VALIDATE_EMAIL))
{
global $email_error;
$email_error = "Invalid email.";
$error_count ++;
}
if (empty($_POST["name"])) {
global $name_error;
$name_error = "Please enter your name.";
$error_count ++;
}
if (!isset($_POST["subject"])) {
global $subject;
$subject_error = "Please select a subject.";
$error_count ++;
}
if (empty($_POST["message"])) {
global $message_error;
$message_error = "Please enter your comments.";
$error_count ++;
}
return $error_count == 0 ? true : false;
}
function isRecaptchaValid()
{
$siteSecret = 'SITE SECRET'; // www.google.com/recaptcha/admin
$recaptcha = new \ReCaptcha\ReCaptcha($siteSecret);
$response = $recaptcha->verify($_POST['g-recaptcha-response'], $_SERVER['REMOTE_ADDR']);
if ($response->isSuccess()) {
return true;
} else {
return false;
}
}
function construct_error_html($error)
{
if(isset($error))
{
if(strlen(trim($error)) > 0)
{
$error_open = "<span class='help-inline text-danger'> ";
$error_close = "</span>";
return $error_open.$error.$error_close;
}
}
}
?>
<!DOCTYPE html>
<html lang="en">
<head>
<?php include("assets/includes/head.php"); ?>
</head>
<body>
<?php include("assets/includes/header.php"); ?>
<div id="pageheaderwrap" class="header_con">
<div class="container">
</div>
</div>
<div id="hero">
<div class="container inner">
<div class="row">
<div class="col-md-8 col-sm-9 col-md-offset-2">
<header>
<?php
if ($email_sent)
{ ?>
<h1>Thank You</h1>
<p>Thank you for contacting us, we'll get back to you as soon as possible.</p>
<?php
} else {
?>
<h1>Contact</h1>
<p>Let us put our expertise to work for you.</p>
<?php }?>
</header>
</div>
</div>
</div>
</div>
<div class="container w">
<div class="row">
<div class="col-md-8 col-md-offset-2">
<div class="section">
<div id="content-area">
<p> We'd love to hear from you. You can call us at (phone) or use the contact form below to send us an email.</p>
</div>
</div>
</div>
</div>
</div>
<?php if (!$email_sent) { ?>
<div class="container gap">
<div class="row">
<div class="col-md-8">
<div class="well well-sm">
<form role="form" method="post" action="emailtest.php">
<div class="row">
<?="<p class='text-danger'>$overview_error</p>";?>
</div>
<div class="row">
<div class="col-md-6">
<div class="form-group">
<label for="name">Name</label><?php print construct_error_html($name_error); ?>
<input type="text" class="form-control" id="name" name="name" placeholder="Enter name"
value="<?= isset($_POST['name']) ? htmlspecialchars($_POST['name']) : '' ;?>" />
</div>
<div class="form-group">
<label for="email">Email Address</label><?php print construct_error_html($email_error); ?>
<div class="input-group">
<span class="input-group-addon"><span class="glyphicon glyphicon-envelope"></span>
</span>
<input type="email" class="form-control" id="email" name="email" placeholder="Enter email"
value="<?= isset($_POST['email']) ? htmlspecialchars($_POST['email']) : '' ;?>" /></div>
</div>
<div class="form-group">
<label for="subject">Subject</label><?php print construct_error_html($subject_error); ?>
<select id="subject" name="subject" class="form-control">
<option value="na" selected="">Choose One:</option>
<option value="service"
<?php if(isset($_POST['subject']) && $_POST['subject'] == 'service')
echo ' selected="selected"'; ?>
>General Customer Service</option>
<option value="suggestions"
<?php if(isset($_POST['subject']) && $_POST['subject'] == 'suggestions')
echo ' selected="selected"'; ?>
>Suggestions</option>
<option value="product"
<?php if(isset($_POST['subject']) && $_POST['subject'] == 'product')
echo ' selected="selected"'; ?>
>Product Support</option>
</select>
</div>
<div class="form-group">
<div class="g-recaptcha" data-sitekey="<?= $siteKey; ?>"></div>
<?="<p class='text-danger'>$recaptcha_error</p>";?>
</div>
</div>
<div class="col-md-6">
<div class="form-group">
<label for="message">
Message</label><?php print construct_error_html($message_error); ?>
<textarea name="message" id="message" class="form-control" rows="10" cols="25"
placeholder="Message"><?= isset($_POST['message']) ? htmlspecialchars($_POST['message']) : '' ; ?></textarea>
</div>
</div>
<div class="col-md-12">
<button type="submit" name="submit" class="btn btn-primary pull-right" id="btnContactUs">
Send Message</button>
</div>
</div>
</form>
</div>
</div>
</div>
</div>
<?php } ?>
<?php include("assets/includes/footer.php"); ?>
</body>
</html>