use strict;
use CGI;

my $query=CGI->new();
my %laundry= ( 'name' => '(\w+)',
               'number' => '(\d+)',
               'choice' => ([ABC]),
             );
my %laundered_params;

foreach my $param (keys %laundry) {

  die "Parameter $param not present\n" unless my $input=$cgi->param($p
+aram);
  my $regexp=$laundry{$param};
  $input =~ /$regexp/;
  die "Invalid input for $param\n" unless $laundered_params{$param}=$1
+;

}
[download]

CU
Robartes-

Or maybe like this:

while(my ($param, $rx) = each %laundry) {
    local $_ = $cgi->param($param)
        or die "Parameter $param not present\n";

    ($laundered_params{$param}) = /$rx/;
        or die "Invalid input for $param\n";
}
[download]

Note that both snippets will behave in probably undesired ways if any of the parameters have multiple values.

Makeshifts last the longest.

Your validation tests look reasonable. One optimisation would be to use an array for the error messages, like so:

# untested, change to suit
my @messages;
if ( ....test.... )
{
    push @messages, "The error message";
}
# more tests

PrintError if @messages;

sub PrintError
{
    # any initial html stuff, or use a template
    print join('<br>', @messages);
}
[download]

...which means you don't need to maintain a flag.

Looking on CPAN, I see CGI::Validate which appears to a a Getopt style validator and Params::Validate which may be useful.

Update:Podmaster suggested Data::FormValidator, which is what I was trying to find.

Update2: I left the & in there because that's what the supplicant had - now removed. For reasons why not to do this, see perlsub. Basically, with the &, the argument list to a sub is optional and you will get the @_ array visible at the time of calling in the subroutine.

Don't use &subroutine; unless you're sure that you want to do that. You very likely want subroutine(); instead.

As for cleaning the code a litte: you could do something like:

Warning: untested code ahead

my %opts = (
  bandwidth => { 
    err => 'Your Chosen Bandwidth Option Does Not Appear
            To Be Valid. Contact Support For Assistance.',
    match => qr/^15|0$/,
  },
  support => {
    err => 'Your Chosen Support Option Does Not Appear
            To Be Valid. Contact Support For Assistance.',
    match => qr/^2|0$/,
  }
   # etc etc.

);

my $error = '';
while (my ($name,$check) = each(%opts)) {
    unless ($q->param($name) =~ $check->{match}) {
       $error .= "<p>$check->{err}</p>\n";
    }
}
if ($error) {
   print_error($error);
}
[download]

-- 
Joost       downtime n. The period during which a system
            is error-free and immune from user input.
[download]

From a code standpoint it doesn't look too bad, but from a logical standpoint you have a lot of issues. First you exclude a whole lot of valid email addresses by making assumptions about what characters are allowed in an email address (pretty much anything is allowed on the left side of the @). You have also made some big assumptions about characters that will not appear in your customers names, unless you don't want any customers named O'Hara or O'Malley. From a purely nit-picky point of view, the reserved-username message will probably lead people to believe their username must be a non-dictionary word.

I used an approach like this in CGI::Search, which could be adapted to your situation CGI.

A series of subroutines are used to validate the input. The subs take in the data to validate and return a list of three values. The first value is a boolean value of wheather the data validated or not. The second is the data that was validated, but in untained form (see perlsec). The third is a string that can be used as an error message if the data didn't validate.

To preform the validation, you make a hash-of-arrays with three elements in the array portion. The key of the hash is the name of the field. The zeroth element of the array is the data to validate. The first element is a referance to a validation subroutine. The second element is a boolean value of wheather the given data is required or not. If that second element is false, than the field is allowed to be blank. If true, then the data still has to pass the validator.

Tieing it all together:

use CGI qw(:standard);

# Validation subroutines defined elsewhere

my %DATA = (
    field1 => [ param('field1'), \&INTEGER, 1 ], 
    field2 => [ param('field2'), \&EMAIL,   1 ], 
    field3 => [ param('field3'), \&WORD,    0 ], 
);

sub do_validation 
{
    foreach my $key (keys %DATA) {
        if($DATA{$key}[2]) {
            my @result = $DATA{$key}[1]->($DATA{$key}[0]);
            die "Didn't validate: $result[2]" unless $result[0];
        }
    }
}
[download]

The advantage of this compared to a regex is flexibility. A subroutine can do whatever checks it wants to the data. For instance, a credit card validator can run Business::CreditCard on the data.

WWW::Form does something similar, but adds the ablity to put the subrountines in an array. This means that a single peice of data must pass more than one validation sub.

----
I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
-- Schemer

Note: All code is untested, unless otherwise stated

A fine distinction should be made between validating the input and untainting it.

Untaininting should be more concerned with security of your web-site (to make sure, the user does not slip in some executable code or other bad things), whereas validating has more to do with obtaining valid data from the user.

It seems you are more concerned with validating here and in order to do that you need to make a model of the data you are expecting and check the input against that model (ask yourself: what is a valid date, name, password, ...).

Whether you do this checking through multiple if tests and regexes or any of the other suggested methods is less important than finding/setting the rules which define valid input.

CountZero

"If you have four groups working on a compiler, you'll get a 4-pass compiler." - Conway's Law