Re: a way to make this more secured and better written?

From: Jupiterhost.Net (mlists_at_jupiterhost.net)
Date: 06/15/04

  • Next message: James Edward Gray II: "Re: How to open a file for read as well as write" 
    Date: Tue, 15 Jun 2004 07:38:53 -0500
To: Jason Gray <perl@cableone.net>

    Jason Gray wrote:
    > #!/usr/bin/perl -w
    >
    > use strict;
    > use warnings;
    >
    > use CGI();
    > use Mail::Mailer;
    >
    > my $q = CGI->new();
    >
    > print $q->header();
    >
    > #-----------------------------------------
    > # * startup methods -> global variables.
    >
    > check_fields();
    >
    > #-----------------------------------------
    >
    > sub check_fields {
    > my $blanks;

        my $blanks = 0; # so you avoid uninitialized warnings if no params
    are given.

    > my @fields = qw(name email city state message);
    > foreach my $field (@fields) {
    > $blanks++ if !$q->param($field);
    > }
        for(@fields) { $blanks ++ if $q->param($_) eq ''; }

    > if($blanks) {
    > print qq(Error: There were some blank fields.);
    > exit;
    > }
    > unless($q->param('email') =~ /\@[^.]*\./) {
    > print qq(Error: Please enter a valid email address.);
    > }
    > send_email();

        if($blanks) {
          print 'Error: There were some blank fields.';
        elsif($q->param('email') ~~ /\@[^.]*\./) {
          print 'Error: Please enter a valid email address.';
        } else {
          send_mail();
        }
    Single quotes mean no interpolation which is a bir faster, aslo not exit
    call.

    > }
    >
    >
    > sub send_email {
    > my $m = Mail::Mailer->new('sendmail');
    > $m->open({ From => $q->param('email'),
    > To => 'perl <perl@cableone.net>',
    > Subject => '[INFO] Site Comment',
    > }) or die $!;
    > print $m "Name: " . ucfirst($q->param('name')) . "\n";
    > print $m "Location: " . ucfirst($q->param('city')) . ", " .
    > $q->param('state') . "\n\n";
    > print $m $q->param('message');
    > $m->close();
    > print qq(Thank you for emailing us. We will contact you in the next 24
    > hours.);

    I'd switch any double quotes (" and qq) to singel if there are no vars
    to be interpolated.

    > }

    Also, why are these in funtions? You're not reusing them. Why not:

    #!/usr/bin/perl

    use warnings;
    use strict

    use CGI();
    use Mail::Mailer;

    my $q = CGI->new();

    print $q->header();

    my $blanks = 0;

    for(@fields) { $blanks ++ if $q->param($_) eq ''; }

    if($blanks) {

        print 'Error: There were some blank fields.';

    } elsif($q->param('email') ~~ /\@[^.]*\./) {

        print 'Error: Please enter a valid email address.';

    } else {

       my $m = Mail::Mailer->new('sendmail');
       $m->open({ From => $q->param('email'),
                   To => 'perl <perl@cableone.net>',
                   Subject => '[INFO] Site Comment',
                 }) or die $!;
       print $m 'Name: ' . ucfirst($q->param('name')) . "\n";
       print $m 'Location: ' . ucfirst($q->param('city')) . ', ' .
       $q->param('state') . "\n\n";
       print $m $q->param('message');
       $m->close();
       print 'Thank you for emailing us. We will contact you in the next 24
    hours.';

    }

    HTH
    Lee.M - JupiterHost.Net

  • Next message: James Edward Gray II: "Re: How to open a file for read as well as write"

    Relevant Pages