Can someone check over my PHP Mail Script please?

Hi,

I’ve put together this simple mail script below and being a PHP newbie I just wondered if someone could check it over and let me know if there’s anything I should change or add (including code layout & structure).

Also i’ve read a little about:

1/ ‘X-Mailer: PHP/’ . phpversion();
and
2/ “-f”.$_REQUEST[email]); - are these things I should add?

3/ A while ago someone told me to add in this snippet of code to help increase security and prevent spam; Is this snippet still relevent, does it work, and if not what should I have instead to stop people injecting my form and using it to spam?

(on testing, adding “/n” and “/r” into the form fields this script doesnt seem to do anything at all to stop or prevent this)

// Security check
if (!get_magic_quotes_gpc()) {
	foreach($_POST as $key => $value) {
		str_replace(array("\
","\\r",":"), "", $_POST[$key]);
		addslashes($_POST[$key]);
	}
}

4/ Oh, and hat does this piece do (Check the location/url of my form?), and do I need it? What is its purpose?

// form location check?
if (!isset($_POST[‘email’])) {
header( “Location: $formurl” );
exit ;

}

Here is my complete script:


<?php

// Form and Destination urls
$formurl = 'http://mydomain.com/index.html' ;
$errorurl = 'http://mydomain.com/error.html' ;
$thankyouurl = 'http://mydomain.com/thankyou.html' ;

// Field Declarations
$name = $_POST['name'] ;
$email = $_POST['email'] ;
$comments = $_POST['comments'] ;

$http_referrer = getenv( "HTTP_REFERER" );


// Security checks and validation

	// Empty Fields check
	if (empty($name) || empty($email) || empty($comments)) {
	   header( "Location: $errorurl" );
	   exit ;
	}
	// Valid Email check
	if (strlen($email) > 0) {
		$clean_email = filter_var($email, FILTER_VALIDATE_EMAIL);
		if ($clean_email === FALSE) {
			header( "Location: $errorurl" );
			exit ;
		}
	}



// To (multiple recipients)
$to  = 'dude@mydomain.com' . ', '; // note the comma
$to .= 'dude2@mydomain.com';

// Subject
$subject = '** New Message';

// Message
$message =
	" \
" .
	"WEBSITE GENERATED EMAIL \
" .
	"------------------------ \
\
" .

	"	Name: $name \
" .
	"	Email: $email \
\
" .
	"	Comments: $comments \
\
\
\
\
\
" .

	"--- \
";
	
	

// To send HTML mail, the Content-type header must be set
$headers  = 'MIME-Version: 1.0' . "\\r\
";
$headers .= 'Content-type: text/plain; charset=utf-8' . "\\r\
"; // Change 'plain' to 'html' for HTML emails

// Additional headers
$headers .= 'To: Dude1 <dude1@mydomain.com>, Dude2 <dude2@mydomain.com>' . "\\r\
";
$headers .= 'From: Website <website@mydomain.com>' . "\\r\
";
$headers .= 'Cc: Girl1@mydomain.com' . "\\r\
";
$headers .= 'Bcc: Girl2@anotherdomain.com' . "\\r\
";
$headers .= 'Reply-To: info@mydomain.com' . "\\r\
";

// Refer to thankyou url on sucessful submission
header( "Location: $thankyouurl" );

// Mail it
mail($to, $subject, $message, $headers);

?>

6/ Finally - what is the difference between defining the recipients with $to midway through the script in that way, and again at the bottom of the script in the headers? It seems a bit pointless defining them twice (albeit in two completely different ways) - is that needed? and if so why?

Thank you so much for any help and advice!

Q 4. If you added these lines, but you do not understand what they do, then why did you add them?


// form location check? <- No

// if the email element of the form was filled in
if (!isset($_POST['email'])) { 

// send the browser to a new location, that defined in $formurl
header( "Location: $formurl" );

// stop processing the rest of this script
exit ;

}

In effect it is a redirect.

Hi Cups, thanks for your reply.

As I said, I was told to add the code to make the form more secure. This was a whlie ago so I cant remember exactly what was said at the time.

A redirect does seem pretty pointless though, so are you recommending I lose that snippet of code you quoted?

…and any word or advice/help on the remaining? :slight_smile: