Need help with email form

[ccarcia3]

To PHPBuilder Forumgoers,

My PHP email form is not sending to the address I specified, and I think it's because I added in an if-else statement to check for input of a valid email address. I've put the HTML and PHP scripts online and filled the live form out with several valid email addresses, with no success. I tried changing the recipient's email, too, with no success. Am I missing a glaring error in my code, or maybe things are just not in the right order?

Any help would be SUPER appreciated. Thank you so much. 🙂

The code (in external PHP file "order.php") :

<?php
$to = "[e-mail]";
$subject = "Custom Order Message";
$email = $_REQUEST['email'] ;
$message = $_REQUEST['message'] ;
$from = $_REQUEST['name'];
$budget = $_REQUEST['budget'];
$colors[] = $_REQUEST['colors[]'];

$headers = "From: $email";
$sent = mail($to, $from, $subject, $message, $budget, $headers) ;

if( !eregi("^[a-z0-9]+([_\\.-][a-z0-9]+)*". "@([a-z0-9]+([\.-][a-z0-9]+))*$",$email)) {
header( 'Location: order/invalidemail.html' ) ;
}else{
header( 'Location: order/messagesent.html' ) ;
}
if($sent)
{print "Your custom order query was sent successfully."; }
else
{print "An error occured while sending your custom order query. Please try again."; }
?>

[bradgrafelman]

You've got a few issues:

1. $colors[] = $_REQUEST['colors[]'];

   For one, where is $colors ever defined first? For another, PHP will never place names like 'colors[]' in the superglobal arrays; instead, $_REQUEST['color'] will exist and be an array.

2. $headers = "From: $email"; 

   For one, you never sanitize the user-supplied data before you put it directly into an e-mail header, which means your code will be vulnerable to header injections and thus could become an open relay.

   For another, if you're going to use a user-supplied e-mail in the 'From:' header, you need to add a 'Sender:' header that points to a valid e-mail address on your domain. Otherwise, MTAs will assume that you're trying to forge an e-mail being sent from a domain over which you don't have the authority to do so. See this article for a more in-depth explanation.

3. $sent = mail($to, $from, $subject, $message, $budget, $headers) ; 

   This makes no sense; [man]mail/man expects at most 5 parameters whereas you've given it 6. Read the manual page linked to in order to learn how this function works.

4. if( !eregi("^[a-z0-9]+([_\\.-][a-z0-9]+)*". "@([a-z0-9]+([\.-][a-z0-9]+))*$",$email)) 

   Again, multiple issues here. For one, don't use any of the ereg*() functions - they've all be deprecated in favor of the [man]PCRE[/man] library of functions instead.

   However, the second problem I have with that is that your regexp pattern rejects perfectly valid e-mail addresses like '[e-mail]' as being invalid. Consider using something more robust such as [man]filter_var/man.

5. header( 'Location: order/invalidemail.html' ) ; 

   The 'Location:' HTTP header requires a full URI to be specified after it (e.g. something that begins with "http(s)://").

[sneakyimp]

There are some helpful tips here for sending mail and figuring out why it is not getting through.

brad's totally right about your script being an open relay. People will use it to spam folks. Like me and you. Please fix it. You should seriously consider using something like PEAR::Mail. At the very least, you should make sure that $email doesn't have any line breaks in it. I malicious hacker could supply an email value containing line breaks like this:

[e-mail]
cc: [e-mail]
cc: [e-mail]
cc: [e-mail]

And then we'd all get emails about v!4gra .