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>

Open in new window

LVL 1
catonthecouchproductionsAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Marco GasiFreelancerCommented:
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
Dave BaldwinFixer of ProblemsCommented:
Try this.  I initialized the variables and put the functions in the code before they are called.  I left redisplayForm() where it is so you could see that putting in that position as a function does not make it display there.  It displays where it is called which up above the title.

Then I checked to see if there was form data POSTed and skipped all that code if it wasn't.
<!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
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);
}

// initialize variables
$FName = "";
$LName = "";
$Email = "";

// check for POST data
if(isset($_POST['submit'])) {
$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);
}

$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 = "";
}
}
}

}

?>
<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>

Open in new window

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Hugh McCurdyCommented:
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.  
Build an E-Commerce Site with Angular 5

Learn how to build an E-Commerce site with Angular 5, a JavaScript framework used by developers to build web, desktop, and mobile applications.

Lukasz ChmielewskiCommented:
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 PaseurCommented:
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 McCurdyCommented:
Thanks Ray & Roads.  Useful advice.  I very much agree with not liking loose typing.  

Ray PaseurCommented:
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 McCurdyCommented:
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/
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
PHP

From novice to tech pro — start learning today.