• Registered: 2009-09-08
  • Posts: 3

Topic: First-time form from scratch - probably doing a LOT wrong

<?php
if (isset($_POST['submit'])) {

    $to = "someone@somewhere.com";
    $subject = "Website Order";
    $name_field = $_POST['name'];
    $phone_field = $_POST['phone'];
    $mobile_field = $_POST['mobile'];
    $email_field = $_POST['email'];
    $address = $_POST['address'];
    $postcode_field = $_POST['postcode'];
    foreach ($_POST['regionalSeries'] as $value) {
        $radio_msg .= "Regional Series: $value";
    }
    if (isset($_POST[‘personalised’])) {
        foreach ($_POST['personalised'] as $value) {
            $radio_msg2 .= "Regional Series: $value\n";
        }
    }
    foreach ($_POST['locations'] as $value) {
        $check_msg .= "Locations: $value\n";
    }
    //foreach ($_POST['personalised'] as $value) {
    //            $radio_msg2 .= "Personalised: $value";
    //        }

    $description = $_POST['description'];
    $instructions = $_POST['instructions'];
    $body = "\nFrom: $name_field\nTelephone: $phone_field\nCellphone: $mobile_field\nE-Mail: $email_field\n\nDelivery Address: $address\nPostcode: $postcode_field\n\n$radio_msg\n$check_msg\n\n$radio_msg2\nPersonalised: $description\nSpecial Instructions: $instructions";

    //echo "Data has been submitted to $to!";
    header("Location: thank-you.html");
    mail($to, $subject, $body);

} else {

    echo "blarg!";

}
?>

What I'm trying to do here is (well, probably obvious) - just not to me.
Any advice on how to make this better?
Error correction. Hmmm... Security? What's that?
Basically, it works (well, the $radio_msg2 doesn't display in the email) so I guess it doesn't really.
I'm just looking for some pointers, to improve the way the form data is handled.
If this isn't the right place to post such a thing - I apologise in advance.
Thanks for looking.
Jas

  • Registered: 2009-01-13
  • Posts: 486

Re: First-time form from scratch - probably doing a LOT wrong

where is the form code?

  • Registered: 2009-09-08
  • Posts: 3

Re: First-time form from scratch - probably doing a LOT wrong

Apologies, here we go;

<form action="sendmail.php" method="post">
      <table width="100%" border="0" cellspacing="0" cellpadding="0">
        <tr>
          <td width="15%"><label for="name"><strong style="color:#F00;">*</strong> Name:</label></td>
          <td width="85%"><input name="name" id="name" type="text" /> (This name will appear on your invoice).</td>
        </tr>
        <tr>
          <td><label for="phone">Telephone:</label></td>
          <td><input name="phone" id="phone" type="text" /></td>
        </tr>
        <tr>
          <td><label for="mobile">Cellphone:</label></td>
          <td><input name="mobile" id="mobile" type="text" /></td>
        </tr>
        <tr>
          <td><label for="email"><strong style="color:#F00;">*</strong> E-mail:</label></td>
          <td><input name="email" id="email" type="text" /></td>
        </tr>
        <tr>
          <td colspan="2"><label for="address"><strong style="color:#F00;">*</strong> Delivery Address:</label></td>
        </tr>
        <tr>
          <td colspan="2"><textarea name="address" id="address" style="width:745px;"></textarea></td>
        </tr>
        <tr>
          <td><label for="postcode">Postcode:</label></td>
          <td><input name="postcode" id="postcode" type="text" /></td>
        </tr>
      </table>
      <table width="100%" border="0" cellspacing="0" cellpadding="0">
        <tr>
          <td colspan="5" class="top-rule">Regional Series</td>
        </tr>
        <tr>
          <td colspan="5">&nbsp;</td>
        </tr>
        <tr>
          <td valign="top">Size:</td>
          <td valign="top"><input type="radio" class="checkbox" name="regionalSeries[]" value="600x1000" /></td>
          <td valign="top">600mm wide x 1000mm high<br />
            <strong>$490</strong></td>
          <td valign="top"><input type="radio" class="checkbox" name="regionalSeries[]" value="900x1500" /></td>
          <td valign="top">900mm wide x 1500mm high<br />
            <strong>$690</strong></td>
        </tr>
      </table>
      <table width="100%" border="0" cellspacing="0" cellpadding="0">
        <tr>
          <td colspan="6">North Island</td>
        </tr>
        <tr>
          <td width="5%"><input type="checkbox" name="locations[]" value="Auckland" /></td>
          <td width="16%">Auckland</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Hawkes Bay/Bay of Plenty/Gisborne" /></td>
          <td width="31%">Hawkes Bay/Bay of Plenty/Gisborne</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Northland" /></td>
          <td width="38%">Northland</td>
        </tr>
        <tr>
          <td width="5%"><input type="checkbox" name="locations[]" value="Wellington" /></td>
          <td>Wellington</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Taranaki/Manawatu/Wanganui" /></td>
          <td>Taranaki/Manawatu/Wanganui</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Waikato" /></td>
          <td>Waikato</td>
        </tr>
        <tr>
          <td colspan="6">&nbsp;</td>
        </tr>
        <tr>
          <td colspan="6">South Island</td>
        </tr>
        <tr>
          <td width="5%"><input type="checkbox" name="locations[]" value="Canterbury" /></td>
          <td>Canterbury</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Tasman/Nelson/Marlborough" /></td>
          <td>Tasman/Nelson/Marlborough</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="West Coast" /></td>
          <td>West Coast</td>
        </tr>
        <tr>
          <td width="5%"><input type="checkbox" name="locations[]" value="Otago" /></td>
          <td>Otago</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Central Otago" /></td>
          <td>Central Otago</td>
          <td width="5%"><input type="checkbox" name="locations[]" value="Southland" /></td>
          <td>Southland</td>
        </tr>
        <tr>
          <td colspan="6">&nbsp;</td>
        </tr>
        <tr>
          <td colspan="6">New Zealand</td>
        </tr>
        <tr>
          <td colspan="6"><input type="checkbox"  name="locations[]" value="New Zealand" /></td>
        </tr>
        <tr>
          <td colspan="6">&nbsp;</td>
        </tr>
      </table>
      <table width="100%" border="0" cellspacing="0" cellpadding="0">
        <tr>
          <td colspan="5" class="top-rule">Personalised</td>
        </tr>
        <tr>
          <td colspan="5">&nbsp;</td>
        </tr>
        <tr>
          <td valign="top">Size:</td>
          <td valign="top"><input type="radio" class="checkbox" name="personalised[]" value="600x1000" /></td>
          <td valign="top">600mm wide x 1000mm high<br />
            <strong>$590</strong></td>
          <td valign="top"><input type="radio" class="checkbox" name="personalised[]" value="1000x1500" /></td>
          <td valign="top">1000mm wide x 1500mm high<br />
            <strong>$790</strong></td>
        </tr>
        <tr>
          <td colspan="5">Please note down approximately 16 names (i.e. place names, roads, streets, mountains, lakes, beaches, rivers etc.) Try to vary the
            length of the words to create more interest. We choose the 12 names that fit best in the design so please underline the
            important ones that you don’t want missed out of the design & we will endeavour to fit them in.</td>
        </tr>
        <tr>
          <td colspan="5">&nbsp;</td>
        </tr>
        <tr>
          <td colspan="5"><textarea name="description" id="description" style="width:745px;"></textarea></td>
        </tr>
        <tr>
          <td colspan="5">&nbsp;</td>
        </tr>
        <tr>
          <td colspan="5" class="top-rule">Special Instructions</td>
        </tr>
        <tr>
          <td colspan="5"><textarea name="instructions" id="instructions" style="width:745px;"></textarea></td>
        </tr>
      </table>
      <input name="submit" id="submit" value="" type="submit" style="background-image:url(../images/jpg/order-now.jpg); width:122px; height:32px;" />
    </form>
  • Registered: 2009-01-13
  • Posts: 486

Re: First-time form from scratch - probably doing a LOT wrong

Aside from the lack of Security, I would do the mail() before the header() at the bottom of the code snippet in the first posting. Otherwise, the email would never be sent since the header would re-direct.
It is really, really, really important that you "screen" the user input before you handle it. This form is wide open for email server high-jacking and a Spammer's Delight if they find it.
There are tons of resources out there for how to manage the POST data for emailing. Google it.

<?php

/* ************************************************************************
*
* function used to clean Mail :: from Larry Ullman at dmcinsights.com
*
* as found here: [url]http://www.dmcinsights.com/phorum/read.php?6,28810[/url]
*
* called by the following line on the mail page prior to using the mail()
*
* $_SAFE_POST = array_map('clear_user_input', $_POST);
*
* cleans each element of the $_POST array before using them in the mail() using array_map
*
*************************************************************************** */

function clear_user_input($value) {

    // Check for bad values:
    if (stristr($value, 'content-type')) return '';
    if (stristr($value, 'bcc:')) return '';
    if (stristr($value, 'to:')) return '';
    if (stristr($value, 'cc:')) return '';
    if (stristr($value, 'href')) return '';


    // Strip quotes, if Magic Quotes are on:
    if (get_magic_quotes_gpc()) $value = stripslashes($value);
    
    // Replace any newline characters with spaces:
    $value = str_replace(array( "\r", "\n", "%0a", "%0d"), ' ', $value);
    
    // Return the value:
    return trim($value);
    
}

?>

here is a function to start your user-input screening.

Last edited by jlhaslip (2009-09-09 19:53:28)

  • Registered: 2009-09-08
  • Posts: 3

Re: First-time form from scratch - probably doing a LOT wrong

Thanks for the feedback jlhaslip,
I knew the security was lacking, I was going to deal with that after I got the thing actually working correctly.
I will (as you say) Google it.
Cheers
Jas