tags:

views:

90

answers:

2

I'm building a simple order system and want to send an email after the form is submitted. My PHP code looks similar to this:

$name=$_POST["orderName"];
$company=$_POST["orderCompany"];
$email=$_POST["orderEmail"];
$phone=$_POST["orderPhone"];
$headers = "From: $email\r\n" .

$item1=$_POST["orderItem1"];
$qty1=$_POST["orderQty1"];

$item2=$_POST["orderItem2"];
$qty2=$_POST["orderQty2"];

$item3=$_POST["orderItem3"];
$qty3=$_POST["orderQty3"];

$date = date("l, F j Y, G:i") ;

$message="Message sent: $date \n\n

Name: $name\n
Company: $company\n
Email: $email\n
Phone: $phone\n\n

Order:\n
$item1 \tx$qty1\n
$item2 \tx$qty2\n
$item3 \tx$qty3\n";

mail("[email protected]", "Order", $message, $headers);

That works fine, except in the body of the email I get the value of $item1 string at the very beginning, before the "Message sent..." - just like I added it to the $message (which I don't as far as I can see).

+6  A: 

Where you have this:

$headers = "From: $email\r\n" .

you want this instead:

$headers = "From: $email\r\n";

Otherwise, you're concatenating whatever comes on the next line (which happens to be the definition for $item1) to the end of $headers. Although that's not technically valid (i.e., the content is part of the message headers and not body), most e-mail clients will effectively shrug and show it anyway.

VoteyDisciple
+1 I bet you that's it.
karim79
Thanks, that was very dumb of me.
maciej.gryka
+1  A: 

Please, please, please add some sanitizing to your POST variables before going with this in production.

Let's see here:

$email=$_POST["orderEmail"];
$headers = "From: $email\r\n";
mail("[email protected]", "Order", $message, $headers);

I could send a POST request where "orderEmail" contains:

"[email protected]\r\n
 From: [email protected]\r\n
 BCC: [email protected], [email protected]"

etc. and your harmless form would work great for me sending spam to the whole world. This site suggects:

if ( ereg( "[\r\n]", $name ) || ereg( "[\r\n]", $email ) ) {
    [... direct user to an error page and quit ...]
}
enkrs
I will do that, thanks for the warning!
maciej.gryka