patternphpMinor
Basic contact form
Viewed 0 times
contactformbasic
Problem
This is for commercial purposes. It's not going to a database, but will just be emailed to a person via phpMailer.
I am not good at PHP (truly awful). I'd like to know whether this is good enough for going production or not.
Here's the page that does the actual mailing:
``
$mail->Port = 587;
$mail->addReplyTo( $email, $first_name );
$mail->addAddress( $email, $first_name );
$mail->addAddress( 'my_email_address', 'Staff' );
$mail->From = 'my_email_address';
$mail->FromName = 'Staff';
$mail->isHTML(true); // Set email format to HTML
$mail->Subject = 'Contact';
$mail->Body = "First Name: $first_name"
."Last Name: $last_nam
I am not good at PHP (truly awful). I'd like to know whether this is good enough for going production or not.
Toggle navigation
New Daimaru Hotel345 E 1st Street Los Angeles, CA 90012
About
Rooms & information
Book to Reserve
Location Info
Additional Info
Contact
English
Japanese
">
" />
First Name
">
* ' . $firstNameErr . ''); ?>
Last Name
">
* ' . $lastNameErr . ''); ?>
Email
">
* ' . $emailErr . ''); ?>
Message
">
* ' . $messageErr . ''); ?>
Submit
Here's the page that does the actual mailing:
``
SMTPDebug = 3; // Enable verbose debug output
$mail->isSMTP(); // Set mailer to use SMTP
$mail->Host = 'hosting_info'; // Specify main and backup SMTP servers
$mail->SMTPAuth = true; // Enable SMTP authentication
$mail->Username = 'testy@URL.com'; // SMTP username
$mail->Password = 'ugh'; // SMTP password
$mail->SMTPSecure = 'tls'; // Enable TLS encryption, ssl` also accepted$mail->Port = 587;
$mail->addReplyTo( $email, $first_name );
$mail->addAddress( $email, $first_name );
$mail->addAddress( 'my_email_address', 'Staff' );
$mail->From = 'my_email_address';
$mail->FromName = 'Staff';
$mail->isHTML(true); // Set email format to HTML
$mail->Subject = 'Contact';
$mail->Body = "First Name: $first_name"
."Last Name: $last_nam
Solution
Input Validation
The
In short: it isn't recommended defense against anything (the recommendation for XSS are to HTML encode when outputting data, not when receiving it, and the function doesn't provide defense against anything else), and it makes your data dirty, leading to usability problems.
Misc
The
test_input function seems to be from here, and it's really not a good function to use (see here and here). In short: it isn't recommended defense against anything (the recommendation for XSS are to HTML encode when outputting data, not when receiving it, and the function doesn't provide defense against anything else), and it makes your data dirty, leading to usability problems.
Misc
- Your indentation is off, and your bracket positioning is inconsistent, making your code harder to read.
- Using a new CSRF token for each request is good for security, but not that good for usability (breaks back button, etc). Evaluate if this is really necessary, and if not, use a token per session.
- Having file names such as
contact9SessionsCRSF2orcontactResult2is a sign that you have files in your code base which are not actually used, but kept "just in case". This can easily lead to confusion, think about deleting them and removing the2from the file names, and/or use version control.
- The token generation and setting is duplicated code, which is not a good idea as it makes it hard to change and clutters up your code. You could extract it into a
generateAndSetTokenfunction.
Context
StackExchange Code Review Q#115296, answer score: 2
Revisions (0)
No revisions yet.