On Sat, Mar 2, 2013 at 5:04 AM, Michael CALDER
<mikecal...@optusnet.com.au> wrote:
>
> -- G'day ,

G'day, cobber!

> Here is the current contact2.php file - but the SUBJECT only shows as
> RVRA Contact Form -

Others have addressed that, as well as not needing the stripslashes.
If you will permit, I have a few other comments. If not, simply
delete. No worries!

I added line numbers to make notation easier.


     1  > <?php
     2  >
     3  > // get posted data into local variables
     4  > $EmailAddress = Trim(stripslashes($_POST['EmailAddress']));
     5  > $EmailTo = "mikecal...@optusnet.com.au";

You might want to use a configured value instead of the direct string.
If you have to change the email address, or other similar things, it's
easier to change a configuration file than looking through code.

     6  > $Subject = "RVRA Contact Form - ,$MessageSubject";

Similarly here, the subject of the email could be a configured item.

     7  > $Name = Trim(stripslashes($_POST['Name']));
     8  > $EmailAddress = Trim(stripslashes($_POST['EmailAddress']));

You've duplicated this from line 4.


     9  > $YesNo = Trim(stripslashes($_POST['YesNo']));
    10  >
    11  > $MessageSubject = Trim(stripslashes($_POST['MessageSubject']));
    12  > $Message = Trim(stripslashes($_POST['Message']));
    13  >
    14  > // send email
    15  > if(!isset($_REQUEST['identiPIC_selected'])){exit;}

Instead of just throwing an exit here, and giving the user a blank
page, maybe better to redirect to some landing page. This is done by
issuing a header before any other text is sent:

header("Location: your-error-landing-page.html");

    16  >
    17  > $identiPIC[1] = "Bird";
    18  > $identiPIC[2] = "Logo";
    19  > $identiPIC[3] = "Flower";
    20  >
    21  > if($_REQUEST['identiPIC_selected'] !== $identiPIC){print "<meta
    22  > http-equiv=\"refresh\" content=\"0;URL=error-pic.html\">"; exit;}

Here, instead of making the browser refresh, do a redirect to a landing page.
See above for header.

    23  >
    24  >
    25  >
    26  >
    27  > // prepare email body text
    28  > $Body = "";
    29  >
    30  > $Body .= "Name: ";
    31  > $Body .= $Name;
    32  > $Body .= "\n";
    33  >
    34  > $Body .= "EmailAddress: ";
    35  > $Body .= $EmailAddress;
    36  > $Body .= "\n";
    37  >
    38  > $Body .= "RVRA Member";
    39  > $Body .= $YesNo;
    40  > $Body .= "\n";
    41  >
    42  >
    43  >
    44  > $Body .= "Message Subject: ";
    45  > $Body .= $MessageSubject;
    46  > $Body .= "\n";
    47  >
    48  > $Body .= "Message: ";
    49  > $Body .= $Message;
    50  > $Body .= "\n";

This is the ideal place for a HEREDOC
(http://www.php.net/manual/en/language.types.string.php#language.types.string.syntax.heredoc)
to make your code more readable and maintainable:

$Body <<ENDOFMAIL

Name: $Name
EmailAddress: $EmailAddress
RVRA Member: $YesNo
Message Subject: $MessageSubject

$Message

ENDOFMAIL

    51  >
    52  >
    53  > // send email
    54  > $success = mail($EmailTo, $Subject, $Body, "From: <$EmailAddress>");

The fourth parameter is actually any additional headers, and should be
constructed in the standard mail format, the header name, the proper
contents and terminated by a CR-LF.

In this case, the From: header needs to correspond to an RFC2922
address, or list of such addresses. If you want to use the angle
brackets, you need to put in a user name before it. Otherwise, simply
omit the brackets. All the additional headers specified in the fourth
parameter need to end with a CR-LF as well, not just a LF, or in this
case, nothing. The way this is set up, the following would be proper
setup:

$AdditionalParms = "From: \"$Name\" <$EmailAddress>\r\n";

Then:

$success = mail ($EmailTo, $Subject, $Body, $AdditionalParms);

The escaped quotes around $Name allow for such things as
non-alnum+space characters, i.e., something like this would be
allowed:

From: "Samuel L. Jackson, Jr." <sammyj...@example.com>


However, in this case, the email is NOT actually being sent from
$EmailAddress, it is being sent from your server, so putting in that
address is technically a spoof. While that's not illegal, some mailing
systems will call that a spam message, as the From and Sender do not
match, and From won't be found at the originating mail node.

    55  >
    56  > // redirect to success page
    57  > if ($success){
    58  >     print "<meta http-equiv=\"refresh\"
content=\"0;URL=thanks.html\">";
    59  > }
    60  > else{
    61  >     print "<meta http-equiv=\"refresh\"
content=\"0;URL=error-pic.html\">";
    62  > }
    63  >

Here again, give the idea of using a header redirect instead of a
browser refresh a go.


    64  > ?>

-- 
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to