3
\$\begingroup\$

Is the following PHP laid out fine to go inside the mail function?

$to = "My Name <[email protected]>";
$subject = "Contact Form: $name";
$message = "Name: $name\r\nEmail: $email\r\nMessage:\r\n$message";
$headers = "From: Contact Form <contactfo[email protected]>";

mail($to, $subject, $message, $headers);

Should there be any spaces around the \r\n? Any important headers to include?

Any tips on how to improve it are welcome.

If you're interested, please the full script below. It is adapted from here — it's allegedly secure... is it?

<?php

// Clean up the input values
foreach($_POST as $key => $value) {
  if(ini_get('magic_quotes_gpc'))
    $_POST[$key] = stripslashes($_POST[$key]);

  $_POST[$key] = htmlspecialchars(strip_tags($_POST[$key]));
}

// Assign the input values to variables for easy reference
$name = $_POST["name"];
$email = $_POST["email"];
$message = $_POST["message"];

// Test input values for errors
$errors = array();
if(strlen($name) < 2) {
  if(!$name) {
    $errors[] = "You must enter a name.";
  } else {
    $errors[] = "Name must be at least 2 characters.";
  }
}
if(!$email) {
  $errors[] = "You must enter an email.";
} else if(!validEmail($email)) {
  $errors[] = "You must enter a valid email.";
}
if(strlen($message) < 10) {
  if(!$message) {
    $errors[] = "You must enter a message.";
  } else {
    $errors[] = "Message must be at least 10 characters.";
  }
}

if($errors) {
  // Output errors and die with a failure message
  $errortext = "";
  foreach($errors as $error) {
    $errortext .= "<li>".$error."</li>";
  }
  die("<span class='failure'>The following errors occured:<ul>". $errortext ."</ul></span>");
}

// Send the email
$to = "YOUR_EMAIL";
$subject = "Contact Form: $name";
$message = "$message";
$headers = "From: $email";

mail($to, $subject, $message, $headers);

// Die with a success message
die("<span class='success'>Success! Your message has been sent.</span>");

// A function that checks to see if
// an email is valid
function validEmail($email)
{
   $isValid = true;
   $atIndex = strrpos($email, "@");
   if (is_bool($atIndex) && !$atIndex)
   {
      $isValid = false;
   }
   else
   {
      $domain = substr($email, $atIndex+1);
      $local = substr($email, 0, $atIndex);
      $localLen = strlen($local);
      $domainLen = strlen($domain);
      if ($localLen < 1 || $localLen > 64)
      {
         // local part length exceeded
         $isValid = false;
      }
      else if ($domainLen < 1 || $domainLen > 255)
      {
         // domain part length exceeded
         $isValid = false;
      }
      else if ($local[0] == '.' || $local[$localLen-1] == '.')
      {
         // local part starts or ends with '.'
         $isValid = false;
      }
      else if (preg_match('/\\.\\./', $local))
      {
         // local part has two consecutive dots
         $isValid = false;
      }
      else if (!preg_match('/^[A-Za-z0-9\\-\\.]+$/', $domain))
      {
         // character not valid in domain part
         $isValid = false;
      }
      else if (preg_match('/\\.\\./', $domain))
      {
         // domain part has two consecutive dots
         $isValid = false;
      }
      else if(!preg_match('/^(\\\\.|[A-Za-z0-9!#%&`_=\\/$\'*+?^{}|~.-])+$/',
                 str_replace("\\\\","",$local)))
      {
         // character not valid in local part unless
         // local part is quoted
         if (!preg_match('/^"(\\\\"|[^"])+"$/',
             str_replace("\\\\","",$local)))
         {
            $isValid = false;
         }
      }
      if ($isValid && !(checkdnsrr($domain,"MX") || checkdnsrr($domain,"A")))
      {
         // domain not found in DNS
         $isValid = false;
      }
   }
   return $isValid;
}

?>

I ended up using the ValidForm Builder because it has better security, great customizability, and an easy implementation — quoting from their site:

  • The API generates XHTML Strict 1.0 compliant code.
  • Field validation on the client side to minimize traffic overhead.
  • Field validation on the server side to enforce validation rules and prevent tempering with the form through SQL injection.
  • Client side validation displays inline to improve user satisfaction. No more annoying popups that don't really tell you anything.
  • Easy creation of complex form structures.
  • Uses the popular jQuery Javascript library for DOM manipulation.
  • Completely customizable using CSS.
  • Automatic creation of field summaries for form mailers in both HTML and plain text.

Quoting other benefits:

  • First of all, it's open source and therefore completely free!
  • Super fast web form creation.
  • Get rid of SQL injection problems.
  • Create standards based CSS forms. No tables inside.
  • Make form entry fun for the user. More feedback from your website.
\$\endgroup\$
2
  • \$\begingroup\$ What do you mean by secure? The link you posted also does not define secure. \$\endgroup\$ Dec 7, 2012 at 1:09
  • \$\begingroup\$ Well, for PHP: php.net/manual/en/security.php \$\endgroup\$
    – Baumr
    Dec 7, 2012 at 12:11

2 Answers 2

4
\$\begingroup\$

As far as I think, your script is good and will achieve the required task. But before finalising on this script, you should read about PHPMAILER. It is easy to implement and will provide you many features like to add CC, BCC, and attachments. The mail() function is the easiest way, but as far as I think, it is a bit limited. Also, PHPMailer is easily available on the Internet.

\$\endgroup\$
2
\$\begingroup\$

Before writing your own mailing class, please please please try to not reinvent the wheel and save yourself some time. There are a lot of fundamental problems with this mailing script, and could be resolved by using something premade from one of the major PHP frameworks;

Either one of those is a testable, and extendable solution to your problem and will be much easier to implement.

\$\endgroup\$
2
  • \$\begingroup\$ I didn't write this mailing class — please see the link. But what problems are there with it? \$\endgroup\$
    – Baumr
    Dec 9, 2012 at 23:22
  • \$\begingroup\$ 454/5000 This is the most depressing answer I have read here, someone should have given this answer to the mind behind Dropbox, or the creator of Zoom, don't go reinventing the wheel, there are many premade solutions out there ... Man, you are a dream killer ! Since we are in it Mr sudobangbang, why learn CSS ?, just use pre-built templates, you don't want to invent your own CMS, use WordPress, you don't want to do programming, just sell tacos. \$\endgroup\$ May 28, 2020 at 14:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.