Form Mail CGI

[andrewmarshall]

Hi All,

I am after some advice if anyone is willing and able to assist.

I have an online form on my website with uses a CGI script that I wrote some time ago! It works very well to validate and send the information submitted but it still send an email even when it tells the user that the information is wrong.

The script is below, I would be grateful if anyone could help to point out my shortfalls

:thumbsup:


Many thanks

Andrew



#!/usr/bin/perl

use CGI;

# Create the CGI object
my $query = new CGI;

# Output the HTTP header
print $query->header ( );

# Capture the form results
my $name = $query->param("name");
my $jobtitle = $query->param("jobtitle");
my $company = $query->param("company");
my $email = $query->param("email");
my $phonenumber = $query->param("phonenumber");
my $faxnumber = $query->param("faxnumber");
my $address = $query->param("address");
my $enquiry = $query->param("enquiry");

#Validation
$validationOK=true;
if ($name eq '') {$validationOK=false;}
if ($company eq '') {$validationOK=false;}
if ($email eq '') {$validationOK=false;}
if ($phonenumber eq '') {$validationOK=false;}
if ($validationOK eq false)
{print "<meta http-equiv=\"refresh\" content=\"0;URL=/onlineerror.html\">";}

# Filter the form results
$name = filter_field ( $name );
$jobtitle = filter_field ( $jobtitle );
$company = filter_field ( $company );
$email = filter_header_field ( $email );
$phonenumber = filter_field ( $phonenumber );
$faxnumber = filter_field ( $faxnumber );
$address = filter_field ( $address );
$enquiry = filter_field ( $enquiry );

# Email the form results
open ( MAIL, "| /usr/lib/sendmail -t" );
print MAIL "From: www.mywebsite.com\n";
print MAIL "To: mymailaddress\n";
print MAIL "Subject: Form Submission\n\n";
print MAIL "Name: $name\n";
print MAIL "Job Title: $jobtitle\n";
print MAIL "Company: $company\n";
print MAIL "Email Address: $email\n";
print MAIL "Phone Number: $phonenumber\n";
print MAIL "Fax Number: $faxnumber\n";
print MAIL "Address: $address\n";
print MAIL "Enquiry: $enquiry\n";
print MAIL "\n.\n";
close ( MAIL );

# Thank the user
print "<meta http-equiv=\"refresh\" content=\"0;URL=/thankyou.html\">";

# Functions for filtering user input
sub filter_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
return $field;
}

sub filter_header_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
$field =~ s/[\0\n\r\|\!\/\<\>\^\$\%\*\&]+/ /g;
return $field;
}

[oesxyl]

Hi All,

I am after some advice if anyone is willing and able to assist.

I have an online form on my website with uses a CGI script that I wrote some time ago! It works very well to validate and send the information submitted but it still send an email even when it tells the user that the information is wrong.

The script is below, I would be grateful if anyone could help to point out my shortfalls

:thumbsup:


Many thanks

Andrew



#!/usr/bin/perl

use CGI;

# Create the CGI object
my $query = new CGI;

# Output the HTTP header
print $query->header ( );

# Capture the form results
my $name = $query->param("name");
my $jobtitle = $query->param("jobtitle");
my $company = $query->param("company");
my $email = $query->param("email");
my $phonenumber = $query->param("phonenumber");
my $faxnumber = $query->param("faxnumber");
my $address = $query->param("address");
my $enquiry = $query->param("enquiry");

#Validation
$validationOK=true;
if ($name eq '') {$validationOK=false;}
if ($company eq '') {$validationOK=false;}
if ($email eq '') {$validationOK=false;}
if ($phonenumber eq '') {$validationOK=false;}
if ($validationOK eq false)
{print "<meta http-equiv=\"refresh\" content=\"0;URL=/onlineerror.html\">";}

# Filter the form results
$name = filter_field ( $name );
$jobtitle = filter_field ( $jobtitle );
$company = filter_field ( $company );
$email = filter_header_field ( $email );
$phonenumber = filter_field ( $phonenumber );
$faxnumber = filter_field ( $faxnumber );
$address = filter_field ( $address );
$enquiry = filter_field ( $enquiry );

# Email the form results
open ( MAIL, "| /usr/lib/sendmail -t" );
print MAIL "From: www.mywebsite.com\n";
print MAIL "To: mymailaddress\n";
print MAIL "Subject: Form Submission\n\n";
print MAIL "Name: $name\n";
print MAIL "Job Title: $jobtitle\n";
print MAIL "Company: $company\n";
print MAIL "Email Address: $email\n";
print MAIL "Phone Number: $phonenumber\n";
print MAIL "Fax Number: $faxnumber\n";
print MAIL "Address: $address\n";
print MAIL "Enquiry: $enquiry\n";
print MAIL "\n.\n";
close ( MAIL );

# Thank the user
print "<meta http-equiv=\"refresh\" content=\"0;URL=/thankyou.html\">";

# Functions for filtering user input
sub filter_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
return $field;
}

sub filter_header_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
$field =~ s/[\0\n\r\|\!\/\<\>\^\$\%\*\&]+/ /g;
return $field;
}

you can put your code between [ code] and [ /code] tags and you can edit previous post to do that, is more easy to read. Thank you.


#!/usr/bin/perl

use CGI;

# Create the CGI object
my $query = new CGI;

# Output the HTTP header
print $query->header ( );

# Capture the form results
my $name = $query->param("name");
my $jobtitle = $query->param("jobtitle");
my $company = $query->param("company");
my $email = $query->param("email");
my $phonenumber = $query->param("phonenumber");
my $faxnumber = $query->param("faxnumber");
my $address = $query->param("address");
my $enquiry = $query->param("enquiry");

#Validation
$validationOK=true;
if ($name eq '') {$validationOK=false;}
if ($company eq '') {$validationOK=false;}
if ($email eq '') {$validationOK=false;}
if ($phonenumber eq '') {$validationOK=false;}
if ($validationOK eq false){
print "<meta http-equiv=\"refresh\" content=\"0;URL=/onlineerror.html\">";
exit; # stop here
}

# Filter the form results
$name = filter_field ( $name );
$jobtitle = filter_field ( $jobtitle );
$company = filter_field ( $company );
$email = filter_header_field ( $email );
$phonenumber = filter_field ( $phonenumber );
$faxnumber = filter_field ( $faxnumber );
$address = filter_field ( $address );
$enquiry = filter_field ( $enquiry );

# Email the form results
open ( MAIL, "| /usr/lib/sendmail -t" );
print MAIL "From: www.mywebsite.com\n";
print MAIL "To: mymailaddress\n";
print MAIL "Subject: Form Submission\n\n";
print MAIL "Name: $name\n";
print MAIL "Job Title: $jobtitle\n";
print MAIL "Company: $company\n";
print MAIL "Email Address: $email\n";
print MAIL "Phone Number: $phonenumber\n";
print MAIL "Fax Number: $faxnumber\n";
print MAIL "Address: $address\n";
print MAIL "Enquiry: $enquiry\n";
print MAIL "\n.\n";
close ( MAIL );

# Thank the user
print "<meta http-equiv=\"refresh\" content=\"0;URL=/thankyou.html\">";
exit; # not really need this because have only sub after this point

# Functions for filtering user input
sub filter_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
return $field;
}

sub filter_header_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
$field =~ s/[\0\n\r\|\!\/\<\>\^\$\%\*\&]+/ /g;
return $field;
}

or you can put the part with mail in a sub and call the sub only if validateOk is true.

I don't test and don't look closer to you script, I guess it work, post results/problems if you need
regards

[FishMonger]

The only validation that you're doing is verifying that the submitted fields are not empty. Other than that, your "validation" doesn't do anything, which is why it sends an email even if the form submission contains useless data.

What is the purpose of your filter_header_field() sub? As far as I can see, as long as you set up your form correctly, there is no reason to have that sub.

[FishMonger]

I was sort on time when I posted my first response, so here's a more detailed response.

Hi All,

The script is below, I would be grateful if anyone could help to point out my shortfalls


#!/usr/bin/perl

use CGI;

# Create the CGI object
my $query = new CGI;

# Output the HTTP header
print $query->header ( );

Good so far, but you left out 2 very important pragmas that should be in every Perl script you write.
use strict;
use warnings;

# Capture the form results
my $name = $query->param("name");
my $jobtitle = $query->param("jobtitle");
my $company = $query->param("company");
my $email = $query->param("email");
my $phonenumber = $query->param("phonenumber");
my $faxnumber = $query->param("faxnumber");
my $address = $query->param("address");
my $enquiry = $query->param("enquiry");

Pulling in individual vars like that is ok, but my personal preference is to import them as a hash.

my %fields = $query->Vars;


#Validation
$validationOK=true;
if ($name eq '') {$validationOK=false;}
if ($company eq '') {$validationOK=false;}
if ($email eq '') {$validationOK=false;}
if ($phonenumber eq '') {$validationOK=false;}
if ($validationOK eq false)
{print "<meta http-equiv=\"refresh\" content=\"0;URL=/onlineerror.html\">";}

A cleaner method would be:

foreach my $form_field ($name, $company, $email, $phonenumber) {
if ( $form_field eq '' ) {
print $query->redirect("http://mysite.com/onlineerror.html");
exit;
}
}


# Filter the form results
$name = filter_field ( $name );
$jobtitle = filter_field ( $jobtitle );
$company = filter_field ( $company );
$email = filter_header_field ( $email );
$phonenumber = filter_field ( $phonenumber );
$faxnumber = filter_field ( $faxnumber );
$address = filter_field ( $address );
$enquiry = filter_field ( $enquiry );

This could be cleaned up with a foreach loop similar to the previous one I showed, and if you use a hash as I prefer, you could do this:

foreach my $form_field ( keys %fields ) {
filter_field($form_field);
}


# Email the form results
open ( MAIL, "| /usr/lib/sendmail -t" );

You need to check the return code of the open call to verify that it succeeded, and take action if it didn't. Also, since perl 5.6 (which has been about 10 years) it's preferable to use a lexical var for the filehandle.

open ( my $MAIL, "| /usr/lib/sendmail -t" ) or die "can't pipe to sendmail $!";


print MAIL "From: www.mywebsite.com\n";
print MAIL "To: mymailaddress\n";
print MAIL "Subject: Form Submission\n\n";
print MAIL "Name: $name\n";
print MAIL "Job Title: $jobtitle\n";
print MAIL "Company: $company\n";
print MAIL "Email Address: $email\n";
print MAIL "Phone Number: $phonenumber\n";
print MAIL "Fax Number: $faxnumber\n";
print MAIL "Address: $address\n";
print MAIL "Enquiry: $enquiry\n";
print MAIL "\n.\n";
close ( MAIL );

It would be cleaner to use a here document and a single print statement.

print $EMAIL <<"EMAIL";
From: www.mywebsite.com
To: mymailaddress
Subject: Form Submission

Name: $name
Job Title: $jobtitle
Company: $company
Email Address: $email
Phone Number: $phonenumber
Fax Number: $faxnumber
Address: $address
Enquiry: $enquiry

EMAIL

However, my personal preference is to use the MIME::Lite module
http://search.cpan.org/~rjbs/MIME-Lite-3.021/lib/MIME/Lite.pm


# Thank the user
print "<meta http-equiv=\"refresh\" content=\"0;URL=/thankyou.html\">";


print $query->redirect("http://mysite.com/thankyou.html");


# Functions for filtering user input
sub filter_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
return $field;
}

sub filter_header_field
{
my $field = shift;
$field =~ s/From://gi;
$field =~ s/To://gi;
$field =~ s/BCC://gi;
$field =~ s/CC://gi;
$field =~ s/Subject://gi;
$field =~ s/Content-Type://gi;
$field =~ s/[\0\n\r\|\!\/\<\>\^\$\%\*\&]+/ /g;
return $field;
}
These 2 subs are identical with the exception of the final regex in filter_header_field(). There's no reason to have that duplication. As far as I can tell, the only purpose of those subs is to strip out hard coded form field data that should not have been there in the first place. Fix your form and remove those subs.

A much better solution would be to replace your script with the well written and tested nms FormMail script.
http://nms-cgi.sourceforge.net/scripts.shtml

[andrewmarshall]

Thankyou to you both, Oesxyl that worked perfectly for a quick fix! and thanks FishMonger for the time spent, I will endevour to use the information you kindly provided to fix and improve my own basic understanding of CGI

Andrew