Posted by kenrbnsn on 06/20/05 05:43

There are a number of errors in the code you posted:


PHP code:
--------------------
<?

$msg = stripslashes($_POST[message]);

$recipient = "jwdoe@nomail.com";
$subject = "$_POST[subject]";

$mailheaders = "From: $_POST[name] <$_POST[email]> \n";
$mailheaders .= "Reply-To: $_POST[email]";

if (!isset($name) || !isset($email) || !isset($subject) ||
!isset($message)) {
header( "Location: contact.php" );
}
elseif (empty($name) || empty($email) || empty($subject) ||
empty($message)) {
header( "Location: contact.php?msg=1" );
}
else {
mail($recipient, $subject, $msg, $mailheaders);
header( "Location: contact.php" );

if (isset($cc)) {
mail( "$email", "$subject (CC: from J. W. Doe's Site)", $message,
"From: jwdoe@nomail.com" );
}
header( "Location: contact.php?msg=2" );
}
?>
--------------------


First, it seems to depend on register_globals being set to "ON". In the
current versions of PHP it is set to "OFF". For more information, see
http://www.php.net/register_globals

Second, in one mail statement you use $msg (which should contain the
message with the quotes unescaped). In the other mail statement you use
$message, which may or may not contain anything.

Here's how I would rewrite the code:

PHP code:
--------------------

<?
if (!isset($_POST['name']) || !isset($_POST['email']) || !isset($_POST['subject']) || !isset($_POST['message'])) // Error conditions should be checked as soon as you start processing
header( "Location: contact.php" );

if (trim($_POST['name']) == '' || trim($_POST['email']) == '' || trim($POST['subject']) == '' || trim($_POST['message']) == '')
header( "Location: contact.php?msg=1" );


$msg = stripslashes($_POST['message']); //you should put the index values in quotes.

$recipient = 'jwdoe@nomail.com'; // use single quotes, unless the string you're quoting needs to be expanded or contains a single quote

$mailheaders = 'From: ' . $_POST['name'] . ' <' . $_POST['email'] . "> \n";
$mailheaders .= 'Reply-To: ' . $_POST['email'];

$result = mail($recipient, $subject, $msg, $mailheaders, '-f ' . $_POST['email']); // a number of ISP are rejecting email if it doesn't look like it really came from the FROM address. The fifth parameter to the mail funciton might help in these cases.
// header( "Location: contact.php" ); // why is this here? With it uncommented, you will never get to the rest of the code

if (!$result) // do something on bad return from mail function


if (isset($_POST['cc']))
$cc_result = mail( $_POST['email'], $_POST['subject'] . " (CC: from J. W. Doe's Site)", $msg,
'From: jwdoe@nomail.com','-f jwdoe@nomail.com');

if (!$cc_result) // do something on bad return

header( "Location: contact.php?msg=2" );

?>

--------------------


Another note:
You don't need curly braces " { } " if you have one statement after a
condition statement or loop control.

Please note, I haven't checked this code for syntax or logic errors.
YMMV

Ken


--
kenrbnsn
------------------------------------------------------------------------
kenrbnsn's Profile: http://techiegroups.com/member.php?userid=3
View this thread: http://www.techiegroups.com/showthread.php?t=47781

[Back to original message]