What am I doing wrong?

Sure, your message will be further down, but how many spam messages have you gotten that have said "below is the result of your feedback form" and other ignorable garbage in the message.

Please, please don't deploy a script like this. {sigh}

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.

So how can I make it more secure?! Wait "x" amount of time until I can figure out what you just said? :(

To avoid contributing to the spam mess, don't deploy anything relating to sending email until you have it reviewed by a security expert, or you become a security expert yourself.

For the former, look around, I'm sure you'll find some. For the latter, start by reading and understanding every relevant CGI security document you can find. If you don't have time for the latter, hire the expert, or don't do it at all.

Please, please. Remember, fundamentally, CGI lets someone else use your computer's resources. Try to keep it to good use, instead of evil use. If you can't be responsible for that, you will be blacklisted, or worse, held liable for your lack of care.

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.

Addressing your second issue first, you probably should use a regular expression rather than index to do it. The index() function will return the position of the first occurrence of the string you are searching for. In your case, this means that unless the first occurrence of an exclamation point, question mark, or period is the last character in the string it will break. I think you can see how that matches your observations. A quick fix would be to use rindex() which searches from the end of the string backward but, as I said, a regex would be better. Something like:

$string .= '.' if $string !~ /[.!?]$/;
[download]

Regarding your first issue, look at the ucfirst() function. After you put the names in the array, you could do it like this:

@array = map ucfirst, @array;
[download]

-sauoq
"My two cents aren't worth a dime.";

Or $_ = ucfirst for @array;

cheers

tachyon

s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

Basically map takes a block and a list, and applies the code in the block to every element in the list (substituting each list element into $_). Then it returns the resulting list.*

If you want to do it in an explicit foreach loop, as in your sample code above, you could write it this way:

foreach my $word (@words) {
  $word = ucfirst $word
}
[download]

*--assuming you're in list context, which in this case you are

        $perlmonks{seattlejohn} = 'John Clyman';

Generally, you've got the right idea, but you just seem a bit confused about Perl syntax. In your code, you are doing @from = ucfirst($from). What you're doing is effectively wiping out the array (as you're assigning to @from with the initial-uppercased version of the first element. You probably want to do something like this instead:

my @friends = split / /, $raw_friends; # space in split() is fine
$_ = ucfirst for @friends;
[download]

From your confusion about @friends and $friends, you've shown that you haven't really learnt Perl in the best way (no offense intended). The difference between @friends and $friends in Perl is pretty fundamental, so it's important that you can understand it. I'd buy one of the books mentioned at the learn Perl site, and work through it. Don't worry, your investment in Perl will pay back very quickly!

Edit: fixed typo - changed uc to ucfirst

http://www.robj.ca/tell.htm

Thanks to SuperCruncher, I have gotten the "From:" name field to have all uppercase first characters, but can't get the same to happen for the "To:" name field to do the same. I know it has something to do with the $_ variable (I think?)

# make the first letters of the NAME of the sender uppercase
my @from = split(/ /, $from);
$_ = ucfirst for @from;

# make the first letters of the NAME of the receiver uppercase
my @to = split(/ /, $to);
$_ = ucfirst for @to;

# make the first letter of the message upper case
$message = ucfirst($message);
[download]

As for using a regex for the next part, I am lost :(
How can I use this line: $string .= '.' if $string !~ /[.!?]$/; in the code below?

##############################################
# puts a period at the end of senders message,
# if one was not there, AND, if there is no
# question OR explanation mark.

my $length = length($message);
my $period = index($message, ".");
my $question = index($message, "?");
my $explanation = index($message, "!");
$length = $length - 1;

if (($period ne $length) && ($question ne $length) && ($explanation ne
+ $length)) {
  $message = $message.'.';
}
##############################################
[download]

How can I use this line: $string .= '.' if $string !~ /[.!?]$/; in the code below?

Change it to $message .= '.' if $message !~ /[.!?]$/; and just replace that whole block of code with that one line.

But really, you should listen to merlyn's admonition and refrain from deploying this script!

-sauoq
"My two cents aren't worth a dime.";

The script works great now, but as per merlyn's request, I am not deploying this script until everyone is happy :)))) It is online, but there are no links to it, except the one on here.

Here is the final script:

#!/usr/bin/perl -wT

################################
################################
###                          ###  
###  written by Rob Johnson  ###
###  www.robj.ca             ###
###  tellscript@robj.ca      ###
###                          ###
################################
################################

use CGI::Carp qw(fatalsToBrowser);
use strict;
use CGI ':standard';

# fixes the error with -t switch in mailprogram line?
# hey I don't know... but it works! :)
$ENV{'PATH'} = "/bin:/usr/bin:/usr/sbin"; 

my $redirect = "http://www.robj.ca/told.htm";
my $sitename = "robj.ca";
my $siteurl = "http://www.robj.ca";
my $from = param('from');
my $to = param('to');
my $frommail = param('frommail');
my $tomail = param('tomail');
my $message = param('message');

###############################################
# make all first characters of names, uppercase

my @from = split(/ /, $from);
@from = map ucfirst, @from;

my @to = split(/ /, $to);
@to = map ucfirst, @to;

###############################################

# make the first letter of the message upper case
$message = ucfirst($message);

##############################################
# puts a period at the end of senders message,
# if one was not there, AND, if there is no
# question OR explanation mark.

$message .= '.' if $message !~ /[.!?]$/;

##############################################

# sending mail now...
open (MAIL, "|/usr/sbin/sendmail -t") || Error ('open', 'mail program'
+);
  print MAIL "From: $frommail\n";
  print MAIL "To: $tomail\n";
  print MAIL "Subject: @to, @from says check out $sitename!\n\n";
  print MAIL "This is NOT spam! You were sent the email by @from ($fro
+mmail),\nat IP: $ENV{'REMOTE_ADDR'}\n\n\n";
  print MAIL "Hello @to, @from has sent you this email inviting you to
+ check out $siteurl\n\n\n";
  print MAIL "@from also had this to say:\n$message\n\n\n" if ($messag
+e ne "");
  print MAIL "So check out $siteurl!";
close (MAIL);

sub Error {
  print "The server can't $_[0] the $_[1]: $! \n";
  exit;
}

# ready browser for html output 
print "Content-type: text/html\n\n";

# redirect the browser...
print "<meta http-equiv=\"refresh\" content=\"0;url=$redirect\">";

# end
[download]

How can this script be made safe agaist people who would want to exploit it?
Would it take much more work?

as per merlyn's request, I am not deploying this script until everyone is happy :)))) It is online, but there are no links to it, except the one on here.

If it's online then it is deployed. And there's a link to it posted on a well-known web site (this one!)

Please take the script down until you have solved the problems.

--
<http://www.dave.org.uk>

"The first rule of Perl club is you do not talk about Perl club."
-- Chip Salzenberg