asked on 10/2/2011

PHP Form submitting blank entry

On this page http://bit.ly/oOsKAQ I am trying to have the guest info save into a text file on submit.

Couple issues

1) Why are my errors displaying at the top of the page even before I submit anything?
2) When I submit info it keeps adding a blank entry at the bottom?

My code is attached. Thoughts?

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">

<head>

<title>Guest Book Sign-in</title>
<meta http-equiv="content-type" content="text/html; charset=iso-8859-1" />
<style type="text/css">
h3
{
color: #C30;
}

input
{
color: #C30;
background-color: #FC9;
}
</style>
</head>

<body>
<?php
$FName = validateInput($_POST['FName'], "First name");
$LName = validateInput($_POST['LName'], "Last name");
$Email = validateInput($_POST['email'], "email");

if ($errorCount>0) {
	echo "Please re-enter the information below.<br />\n";
	redisplayForm($FName, $LName, $Email);
}

function displayRequired($fieldName) {
	echo "The field \"$fieldName\" is required.<br />\n";
}
function validateInput($data, $fieldName) {
	global $errorCount;
	if (empty($data)) {
		displayRequired($fieldName);
		++$errorCount;
		$retval = "";
	}
	else { //Only clean up the input if it isn't empty
	$retval = trim($data);
	$retval = stripslashes($retval);
	}
	return($retval);
}
$errorCount = 0;

if (isset($_POST['submit'])) {
$FName = stripslashes($_POST['FName']);
$LName = stripslashes($_POST['LName']);
$Email = stripslashes($_POST['email']);
// Replace any '~' characters
// with '-' characters
$FName = str_replace("~", "-", $FName);
$LName = str_replace("~", "-", $LName);
$Email = str_replace("~", "-", $Email);

$ExistingGuests = array();
	if(file_exists("GuestBook/guests.txt") &&
	filesize("GuestBook/guests.txt") > 0){
	$GuestArray = file("GuestBook/guests.txt");
	$count = count($GuestArray);
	for ($i = 0; $i < $count; ++$i) {
		$CurrGst = explode("~", $GuestArray[$i]);
		$ExistingGuests[] = $CurrGst[0];
		}
		}
		if (in_array($FName, $ExistingGuests)) {
		echo "<p>The first name you entered already exixts!<br />\n";
		echo "Your messge was not saved.</p>";
		$FName = "";
		}
		else {

$GuestRecord = "$FName~$LName~$Email\n\r";
$GuestFile = fopen("GuestBook/guests.txt", "ab");
if ($GuestFile === FALSE)
echo "There was an error saving your information!\n\r";
else
{
fwrite($GuestFile, $GuestRecord);
fclose($GuestFile);
echo "Your information has been saved.\n\r";
$FName = "";
$LName = "";
}
}
}
else {
$FName = "";
$LName = "";
$Email = "";
}

?>
<h2 style="text-align:center">Guest Book Registry</h2>
<?php

function redisplayForm($firstName, $lastName) {
	?>

    <?php
}

?>

<h3>Post New Guest</h3>

<hr />

<form action="GuestBookSignIn.php" method="POST">
<span style="font-weight: bold">First Name:</span>
<input type="text" name="FName"
value="<?php echo $FName; ?>" /><br />

<span style="font-weight: bold">Last Name:</span>
<input type="text" name="LName"
value="<?php echo $LName; ?>" /><br />

<span style="font-weight: bold">Email:</span>
<input type="text" name="email"
value="<?php echo $Email; ?>" /><br />


<input type="submit" name="submit" value="Post Message" />
<input type="reset" name="reset" value="Reset Form" />
</form>
<hr />
<p>
<a href="GuestBookPostings.php">View Guests</a>
</p>

</body>

</html>

Last Comment

10/3/2011

10/2/2011

For the first question simply put these lines

$FName = validateInput($_POST['FName'], "First name");
$LName = validateInput($_POST['LName'], "Last name");
$Email = validateInput($_POST['email'], "email");

after the

if (isset($_POST['submit'])) {
$FName = stripslashes($_POST['FName']);
$LName = stripslashes($_POST['LName']);
$Email = stripslashes($_POST['email']);
// Replace any '~' characters
// with '-' characters
$FName = str_replace("~", "-", $FName);
$LName = str_replace("~", "-", $LName);
$Email = str_replace("~", "-", $Email);

For the second question I have no time to test to test it now, but I come back later :-)

Cheers

ASKER CERTIFIED SOLUTION

Dave Baldwin

10/2/2011

THIS SOLUTION IS ONLY AVAILABLE TO MEMBERS.

View this solution by signing up for a free trial.

Members can start a 7-Day free trial and enjoy unlimited access to the platform.

See Pricing Options

Start Free Trial

Hugh McCurdy

10/2/2011

Based on reading the other posts, I'm just going to make a supporting post. (No points please.)

It is very important in programming to explicitly initialize your variables unless you are sure it isn't necessary. It's always a good idea in PHP.

Lukasz Chmielewski

10/2/2011

You should take a closer look into this command:
http://php.net/manual/pl/function.error-reporting.php

Not initializing a viariable is not really an error it it just a notice. This could be handled with adding the
error_reporting(E_ERROR);
at the top of your script;

Ray Paseur

10/3/2011

Agree with hmccurdy and Roads_Roads, 100%. PHP allows you to use undefined variables and (WTF?!) it will accept silent "default" definitions of the variables. From a computer science perspective this is a horrible design. Most languages used by professional programmers prohibit this sort of crap. What PHP does causes hundreds of thousands, if not millions, of programming errors every year. But because PHP let it happen in the beginning, there is no way out of the predicament now, except for education. Hopefully the responses to your question will give you some of that education.

Why is this silent confusion bad? Well, consider these two variables: $CatonTheCouchProductions and $CatontheCouchProductions. Easy to confuse. These are not the same variable. There are similar stupid things about the names of array keys. This is called "loose data typing" and it is something that you must live with if you will be a PHP programmer. It was an effort to make the language "easy" for everyone. In fact it made the language annoying to computer scientists and confounding to novice programmers.

You will see in all of my programming examples that error_reporting(E_ALL) is at the top of my scripts. If you're smart you will do that, too.

Hugh McCurdy

10/3/2011

Thanks Ray & Roads. Useful advice. I very much agree with not liking loose typing.

Ray Paseur

10/3/2011

I was looking over your script and I'd like to suggest a few design changes. You can refer to the code snippet for straightforward code examples of what I'm talking about.

When you package a form and action script together, you would always want to put the PHP code that runs the action script first. The form, even though it is logically first from the client's perspective, is not written first in the programming. It is written last.

The first thing your PHP script should do (after error_reporting(E_ALL);) is initialize any variables that might be needed. See line 13 where we set up the initial values for these variables that control the display. See line 21 where we capture and normalize external input, if any, setting default values if the input is missing.

After initialization, we test for the presence of true external input by inspecting the $_POST array (line 25). If there is no external input, we skip all of this stuff and just put up the form for the client.

If there is true external input, we examine the captured and normalized external input with coding that applies the standard, accept only known good values. In this teaching sample, the requirement is for the client to enter ABC and XYZ so we test for those values in the appropriate fields. This is called "validation" of external input. Because we initialized these fields (line 21) we have either the external strings from the client request or the NULL string that we set as a default value.

If either of the external variables fails the validation (line 28 and line 37) we set the display variables to show that there was an error and to highlight the error with a red input box.

Once all of the validations are complete we will have a certain indicator of error or success in the $error_any variable. If that signals that we have no errors, we can do the data base updates, or write the text files, or do whatever the client form was expected to cause. In this teaching example, there is nothing to update and the entire action script is contained on lines 49-50.

If this is the first load of the script, if nothing was posted, or if there were errors in a previous submission we will need to show the HTML form. See line 56, et seq, where we use HEREDOC notation to create the form. You may find that HEREDOC notation is much easier to use than trying to get all the quote marks escaped correctly in echo statements. More on HEREDOC:
http://php.net/manual/en/language.types.string.php

Hope this helps shed some light on the overall design of a form-and-action script. Best regards, ~Ray

<?php // RAY_form_highlight_errors.php
error_reporting(E_ALL);


// DEMONSTRATE HOW TO HIGHLIGHT ERRORS IN FORM INPUT
// CLIENT IS ASKED TO PUT IN A VALUE
// IF THE VALUE FAILS OUR TEST WE SHOW AN ERROR MESSAGE
// WE PUT A MARKER NEXT TO THE INPUT CONTROL ON THE FORM
// WE TURN THE FORM BORDER RED
// SEE http://www.w3schools.com/CSS/pr_class_visibility.asp


// THESE CONDITIONS ARE SET FOR THE INITIAL ENTRY
$error_abc = 'hidden';
$boxer_abc = 'black';
$error_xyz = 'hidden';
$boxer_xyz = 'black';
$error_any = 'hidden';


// CAPTURE AND NORMALIZE THE POST VARIABLES - ADD YOUR OWN SANITY CHECKS HERE
$abc = (isset($_POST["abc"])) ? trim(strtoupper($_POST["abc"])) : NULL;
$xyz = (isset($_POST["xyz"])) ? trim(strtoupper($_POST["xyz"])) : NULL;

// IF ANYTHING WAS POSTED, VALIDATE IT
if (!empty($_POST))
{
    // VALIDATE THE 'abc' FIELD
    if ($abc != 'ABC')
    {
        $error_any = 'visible';
        $error_abc = 'visible';
        $boxer_abc = 'red';
        // $abc       = NULL;
    }

    // VALIDATE THE 'xyz' FIELD
    if ($xyz != 'XYZ')
    {
        $error_any = 'visible';
        $error_xyz = 'visible';
        $boxer_xyz = 'red';
        // $xyz       = NULL;
    }

    // DO WE HAVE INPUT FREE FROM ANY ERRORS?
    if ($error_any != 'visible')
    {
        echo "CONGRATULATIONS";
        die();
    }

    // OOPS - WE HAVE ERRORS
}

// IF NOTHING WAS POSTED, OR IF THERE ARE ERRORS, WE NEED NEW CLIENT INPUT
$form = <<<ENDFORM
<style type="text/css" media="all">
.error_any { visibility:$error_any; }
.error_abc { visibility:$error_abc; }
.error_xyz { visibility:$error_xyz; }
</style>
<pre>
<form method="post">
<span class="error_any">PLEASE CORRECT THE FOLLOWING ERRORS</span>
<span class="error_abc">YOU MUST ENTER 'abc' IN THIS FIELD</span>
PLEASE ENTER "ABC" HERE: <input style="border-color:$boxer_abc;" name="abc" value="$abc" />
<span class="error_xyz">YOU MUST ENTER 'xyz' IN THIS FIELD</span>
PLEASE ENTER "XYZ" HERE: <input style="border-color:$boxer_xyz;" name="xyz" value="$xyz" />
<input type="submit" />
</form>
ENDFORM;

// WRITE THE FORM WITH THE APPROPRIATE CSS STYLES ON THE ERROR MESSAGE FIELDS
echo $form;

Open in new window

Hugh McCurdy

10/3/2011

Ray makes a great point about sanitizing data originating from outside the program "accept only known good values." This includes both data typed in by the user and data coming from your database.

Here's a cartoon that illustrates a problem -- http://xkcd.com/327/