email form jumble

two templates do not equal one

[simulacrud]

I've combined two email form templates to try to better suit my needs, but clearly i've messed something up.

if

there is an injection, then it should redirect to the 404 page and

die

.

else

, it should send the email and redirect to the success page. currently it only redirects to the 404 page, no email sent. I've tried and retried manipulating the code to work, but I really just don't understand the basic php grammar yet, so I'm very stuck. this website is supposed to go live tomorrow (!)
What am i doing wrong?
Btw, i've removed all of the php generated error messages, because I don't know how to make them look as good as the js+css error validation messages. perhaps I've removed too much?

<?php

$myemail = "[e-mail]";

function checkFields($values){
$injection = false;
for ($n=0;$n<count($values);$n++){
if (eregi("%0A",$values[$n]) || eregi("%0D",$values[$n]) || eregi("\\r",$values[$n]) || eregi("\\n",$values[$n])){
$injection = true;
}
}
return $injection;
}

$name = $_POST['name'];
$subject = "Message from _____ Site";
$email = $_POST['email'];
$website = $_POST['website'];
$comments = $_POST['comments'];
$result = checkFields(Array($name,$email,$website,$comments));

if ($result==true){
header('Location: ../../404.html');
die;
} else {

$message = "

Your contact form has been submitted by:

Name: $name
E-mail: $email
URL: $website

Comments:
$comments

";

mail($myemail, $subject, $message);

header('Location: ../success.html');
exit;
}

?>

[Matthew1980]

Greetings simulacrud,

Welcome to the forum :) [webmasterworld.com ]

Just to advise, the ereg functions are now deprecated, meaning that some servers will now give errors if using newer versions of PHP: [webmasterworld.com ] have a read of that to understand more :)

As for the email script, you say you are trying to catch in injection? Are you meaning cleansing the user generated input, of so, strip_tags(), and trim() would do the job, that is unless I have misunderstood you :)

As for the email formatting you could do this:-

$name = strip_tags($_POST['name']);
$subject = "Message from _____ Site";
$email = strip_tags($_POST['email']);
$website = strip_tags($_POST['website']);
$comments = strip_tags($_POST['comments']);

$message = "Your contact form has been submitted by:\n\r";
$message .= "Name: ".$name."\n\r";
$message .= "E-mail: ".$email."\n\r";
$message .= "URL: ".$website."\n\r";
$message .= "Comments:".$comments."\n\r";
//type plain text or html, delete/comment out as you need :)
$mailheaders = "MIME-version: 1.0\r\n";
$mailheaders .= "content-type: text/plain; charset=UTF-8\r\n";
//for this example I will use plain text
//$mailheaders .= "content-type: text/html; charset=ISO-8859-1\r\n";

mail($myemail, $subject, $message, $mailheaders);

With the function as you have it, I am not sure what you are trying to filter for, but just using strip_tags() helps with making the sent mail more secure. I tidied up the mail formatting now so that it's not all on one line now, and added the mail headers for you too.

Also, when using header(); it may be wise to use the absolute url filepath to the success file, ie:-
header('Location: h t t p://www.path/to/the/file/success.html');
exit;

WRT the $result var, have you tried echoing that to see if the state ever changes true/false, before you engage the if/else clause - because something doesn't look right with the function to me, but I don't know enough about regex patterns to advise there...

Hope this helps a little,

Cheers,
MRb

[simulacrud]

Your script certainly works, thank you!
I was trying to prevent injections like the ones mentioned here [w3schools.com ]. Will strip_tags() help with that?

[Matthew1980]

Hi there simulacrud,

Cool, glad It did what it needed to. strip_tags() is probably the most common way of 'stripping' html style tags from whatever the user has imputted, if you were simultaneously inserting into a sql DB then using mysql_real_escape_string() would be a way to make data safe for use in sql queries.

From what I have read on that link, it wouldn't prevent extra mailheaders being sent in the example they give - but I can now see what you tried to do :)

So, because you only need to validate ONE field, you can just adapt you existing script to something like this:-

function EmailSanitise($field)
{
//Sanitize email
$field=filter_var($field, FILTER_SANITIZE_EMAIL);

//Filter nominated address using FILTER_VALIDATE_EMAIL
if(filter_var($field, FILTER_VALIDATE_EMAIL))
{
return TRUE;
}
else
{
return FALSE;
}
}

Then:-

You need to set a form 'catch' handler, ie the submit button & its value:

if(isset($_POST['submit']) && ($_POST['submit'] == "send email")){
if(empty($_POST['email']) || empty($_POST['name']) || empty($_POST['subject']) || empty($_POST['comments'])){
//blank form submission redirect back to form :)
header("location: yourEmailForm.php");
exit;
}
//form filled in correctly

//assign $_POST vars etc
$emailFromForm = strip_tags($_POST['emailFromForm']);
$name = strip_tags($_POST['name']);
$subject = "Message from _____ Site";
$email = strip_tags($_POST['email']);
$website = strip_tags($_POST['website']);
$comments = strip_tags($_POST['comments']);

//perform your checks from W3C Schools :)
if(!EmailSanitise($emailFromForm))){
echo "Only valid email address's please";
exit;//or redirect back to form using header("Location: yourEmailForm.php");
}

//carry on with the email :)

//Construct message
$message = "Your contact form has been submitted by:\n\r";
$message .= "Name: ".$name."\n\r";
$message .= "E-mail: ".$email."\n\r";
$message .= "URL: ".$website."\n\r";
$message .= "Comments:".$comments."\n\r";
//type plain text or html, delete/comment out as you need :)
$mailheaders = "MIME-version: 1.0\r\n";
$mailheaders .= "content-type: text/plain; charset=UTF-8\r\n";
//for this example I will use plain text
//$mailheaders .= "content-type: text/html; charset=ISO-8859-1\r\n";

//send email - this function does not guarantee sending an email, the only confirmation
//is when you get it in the inbox :(
if(mail($myemail, $subject, $message, $mailheaders)){
//succesful mail (a boolean is returned)
header("location: successpage.php");
exit;
}
else{
header("location: error_page.php");//have a link back to form on the error page?
exit;
}

}
else{
//redirect back to form, erroneous form post?
header("location: yourEmailForm.php");
}

Well you can see the logic in that example I hope, typed On-The-Fly, so could be a typo or two there, but you get the idea.

Also, for reference:-

$result = checkFields();//leave parameter field blank

Only reason I suggest this, is because, the $_POST data is a superglobal, so by definition, this & its data/value is available throughout the scope of your script from when its set, no need to pass in into a function as it is already available - pretty much the same behaviour as a constant - except the data is variable :)

Hope this helps a little,

Cheers,
MRb

[g1smd]

The

../../404.html

construct will come back to haunt you in multiple ways.

Always use the absolute full protocol, domain, and path in a header directive.

However in this case, do not use the header with a URL. Your code produces a 302 redirect to an error message that is then served with 200 OK status. That's a bad thing.

Instead, change the header directive to directly send the HTTP 404 Not Found status code, and then "include" the error message page so that the error message is directly served with the attached 404 response.

Do not redirect.

[simulacrud]

Matthew: thanks so much for your thorough assistance. I have a better idea of how to put it all together now, though I still haven't gotten the combination you suggested to work properly. I need to study up on my php grammar.

@g1smd: You're right! I didn't realize how important it was to use the absolute path in dynamic files.

[Matthew1980]

Hi there simulacrud,

>>Also, when using header(); it may be wise to use the absolute url filepath to the success file, ie:-
header('Location: h t t p://www.path/to/the/file/success.html');
exit;

Definitely helps to cure any typo's, especially if you use the $_SERVER['DOCUMENT_ROOT']; global, this will save time and is always available throughout the script - so long as it defined in the ini file correctly, this can be used like this:-

header("location:".$_SERVER['DOCUMENT_ROOT']."somefile.php");
exit;

though I can't remember if you need the trailing slash there or not, but you get the idea :)

Well, keep playing and should you get any more error's or need any more help, just post a thread, and there are plenty of people here who can help/offer assistance.

Have fun!

Cheers,
MRb