[eLECTROLOSIS]

Hey this is driving me crazy & PHP is my weak point,

Designing this site: http://NathanJY.com/envirohome/index2.html

HTML Form:

<form action="mail.php" action="POST">
<input id="name" type="text" name="name">
<input id="phone" type="text" name="phone">
<input id="email" type="text" name="email">
<textarea id="message" name="message"></textarea>
<button id="send" type="submit" value="Send"></button>
</form>

PHP Code:

<?php $name = $_POST['name'];
$email = $_POST['email'];
$message = $_POST['message'];
$formcontent="From: $name \n Message: $message";
$recipient = "enquiry@s411197265.websitehome.co.uk";
$subject = "Contact Form";
$mailheader = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
echo "Thank You!";
?>

Please help

[estel]

What do you want to do? What about it isn't working?

[GuitarWizard]

Remember to format your code. Below is formatted correctly (it will look better when pasted into an editor) its easier to read.

<?php
$name        = $_POST['name'];
$email       = $_POST['email'];
$message     = $_POST['message'];
$formcontent = "From: $name \n Message: $message";
$recipient   = "enquiry@s411197265.websitehome.co.uk";
$subject     = "Contact Form";
$mailheader  = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
echo "Thank You!";
?>

Your not checking if the Send button has been pressed in the PHP script. I'd use the isset function
http://uk3.php.net/manual/en/function.isset.php

[mfaxford]

It would help if you told us why it wasn't working. What happens when you try running this ?

It may be possible your web provider doesn't allow use of the mail function.

More general comments. You should have some error and user input checking in there. For any program never trust that the user will input what they're supposed to do.

A better way to write this type of script might be to combine everything in a single file. The initial thing to do is check if the form has been submitted and check user input. If the user input all passes the checks send the email. If there are missing fields and/or invalid input display the form. You can then include the already filled out fields for the user so they don't have to re-type everything.

[eLECTROLOSIS]

(Original post by GuitarWizard)
Remember to format your code. Below is formatted correctly (it will look better when pasted into an editor) its easier to read.

Your not checking if the Send button has been pressed in the PHP script. I'd use the isset function
http://uk3.php.net/manual/en/function.isset.php

(Original post by estel)
What do you want to do? What about it isn't working?

(Original post by mfaxford)
It would help if you told us why it wasn't working. What happens when you try running this ?

It may be possible your web provider doesn't allow use of the mail function.

More general comments. You should have some error and user input checking in there. For any program never trust that the user will input what they're supposed to do.

A better way to write this type of script might be to combine everything in a single file. The initial thing to do is check if the form has been submitted and check user input. If the user input all passes the checks send the email. If there are missing fields and/or invalid input display the form. You can then include the already filled out fields for the user so they don't have to re-type everything.

Okay basically, The email comes through to my 1&1 webmail account however the actual <input> data is not being pulled into the email.

@GuitarWizard will try your script later and let you know because Im actually at work and the design agency I work for probably wont take kindly to me working freelance during working hours haha

[alex-hs]

(Original post by eLECTROLOSIS)
Hey this is driving me crazy & PHP is my weak point,

Designing this site: http://NathanJY.com/envirohome/index2.html

HTML Form:

<form action="mail.php" action="POST">
<input id="name" type="text" name="name">
<input id="phone" type="text" name="phone">
<input id="email" type="text" name="email">
<textarea id="message" name="message"></textarea>
<button id="send" type="submit" value="Send"></button>
</form>

PHP Code:

<?php
$name = $_POST['name'];
$email = $_POST['email'];
$message = $_POST['message'];
$formcontent="From: $name \n Message: $message";
$recipient = "enquiry@s411197265.websitehome.co.uk";
$subject = "Contact Form";
$mailheader = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
echo "Thank You!";
?>

Please help

Try method="POST" instead of action="POST"

Without the method attribute, I think it defaults to GET rather than POST.


Also, I'd change the button element to an input: if its type attribute is set to submit, it becomes a button by default anyway, and is in my experience the standard way of doing it.

[JamieClark]

Indeed - alex-hs is correct - you have no 'method' attribute specified and so it will default to GET where as your processor is reading POST data which is not being sent with the form.

Furthermore, you should really validate and/or sanitize the user input to that form processor. The $mailheader variable in particular is open to header injection since the email field could contain any arbitrary mail headers and allow attackers to abuse the form.

Though Suhosin or mod_security may be installed to prevent such abuse it is not safe to assume that this is the case. In this case an easy fix would be to ensure that the value in the email field is actually an e-mail address:

<?php
$email = filter_var($_POST['email'], FILTER_VALIDATE_EMAIL);
if ( $email === FALSE )
{
    // invalid email
}
?>

[tamimi]

Hey OP, couldn't spot anything wrong with it. Except that you used action="post".

Did you mean Method="post"?

Also, kudos, liking the website. Quite shiny.

[Dez]

(Original post by alex-hs)
Also, I'd change the button element to an input: if its type attribute is set to submit, it becomes a button by default anyway, and is in my experience the standard way of doing it.

To expand on this a little, a button is basically the same thing as an <input type="submit"> except you can use arbitrary HTML inside it rather than just plain text. Generally you should use inputs unless you definitely need the extra functionality that <button> provides.

[eLECTROLOSIS]

Thanks Guys,

Sorry about the late reply, been really busy.

I've rep'd most of you, will do the rest when I get more rep :P

[xXedixXx]

First of all. You NEED to clean the data you're getting from the user. You should be doing $email = strip_tags($_POST['email'); at the very least.

[JamieClark]

(Original post by xXedixXx)
First of all. You NEED to clean the data you're getting from the user. You should be doing $email = strip_tags($_POST['email'); at the very least.

Whilst I do advocate cleaning the data, why exactly would you recommend using strip_tags() in this instance?

That function is primarily used to clean user input data that is intended to be output amongst some HTML (be it a web page or HTML e-mail) and that is not the case here. Since the e-mail is text only the HTML won't be rendered.

Furthermore it would still leave the form open to mail header injection. Since the e-mail address has a fixed format it would be far better in this case to check the e-mail address is valid.

[xXedixXx]

(Original post by JamieClark)
Whilst I do advocate cleaning the data, why exactly would you recommend using strip_tags() in this instance?

That function is primarily used to clean user input data that is intended to be output amongst some HTML (be it a web page or HTML e-mail) and that is not the case here. Since the e-mail is text only the HTML won't be rendered.

Furthermore it would still leave the form open to mail header injection. Since the e-mail address has a fixed format it would be far better in this case to check the e-mail address is valid.

I understand what you're saying; that was merely an example of what he should be doing.

I personally usually create a function along the lines of:

class utils {

   public function cleanString($string) {
      $string = strip_tags($string);
      $string = mysql_real_escape_string($string);
      ...
      ...
      ...
      return $string;
   }

} 

[mfaxford]

(Original post by xXedixXx)
I understand what you're saying; that was merely an example of what he should be doing.

I personally usually create a function along the lines of:

Just running all input through the likes of strip_tags, mysql_real_escape_string, and other similar functions could lead to a false sense of security and cause you other issues.

It's usually best to perform checks based on what the input should be. If it's an email ensure it contains an @ symbol and at least one . after the @ and that the address and domain only contain valid characters. You should be able to check that with a single preg_match or preg_grep line. If you just used the standard php functions it will take more code and potentially not stop email header injection.

[xXedixXx]

(Original post by mfaxford)
Just running all input through the likes of strip_tags, mysql_real_escape_string, and other similar functions could lead to a false sense of security and cause you other issues.

It's usually best to perform checks based on what the input should be. If it's an email ensure it contains an @ symbol and at least one . after the @ and that the address and domain only contain valid characters. You should be able to check that with a single preg_match or preg_grep line. If you just used the standard php functions it will take more code and potentially not stop email header injection.

A cleanString function is easy to manipulate; for example you could send another parameter, which could be the type of clean you need to do; whatever suits the purpose. All I'm doing is giving an example of basic security; all input from the user should go through a cleaning process. This is why I'm starting to sway more towards Django.. all the input is cleaned for you.

[Dez]

(Original post by xXedixXx)
A cleanString function is easy to manipulate; for example you could send another parameter, which could be the type of clean you need to do; whatever suits the purpose. All I'm doing is giving an example of basic security; all input from the user should go through a cleaning process. This is why I'm starting to sway more towards Django.. all the input is cleaned for you.

So... like magic_quotes then?

[xXedixXx]

(Original post by Dez)
So... like magic_quotes then?

For a start, magic quotes are deprecated and are being removed in version 5.4 of PHP. Secondly unlike magic quotes Django still allows you to use the raw data if you choose to. Django also provides a very simple way to stop cross site scripting by using a csr token, and doesn't allow POST data from pages that don't use a csr token.

[estel]

(Original post by xXedixXx)
For a start, magic quotes are deprecated and are being removed in version 5.4 of PHP. Secondly unlike magic quotes Django still allows you to use the raw data if you choose to. Django also provides a very simple way to stop cross site scripting by using a csr token, and doesn't allow POST data from pages that don't use a csr token.

There are frameworks that do all this on PHP though, right? It's not entirely fair to compare PHP to Django.