Allowing the To: address to be passed from the form makes this an easy target for spammers to abuse and send mail to anyone they like.

Please   DO NOT   ever actually use this script.

It is also severly broken in several other ways. You should definitely read Ovid's excellent CGI course to understand how. For just getting your job done, you should use the formmail script from the NMS project's collection.

Makeshifts last the longest.

You are working from a bad model.

use CGI; to parse cgi data.

You should look at the nms version of formmail.pl.

After Compline,
Zaxo

$ REQUEST_METHOD=GET QUERY_STRING='bob=jack&jane=janice' ./sendit.pl 
$ REQUEST_METHOD=POST CONTENT_LENGTH=20 ./sendit.pl <<EOF
bob=jack&jane=janice
EOF
[download]

Undefined subroutine &main::LMAIL called at ./test.pl line 23.
[download]

22:  open (LMAIL, "|usr/sbin/sendmail -t");
23:  print LMAIL("To: $in{myEmail}\n");
24:  print LMAIL ("From: $in{Email}\n");
[download]

Incidently, those error messages will also appear in your webservers error logs.

Err, yes, I was going to say something about that and forgot: any time you're working on a cgi script, you probably want to have a window tailing the server's error log. If you don't have access to the error logs on the server where this is ultimately going to run, then do your testing on another system first, where you do have access. I run Apache on my home PC and also on my workstation at work, not because they're servers, but so that I can test stuff before putting it on the server. If you're going to be doing cgi programming, it's well worth having Apache on your computer, for testing. Then you can watch the log as you make a change to your script, save it, switch back over to the browser, and hit reload.

---

sub H{$_=shift;while($_){$c=0;while(s/^2//){$c++;}s/^4//;$
v.=(' ','|','_',"\n",'\\','/')[$c]}$v}sub A{$_=shift;while
($_){$d=hex chop;for(1..4){$pl.=($d%2)?4:2;$d>>=1}}$pl}$H=
"16f6da116f6db14b4b0906c4f324";print H(A($H)) # -- jonadab
[download]

#!/bin/perl

if ($ENV{'REQUEST_METHOD'} eq "GET") {$buffer = $ENV{'QUERY_STRING'};}
elsif ($ENV{'REQUENT_METHOD'} eq "POST")
    {read(STDIN,$buffer,$ENV{'CONTENT_LENGTH'});}

$bufferb = $buffer;
@forminputs = split(/&/, $bufferb);

foreach $forminput (@forminputs)
{
    ($name, $value) = split(/=/, $forminput);

    $value =~ tr/+/ /;
    $value =~ s/%([a-fA-f0-9][a-fA-F0-9])/pack("C", hex($1))/eg;

    $in{$name} = $value;
}
[download]

Let's start with that much. First of all, getting form input is something 80% or more of the cgi scripts you ever write will need to do. Rather than inlining it, make it a subroutine. That also improves the legibility of your code, because it's very clear what getforminput() does. Also, you can put your subroutine in a my-cgi-includes.pl file and require it from all your various scripts, so that as you discover problems with it, you can fix it in one place and all your scripts can benefit. My advice is to write it so that it returns a reference to a hash of name/value pairs. Then your code can do this:

require "my-cgi-includes.pl";
my %input = %{getforminput()};
[download]

Someone will quickly jump up and say that you should use cgi instead, but I say that the instead part is wrong: using cgi is an implementation detail of your subroutine. The key principle that applies generally is to move that oft-repeated code off by itself so that you can easily fix it up in whatever way (e.g., by deciding to use cgi) when you figure out how or get around to it. For starters, and for learning purposes, let's take what you've got and work with it:

sub getforminput {
  my %in;
  my $buffer; # Because this is now a subroutine that can get
              # called from various places, we must be careful
              # not to tromp on the script's own variables.
  if ($ENV{'REQUEST_METHOD'} eq "GET") {$buffer = $ENV{'QUERY_STRING'}
+;}
  elsif ($ENV{'REQUENT_METHOD'} eq "POST") {
    read(STDIN,$buffer,$ENV{'CONTENT_LENGTH'});
  } else {
    return undef; # If $ENV{REQUEST_METHOD} is not set, maybe there's 
+no input.
                  # In that case the script will probably print a blan
+k form.
  }
  # I'm not sure what your assigning from $buffer to $bufferb was
  # intended to accomplish, but I think it just creates an extra
  # variable.  Skip it, and go straight to the split:
  my @forminputs = split(/&/, $buffer);
  foreach $forminput (@forminputs)
    {
      my ($name, $value) = split(/=/, $forminput);
      $value =~ tr/+/ /;
      $value =~ s/%([a-fA-f0-9][a-fA-F0-9])/pack("C", hex($1))/eg;
      # Some of that stuff cgi could do for you, possibly better, but
      # for now we'll move on...

      $in{$name} = $value;
      # Good.  You did right to put the input in a hash.  Not
      # only is it convenient to access that way, but it is
      # also all together in one nice container that can be
      # easily passed around, and there is no risk of tromping
      # on other variables (as there would be with $$name=$value
      # and similar hacks).
    }
  return \%in;
}
[download]

I only made minor changes, but now what you have is not only more re-usable, but if you discover a problem with it you can fix it up without much risk to the rest of the script(s).

Let's move on...

print "Content-type: text/html\n\n";

#mail sent to admin
open (LMAIL, "|usr/sbin/sendmail -t");
print LMAIL("To: $in{myEmail}\n");
print LMAIL ("From: $in{Email}\n");
print LMAIL ("Subject: $in{Form}\n");

print LMAIL ("--------------------\n\n");
print LMAIL("$in{Message}\n\n");

#mail sent to user
open (MAIL, "|/usr/sbin/sendmail -t");
print MAIL<<toEnd;
[download]

mmmmkay... First you print out your headers, then you try to send the mail. If it were me, I would be doing it the other way, sending the mail first and then printing the page that tells the user what we did. But that is a minor thing.

More important, as the other poster pointed out, we need to do some error checking before we send the message...

   my %input=%{getforminput()};
   my $validto = "jonadab\@bright.net,jonadab_homeip_net\@yahoo.co.uk"
+;
   if ($validto !~ $input{myEmail}) {
      $errors .= "<li>The To: address you selected was not one of the 
+available choices.  Please pick an option from the dropdown list.</li
+>\n"; }
   if ($input{Email} !~ /^[^@]+[@][^@]+$/) {
      $errors .= "<li>The From: address you entered does not appear to
+ be valid.  Please enter a real email address.</li>\n"; }
   if (not $input{Subject}) {
      $errors .= "<li>You left the subject blank.  Please fill in a sh
+ort description of what your message is about.</li>\n"; }
   if (not $input{Message}) {
      $errors .= "<li>You left the message itself blank.  Surely you h
+ad something to say?</li>\n"; }
   if ($errors) {
      print "Content-type:  text/html

<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\" \"htt
+p://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">
<html>\n<head>\n<title>Error(s) encountered</title>\n</head>
<body>\n<p>One or more errors were encountered when trying to send you
+r message.
      Please hit your browser's back button and correct the following 
+issues:\n</p>
<ul>$errors</ul>\n</body>\n</html>\n";
   exit 0; # Update:  If there were errors, we don't want to send the 
+message.
   }
[download]

If you might write multiple scripts that send mail, you should make that validation into a subroutine that you can include. Also, the actual mail sending code can be a subroutine that takes the from, to, subject, and body as four arguments... If it were me I'd be using Net::SMTP, but for now let's take what you have and make it a subroutine...

sub sendmail {
  my ($to, $from, $subject, $body) = @_;
  open (LMAIL, "|usr/sbin/sendmail -t");
  print LMAIL("To: $to\n");
  print LMAIL ("From: $from\n");
  print LMAIL ("Subject: $subject\n");
  print LMAIL ("--------------------\n\n");
  print LMAIL ("$body\n\n");
  print LMAIL ("-- \nSent from $ENV{REMOTE_ADDR} using $0 on $ENV{HOST
+NAME}\n");
  close LMAIL;
}
[download]

As an added bonus, making that a subroutine means you can send one message to the admin and another to the user by just calling it twice:

# mail sent to admin
sendmail($input{myEmail}, $input{Email}, $input{Subject}, $input{Messa
+ge});

# mail sent to user
sendmail($input{Email}, $input{myEmail}, "Re: ".$input{Subject}, "Than
+ks for your email! \n\n");
[download]

Later if you move the script to a server that does not have /usr/bin/sendmail, you just rewrite your sendmail() subroutine, and the rest will go unchanged. Now, on to your web page output...

#submit screen

print ("<html><head<title>$in{Form}</title></head>");
print ("<body bgcolor=\"ffffff\">");

print ("Thanks for your email.");

print ("</body></html");
[download]

Move the part that prints the content-type header down here too, to save confusion. This is very simple HTML, but it looks wellformed and mostly gets the point across. You could make it more elaborate later, once you get the thing working.

One little thing, though: NEVER EVER set a background colour without also setting the foreground. If you specify a white background, you need to specify a dark foreground. (Black, dark blue, whatever; as long as it shows up against white.)

Oh, and rather than put the form in a separate HTML file, I generally have the script print the blank form if it doesn't get any input. But that's more a matter of website structure than of Perl.

sub H{$_=shift;while($_){$c=0;while(s/^2//){$c++;}s/^4//;$
v.=(' ','|','_',"\n",'\\','/')[$c]}$v}sub A{$_=shift;while
($_){$d=hex chop;for(1..4){$pl.=($d%2)?4:2;$d>>=1}}$pl}$H=
"16f6da116f6db14b4b0906c4f324";print H(A($H)) # -- jonadab
[download]

print "Content-type: text/html\n\n";
[download]

Update 2003-02-25: Oops! chromatic (see below) is correct. The header is there, buried up top where I didn't see it.

I downvoted this node because the first half is incorrect.