While I concur with hangon's endorsement of Mime::Lite, I primarily wanted to offer you a couple of notes:

First, you go through the trouble of ripping through your argument list, but in some cases, you're just passing the same argument list to another function and returning. I'd suggest you change the start of your sub to something like this:

sub send_mail { $CONFIG{mail_format} = 2; if($smtp_path =~ /\w+/){ $CONFIG{mailprogram} = $smtp_path; $CONFIG{mailuser} = $smtp_user; $CONFIG{mailpass} = $smtp_pass; return &send_mail_NT(@_); }

Some may argue against this suggestion: If you change the argument list to your sub, then you'll have to make changes to call the send_mail_NT function anyway. But since you already have another send_mail function, it's advisable to use compatible argument lists. (Note: I also removed the creation of the $VAR{res} slot which you use only to hold a return value.)

Second, you're constructing a hash from your arguments, but you don't need any hash features in your sub, so it's just a lot of extra typing. I'd suggest you just split your argument list into named variables and use them throughout your sub:

my ($sndrEml, $sndrNme, $rcvrEml, $rcvrNme, $subject, $message, $c +crcEml, $ccrcNme, $attPths) = @_;

Third, you perform identical operations on several values. You might want to either (a) create a sub to handle the task, or (b) do them all in a single statement. For example, your statements:

$sndrNme =~ s/\&lt;/\</ig; $rcvrNme =~ s/\&lt;/\</ig; $subject =~ s/\&lt;/\</ig; $ccrcNme =~ s/\&lt;/\</ig; $sndrNme =~ s/\<.*?>//ig; $rcvrNme =~ s/\<.*?>//ig; $subject =~ s/\<.*?>//ig; $ccrcNme =~ s/\<.*?>//ig;
all perform the same cleanup tasks. If you need to change your cleanup code, you have three places to change it in. You can create a sub like this:

sub _clean_value { local $_ = shift; s/\&lt;/\</ig; s/\<.*?>//ig; $_; }

allowing you to clean up your values like this:

$sndrNme = _clean_value($sndrNme); $rcvrNme = _clean_value($sndrNme); $subject = _clean_value($subject); $ccrcNme = _clean_value($ccrcNme);

Alternately, if you don't think it's worth a subroutine, you could perform all the operations at one go by taking advantage map, like so:

($sndrNme, $rcvrNme, $subject, $ccrcNme) = map { s/\&lt;/\</ig; s/\<.*?>//ig; $_; } ($sndrNme, $rcvrNme, $subject, $ccrcNme);

Finally, your sub serves two purposes, and it's better to break down your program such that each sub serves a single, well-defined purpose. The first purpose your sub has is to decide between methods used to send an EMail. The second is to actually send the EMail. So I'd suggest you chop up your sub into something like this1:

sub send_email { # Set up values that *all* the EMail senders will need to know $CONFIG{mail_format} = 2; # Which method should we use to send the EMail? if($smtp_path =~ /\w+/){ # specific values used by send_mail_NT $CONFIG{mailprogram} = $smtp_path; $CONFIG{mailuser} = $smtp_user; $CONFIG{mailpass} = $smtp_pass; return send_mail_NT(@_); } else { # Our default method if no better method was found $CONFIG{mailprogram} = $send_mail_path; return send_mail_Default(@_); } } sub send_email_Default { my ($sndrEml, $sndrNme, $rcvrEml, $rcvrNme, $subject, $message, $c +crcEml, $ccrcNme, $attPths) = @_; my $Host = gethostname; my $CRLF = "\n"; my $message .= qq~$CRLF$CRLF~ .= qq~IP Address: $ENV{REMOTE_ADDR}$CRLF~ . qq~Host Node: $Host$CRLF~ . qq~User Agent: $ENV{HTTP_USER_AGENT}~; $message =~ s/\r//ig; $message =~ s/\n{3,300}/$CRLF$CRLF/ig; $attPths =~ s/\s+//ig; my $HTMLmess = $message; my $TEXTmess = $message; $TEXTmess =~ s/\&quot;/"/g; $TEXTmess =~ s/\&lt;/\</ig; my $files = ""; my $hasAtts = 1; ($sndrNme, $rcvrNme, $subject, $ccrcNme) = map { s/\&lt;/\</ig; s/\<.*?>//ig; $_; } ($sndrNme, $rcvrNme, $subject, $ccrcNme); my $bound1 = qq~----=_NextPart_P_115Dream~; # printable my $bound2 = qq~----=_NextPart_A_SubHB~; # attachments my $bound3 = qq~----=_NextPart_E_SlowTrain~; # embded my $fm = _fixNme($sndrEml, $sndrNme); my $to = _fixNme($sndrEml, $sndrNme); my $cc = _fixNme($sndrEml, $sndrNme); if(open(M, qq~|$CONFIG{mailprogram} -t ~)){ print M qq~From: $fm$CRLF~; print M qq~To: $to$CRLF~; print M qq~CC: $cc$CRLF~ if $ccrcEml; print M qq~Subject: $subject$CRLF~; print M qq~MIME-Version: 1.0$CRLF~; print M qq~$VAR{fileHeaders}~; print M qq~Content-Type: text/plain;$VAR{CRLF}~; print M qq~Content-Transfer-Encoding: 7bit$VAR{CRLF}$VAR{CRLF} +~; print M qq~$VAR{TEXTmess}$VAR{CRLF}$VAR{CRLF}$VAR{files}~; close M; return 1; } else { return; } } sub _fixNme { my ($Eml, $Nme) = @_; return $Nme =~ /\w+/ ? qq~$Eml ($Nme)~ : $Eml; }

Notes:

  1. Since $CRLF is defined as "\n", I removed the s/\n/$CRLF/ig statement.
  2. You explicitly set $CONFIG{mail_format} to 2, so I removed code for other value(s).
  3. I didn't add any error checking where it's obviously missing (e.g. on open statement).
  4. I tried to leave most of your code alone, other than the suggestions made.
  5. Standard disclaimers apply: Untested code, you can keep all the pieces when it breaks, most work left as an exercise for the reader, yadda yadda yadda.

...roboticus

Update: Node seems to be a bit long, so I added readmore tag to shorten it a bit; also fixed a footnote reference.


In reply to Re: How to add attachments to an email using cgi/perl by roboticus
in thread How to add attachments to an email using cgi/perl by Anonymous Monk

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.