Forum Moderators: coopster
I am having trouble with my contact form not submitting - I would like it to error check and highlight the fields not filled in (keeping the ones that were filled in). I tried it on firefox once and it came up with a message saying that the png had an error in it - could be a local setting.
Here it is:
<?php
Header("Content-Type: image/png");
session_start();
if (isset($_POST[submit])) {
$fo = $_POST['for'];
$fn = $_POST['firstname'];
$ln = $_POST['lastname'];
$ea = $_POST['emailaddress'];
$qu = $_POST['question'];
$random = trim($random);
if (!$new_string == $random){
header("Location: contact.php?error=verification&errornote=TRUE");
}
elseif ($fn == "") {
header("Location: contact.php?error=firstname&errornote=TRUE");
}
elseif (($ea == "") ¦¦ (!eregi("^[a-zA-Z0-9_]+@[a-zA-Z0-9\-]+\.[a-zA-Z0-9\-\.]+$", $ea))) {
header("Location: contact.php?error=emailaddress&errornote=TRUE");
}
elseif ($qu == "") {
header("Location: contact.php?error=question&errornote=TRUE");
}
else {
mail ("email@myemail.com", "$fo", "$question", "from: $ea");
header("Location: contact.php?note=thankyouverymuch");
}
}
elseif ($note==thankyouverymuch)
{
$page="contact"; require ("inc/top.inc.php");
echo "<table cellpadding=\"0\" cellspacing=\"0\" width=\"100%\" class=\"content\">";
echo "<tr>";
echo "<td valign=\"top\">";
echo "<h1>Thank you!</h1>";
echo "<br />";
echo "<h3>We will be in contact with you as soon as possible</h3>";
echo "</div>";
echo "</td>";
echo "</tr> ";
echo "</table>";
require ("inc/footer.inc.php");
}
else {
$page="contact"; require ("inc/top.inc.php");
$new_string;
session_register('new_string');
$im = ImageCreate(200, 40);
$white = ImageColorAllocate($im, 255, 255, 255);
$black = ImageColorAllocate($im, 0, 0, 0);
srand((double)microtime()*1000000);
$string = md5(rand(0,9999));
$new_string = substr($string, 17, 5);
ImageFill($im, 0, 0, $white);
ImageString($im, 4, 45, 19, $new_string, $black);
/* Directory inc/c/ is CHMODDed to 777 */
ImagePNG($im, "inc/c/verify.png");
ImageDestroy($im);
echo "<table cellpadding=\"0\" cellspacing=\"0\" width=\"100%\" class=\"content\">";
echo "<tr>";
echo "<td valign=\"top\">";
echo "<h1>Contact us</h1>";
echo "<br />All fields maked * are required";
if ($errornote=='TRUE') {
echo "<br /><br />Please fill in all fields marked <span style=\"color: red;\">RED</span>";
}
echo "<div id=\"contactform\">";
echo " <form action=\"contact.php\" method=post>";
echo "<label for=\"for\">For: *</label>";
echo "<select name=\"for\">";
echo "<option>Business";
echo "<option>Help";
echo "<option>Personal";
echo "</select><br />";
echo "<label for=\"firstname\">First name: *</label>";
echo "<input type=\"text\"";
echo "name=\"firstname\"";
if ($error==firstname) {
echo " style=\"border: 1px solid red;\"><br />";
}
else {
echo "><br />";
}
echo "<label for=\"lastname\">Last name:</label>";
echo "<input type=\"text\" name=\"lastname\"";
if ($error==lastname) {
echo " style=\"border: 1px solid red;\"><br />";
}
else {
echo "><br />";
}
echo "<label for=\"emailaddress\">Email address: *</label>";
echo "<input type=\"text\" name=\"emailaddress\"";
if ($error==emailaddress) {
echo " style=\"border: 1px solid red;\"><br />";
}
else {
echo "><br />";
}
echo "<label for=\"question\">";
if ($error==question) {
echo "<span style=\"color: red;\">Question *</span></label>";
}
else {
echo "Question</label>";
}
echo "<textarea name=\"question\" rows=\"10\" cols=\"10\"></textarea><br />";
echo "<img src=\"inc/c/verify.png\">";
echo "<br />";
echo "Please type the contents of the image *";
echo "<br /><br />";
echo "<input name=\"random\" type=\"text\" value=\"\"";
if ($error==verification) {
echo "style=\"border: 1px solid red;\">";
}
else {
echo ">";
}
echo "<br /><br />";
echo "<input type=\"submit\" value=\"Send\">";
echo "</form>";
echo "</div> ";
echo "</td> ";
echo "</tr> ";
echo "</table> ";
echo "<h3>We respect your privacy and we will never sell your email address to any third party. The information submitted in this form will be sent directly to Erin Wheelan, managing director of Eyelights Ltd.</h3>";
require ("inc/footer.inc.php");
}
?>
The one thing that you do NOT want to do is to use received variables without checking. $_POST[] variables are one classic example.
Somewhere near the top of your code you need a little security section in which you disinfect all your $_POST[], $_GET[], $_COOKIE[] etc variables. As an obvious example, convert all the boolean entries:
$INTERNAL = ( isset( $_REQUEST[ 'booleanvariablename' ]))? TRUE : FALSE;
$INTERNAL is now a 'clean' copy of $booleanvariablename. Integers should be converted to Integer type, and strings need to be checked for illegal contents.
Simply: never, ever, assume that received external variables are clean unless you have disinfected them. If you want to get the point, examine a little of the phpBB2 history. You may well think that it is not necessary in your particular case. Until the day comes that your site gets hacked.
Where do I put the $_POST is it the "$_" which is the problem?
Do this:
$firstname = ( string ) $_POST['firstname'];
(now disinfect $firstname - eg if max 60 chars, then truncate to 60 chars, etc etc. Then:)
.
echo "<label for='firstname'>First name: *</label>
<input type='text' value='$firstname' name='firstname'";
this is my updated code. I don't get how to implement the security fix... do I have that for a sample variable - like $fn? - also, I have already made $fn have the value of $_POST['firstname']...
<?php
session_start();
$INTERNAL = ( isset( $_REQUEST[ '$fn' ]))? TRUE : FALSE;
if (isset($_POST[submit])) {
$fo = $_POST['for'];
$fn = $_POST['firstname'];
$ln = $_POST['lastname'];
$ea = $_POST['emailaddress'];
$qu = $_POST['question'];
$firstname = (string) //? What string? nl2br?
$_POST['firstname'];
$random = trim($random);
if (!$new_string == $random){
header("Location: contact.php?error=verification&errornote=TRUE");
}
elseif ($fn == "") {
header("Location: contact.php?error=firstname&errornote=TRUE");
}
elseif (($ea == "") ¦¦ (!eregi("^[a-zA-Z0-9_]+@[a-zA-Z0-9\-]+\.[a-zA-Z0-9\-\.]+$", $ea))) {
header("Location: contact.php?error=emailaddress&errornote=TRUE");
}
elseif ($qu == "") {
header("Location: contact.php?error=question&errornote=TRUE");
}
else {
mail ("email@address.com", "$fo", "$qu from $fn $ln", "from: $ea");
header("Location: contact.php?note=thankyouverymuch");
}
}
elseif ($note==thankyouverymuch)
{
$page="contact"; require ("inc/top.inc.php");
echo "<table cellpadding=\"0\" cellspacing=\"0\" width=\"100%\" class=\"content\">";
echo "<tr>";
echo "<td valign=\"top\">";
echo "<h1>Thank you!</h1>";
echo "<br />";
echo "<h3>We will be in contact with you as soon as possible<br /><br /><a href=\"$path/index.php\">Home</a></h3>";
echo "</td>";
echo "</tr> ";
echo "</table>";
require ("inc/footer.inc.php");
}
else {
$page="contact"; require ("inc/top.inc.php");
$new_string;
session_register('new_string');
$im = ImageCreate(200, 40);
$white = ImageColorAllocate($im, 255, 255, 255);
$black = ImageColorAllocate($im, 0, 0, 0);
srand((double)microtime()*1000000);
$string = md5(rand(0,9999));
$new_string = substr($string, 17, 5);
ImageFill($im, 0, 0, $white);
ImageString($im, 4, 45, 19, $new_string, $black);
/* Directory inc/c/ is CHMODDed to 777 */
ImagePNG($im, "inc/c/verify.png");
ImageDestroy($im);
echo "<table cellpadding=\"0\" cellspacing=\"0\" class=\"content\">";
echo "<tr>";
echo "<td valign=\"top\">";
echo "<h1>Contact us</h1>";
echo "<br />All fields maked * are required";
if ($errornote=='TRUE') {
echo "<br /><br />Please fill in all fields marked <span style=\"color: red;\">RED</span>";
}
echo "<div id=\"contactform\">";
echo " <form action=\"contact.php\" method=post>";
echo "<label for=\"for\">For: *</label>";
echo "<select name=\"for\">";
echo "<option>Product";
echo "<option>Business";
echo "<option>Help";
echo "<option>Personal";
echo "</select><br />";
echo "<label for='firstname'>First name: *</label>
<input type='text' value='$firstname' name='firstname'";
if ($error==firstname) {
echo " style=\"border: 1px solid red;\"><br />";
}
else {
echo "><br />";
}
echo "<label for=\"lastname\">Last name:</label>";
echo "<input type=\"text\" name=\"lastname\">";
echo "<br />";
echo "<label for=\"emailaddress\">Email address: *</label>";
echo "<input type=\"text\" name=\"emailaddress\"";
if ($error==emailaddress) {
echo " style=\"border: 1px solid red;\"><br />";
}
else {
echo "><br />";
}
echo "<label for=\"question\">";
if ($error==question) {
echo "<span style=\"color: red;\">Question *</span></label>";
}
else {
echo "Question *</label>";
}
echo "<textarea name=\"question\" rows=\"10\" cols=\"10\"></textarea><br />";
echo "<img src=\"inc/c/verify.png\">";
echo "<br />";
echo "Please type the contents of the image *";
echo "<br /><br />";
echo "<input name=\"random\" type=\"text\" value=\"\"";
if ($error==verification) {
echo "style=\"border: 1px solid red;\">";
}
else {
echo ">";
}
echo "<br /><br />";
echo "<input type=\"submit\" name='submit' value=\"Send\">";
echo "</form>";
echo "</div> ";
echo "</td> ";
echo "</tr> ";
echo "</table> ";
echo "<br /><h3 class=\"content\"><b>We respect your privacy and we will never sell your email address to any third party. The information submitted in this form will be sent directly to person.</b></h3>";
require ("inc/footer.inc.php");
}
?>
Do you think it is the header location that is the problem? Mayeb I should change this to PHP SELF?
Thanks for your patience.
Do you think it is the header location that is the problem?
There are two things now mixed up in this thread. My fault, since it was me that added the issue of security considerations. I shall reply separately on each score to try to un-mix them. This post will be about the security issues of dealing with user-supplied variables.
Sanitising User Input
This is far too large a topic to be able to encapsulate the whole issue into a 100-word posting. My earlier post at the top of this page was designed first to alert you to the concern that you were using $REQUEST[] variables in your code without any checking. That will, eventually, lead you into grief. And second, to give a couple of pointers as to what you could do to sanitise those variables.
So, a couple of further pointers (not the whole story):
All $REQUEST[] variables will be:
$INTERNAL_BOOLEAN = ( isset( $_REQUEST[ 'externalBooleanVariableName' ]))? TRUE : FALSE;(I use CAPS with these variable names to say that they have been submitted externally then sanitised.)
$INTERNAL_INTEGER = ( int ) $_REQUEST[ 'externalIntegerVariableName' ];
$INTERNAL_FLOAT = ( float ) $_REQUEST[ 'externalFloatVariableName' ];
$INTERNAL_STRING = ( string ) $_REQUEST[ 'externalStringVariableName' ];
The first 3 are now safe (although bounds-checking should be done on the integers and floats), and the only ones that you need to concern yourself with is the $INTERNAL_STRINGs. Sanitising those depends on the way that you are going to use those strings within the code. Here is just one way of checking (use more than this):
$okChars = '][()_A-Za-z0-9-';
$hackMsg = "The only characters which my program will accept are: $okChars";
if( preg_match( "/[^$okChars]/", $INTERNAL_STRING)) die( $hackMsg );
I am trying to pass an attitude across here, rather than give specifics on what to do. I am trying to save you future grief. For the moment, ignore everything that I have written about security! Get your program working first, then return to this topic.
Do you think it is the header location that is the problem?
Understand - I am trying to help, not just criticise. You need to try to isolate where the problem may be so that others can help.
There is one obvious problem(s) with the code:
elseif ($note==thankyouverymuch)"thankyouverymuch" needs to be either quoted or a constant:
elseif ($note=='thankyouverymuch')
What that means is that you are using the default error-reporting (which does not include NOTICEs). Turn full error-reporting on. If you do not have access to php.ini it can be done in code with:
error_reporting( E_ALL );
define( "E_NONE", 0 );
error_reporting( E_NONE );
Fix all the errors and try again. If it does not work, and you cannot find the error, then isolate the section which has the issue and post it and the error message again.
Good luck.
The problem is that with this code - aside from securty - is that it does not remember the fields that were filled in correctly. I think it might be to do with the part:
elseif ($fn == "") {
header("Location: contact.php?error=firstname&errornote=TRUE");
}
Perhaps the header("Location: contact.php? part could be $PHP_SELF?.
errornote=TRUE will work, but errornote=1 is better if you do the boolean test at the top as shown. You would then follow up with a switch statement:
if( $ERRORNOTE ) {
switch( $ERROR ) {
case 'firstname':
...
}
}
Even better to define() some numeric constants for each section, set error=CONSTANT, and switch on the constants.
PS Get into the habit of using the HTML entity instead of the ampersand (will not validate otherwise):
header( 'Location: [mydomain.com...] );
Something like this:
if (!$new_string == $random){
$error=verification;
$errornote=1);
}
elseif ($fn == "") {
$error['fn']="<p class=\"error\">There was an error with your first name.</p>";
$errornote=1;
}
elseif (($ea == "") ¦¦ (!eregi("^[a-zA-Z0-9_]+@[a-zA-Z0-9\-]+\.[a-zA-Z0-9\-\.]+$", $ea))) {
$error['em']="<p class=\"error\">There was an error with your email address.</p>";
$errornote=1;
}
elseif ($qu == "") {
$error['q']="<p class=\"error\">There was an error with your question.</p>";
$errornote=1;
}
else {
mail ("email@address.com", "$fo", "$qu from $fn $ln", "from: $ea");
header("Location: contact.php?note=thankyouverymuch");
}
Then in the form section:
// show the errors:
foreach($error as $fix) {
echo $fix;
}
// set the error style only once
$error_style="style=\"border: 1px solid red;\"";
echo "<div id=\"contactform\">";
echo "<form action=\"contact.php\" method=post>";
echo "<label for=\"for\">For: *</label>";
echo "<select name=\"for\">";
echo "<option>Business";
echo "<option>Help";
echo "<option>Personal";
echo "</select><br />";
echo "<label for=\"firstname\">First name: *</label>";
echo "<input type=\"text\" name=\"firstname\"";
if(isset($error['fn'])) { echo $error_style; }
echo ">";
echo "<input type=\"text\" name=\"emailaddress\"";
if(isset($error['em'])) { echo $error_style; }
echo ">";
// and so on
Anyway, just thought I would throw something out there, hope this makes a little sense... maybe I missed the reasoning behind it?
Justin
It says it is an invalid foreach loop. Sorry to throw a whole load of code into this post but here is the contact form as it stands. I still don't get how to sanitise, sorry.
<?php
session_start();
if (isset($_POST[submit])) {
$fo = $_POST['for'];
$fn = $_POST['firstname'];
$ln = $_POST['lastname'];
$ea = $_POST['emailaddress'];
$qu = $_POST['question'];
$random = trim($random);
if (!$new_string == $random){
$error=verification;
$errornote=1;
}
elseif ($fn == "") {
$error['fn']="<p class=\"error\">There was an error with your first name.</p>";
$errornote=1;
}
elseif (($ea == "") ¦¦ (!eregi("^[a-zA-Z0-9_]+@[a-zA-Z0-9\-]+\.[a-zA-Z0-9\-\.]+$", $ea))) {
$error['em']="<p class=\"error\">There was an error with your email address.</p>";
$errornote=1;
}
elseif ($qu == "") {
$error['q']="<p class=\"error\">There was an error with your question.</p>";
$errornote=1;
}
else {
mail ("emailasdasd@asdf.com", "$fo", "$qu from $fn $ln", "from: $ea");
header("Location: contact.php?note=thankyouverymuch");
}
// show the errors:
foreach($error as $fix) {
echo $fix;
}
// set the error style only once
$error_style="style=\"border: 1px solid red;\"";
}
elseif ($note==thankyouverymuch)
{
$page="contact"; require ("inc/top.inc.php");
echo "<table cellpadding=\"0\" cellspacing=\"0\" width=\"100%\" class=\"content\">";
echo "<tr>";
echo "<td valign=\"top\">";
echo "<h1>Thank you!</h1>";
echo "<br />";
echo "<h3>We will be in contact with you as soon as possible<br /><br /><a href=\"$path/index.php\">Home</a></h3>";
echo "</td>";
echo "</tr> ";
echo "</table>";
require ("inc/footer.inc.php");
}
else {
$page="contact"; require ("inc/top.inc.php");
$new_string;
session_register('new_string');
$im = ImageCreate(200, 40);
$white = ImageColorAllocate($im, 255, 255, 255);
$black = ImageColorAllocate($im, 0, 0, 0);
srand((double)microtime()*1000000);
$string = md5(rand(0,9999));
$new_string = substr($string, 17, 5);
ImageFill($im, 0, 0, $white);
ImageString($im, 4, 45, 19, $new_string, $black);
/* Directory inc/c/ is CHMODDed to 777 */
ImagePNG($im, "inc/c/verify.png");
ImageDestroy($im);
echo "<table cellpadding=\"0\" cellspacing=\"0\" class=\"content\">";
echo "<tr>";
echo "<td valign=\"top\">";
echo "<h1>Contact us</h1>";
echo "<br />All fields maked * are required";
if ($errornote=='TRUE') {
echo "<br /><br />Please fill in all fields marked <span style=\"color: red;\">RED</span>";
}
echo "<div id=\"contactform\">";
echo "<form action=\"contact.php\" method=post>";
echo "<label for=\"for\">For: *</label>";
echo "<select name=\"for\">";
echo "<option>Business";
echo "<option>Help";
echo "<option>Personal";
echo "</select><br />";
echo "<label for=\"firstname\">First name: *</label>";
echo "<input type=\"text\" name=\"firstname\"";
if(isset($error['fn'])) { echo $error_style; }
echo ">";
echo "<input type=\"text\" name=\"emailaddress\"";
if(isset($error['em'])) { echo $error_style; }
echo ">";
echo "<label for=\"question\">";
if ($error==question) {
echo "<span style=\"color: red;\">Question *</span></label>";
}
else {
echo "Question *</label>";
}
echo "<textarea name=\"question\" rows=\"10\" cols=\"10\"></textarea><br />";
echo "<img src=\"inc/c/verify.png\">";
echo "<br />";
echo "Please type the contents of the image *";
echo "<br /><br />";
echo "<input name=\"random\" type=\"text\" value=\"\"";
if ($error==verification) {
echo "style=\"border: 1px solid red;\">";
}
else {
echo ">";
}
echo "<br /><br />";
echo "<input type=\"submit\" name='submit' value=\"Send\">";
echo "</form>";
echo "</div> ";
echo "</td> ";
echo "</tr> ";
echo "</table> ";
echo "<br /><h3 class=\"content\"><b>We respect your privacy and we will never sell your email address to any third party. The information submitted in this form will be sent directly to person, managing director of Eyelights Ltd.</b></h3>";
require ("inc/footer.inc.php");
}
?>
