Seeking Feedback on Chat Program (was Seeking Feed back)

Cobo has asked for the wisdom of the Perl Monks concerning the following question:

Hey, I'm a Perl newbie heh and I was just wondering if any of the people here could give me any feed back, this is a chat program I made, (well the actual program part) it is put into a frame with a page that it appends to that is located outside of my cgi-bin so that perl scripts are'nt executed, any ways here it is lol.

#!/usr/local/bin/perl

use CGI;

use CGI ':standard';
print header;
print start_html('Chat'),
h1('Chat'),
    start_form,
    "Name ",textfield('name'),
    p,
    "Say ",textfield('say'),
    p, 
    submit,
    end_form,
    hr;

if (param()) {
    
$say=(param('say')); 
$name=(param('name'));
       
     
    $say =~  s/</&lt;/g; #Stops user from entering the < in both name 
+and speech
    $name =~ s/</&lt;/g; #Leaves the > allowed, you can't enter HTML w
+ithout <
        
        $say =~ s/!{5}/!/g;  #Although this won't take care of all mul
+tiple ! it can reduce 
                 #the number to help avoid multiple lines of just !'s 
        
     open CHAT, ">>../chatty2.html";
        print CHAT "<h5><i>",$name,": </i><b>",$say,"</b></h5>\n";
        close CHAT

    }
      
 print end_html;
[download]

I'm proably doing a bunch of stuff wrong heh, any feed back you can provide would be helpful, thanks :)

Comment on Seeking Feedback on Chat Program (was Seeking Feed back) Download Code

Replies are listed 'Best First'.
Re: Seeking Feed back by azatoth (Curate) on Jul 05, 2001 at 12:34 UTC
For starters, all people new to Perl should be made aware of strict.pm. This is a Perl Module that ensures your program runs smoothly with regard to Scoping. You should also read up on use warnings; or the -w switch, which basically adds an extra level of integrity to your program. Also on your filehandle call, you should always finish the statement off with `or die "Could not open $fh : $!\n";` The die function enables error handling, storing the error value in the special variable $!. So your code should now look like `open CHAT, ">>../chatty2.html" or die "Could not open file : $!\n"; # your script will fail if you can't open the file, and print the reas +on why` [download] In fact, you should use die on most function calls. Read up on that, it'll save you a lot of heartache in the future. Finally, enjoy your time on Perlmonks. It's a very nice place. Azatoth a.k.a Captain Whiplash Make Your Die Messages Full of Wisdom! Get YOUR PerlMonks Stagename here! Want to speak like a Londoner?	[reply] [d/l] [select]
Re: Re: Seeking Feed back by orkysoft (Friar) on Jul 05, 2001 at 13:04 UTC
Actually, in my 1200-line Mapster program (currently down), I used constructs like: `if(open FILE, ">$filename") { flock FILE, 2; ... # Write stuff to FILE close FILE; } else { print "An I/O error occurred!"; log_error $!; # Call a sub }` [download] This allows my program to exit gracefully in the unlikely event of an error.	[reply] [d/l]
Re (tilly) 3: Seeking Feed back by tilly (Archbishop) on Jul 05, 2001 at 15:58 UTC
And so you have an error, you know that there is no such file or directory. What then? How does this help in debugging. As perlstyle says: `o Always check the return codes of system calls. Good error messages should go to STDERR, include which program caused the problem, what the failed system call and arguments were, and (VERY IMPORTANT) should contain the standard system error message for what went wrong. Here's a simple but sufficient example: opendir(D, $dir) or die "can't opendir $dir: $!";` [download] By not including debugging information on your arguments you will make such basic things as a renamed directory painful to track down.	[reply] [d/l]
Re: Re (tilly) 3: Seeking Feed back by azatoth (Curate) on Jul 05, 2001 at 16:20 UTC
Re (tilly) 5: Seeking Feed back by tilly (Archbishop) on Jul 05, 2001 at 18:01 UTC
Re3: (Azatoth): Seeking Feed back by pmas (Hermit) on Jul 05, 2001 at 18:03 UTC
Some notes below your chosen depth have not been shown here
Re: Re (tilly) 3: Seeking Feed back by orkysoft (Friar) on Jul 05, 2001 at 16:53 UTC
Re (tilly) 5: Seeking Feed back by tilly (Archbishop) on Jul 05, 2001 at 17:52 UTC
Some notes below your chosen depth have not been shown here
Re: Re: Re: Seeking Feed back by Chady (Priest) on Jul 05, 2001 at 15:46 UTC
what? `if () {} else {}` ? what about: `open FILE, ">$filename" or &exit_with_error ('An I/O error occured!', +$!);` [download] and in your `&exit_with_error` sub you do more stuff and `exit` just a thought He who asks will be a fool for five minutes, but he who doesn't ask will remain a fool for life. Chady \| http://chady.net/	[reply] [d/l] [select]
Re: Seeking Feed back by Maestro_007 (Hermit) on Jul 05, 2001 at 19:20 UTC
One more thing - I would add `-T` to the shebang line. You do a few checks on the data to make sure nobody's trying to hack it, but, to quote various sources on the subject, "never trust any input from the user". Your checks are good for what you've thought of, but there may be some things that you haven't thought of. Also, you may end up altering the code some day in such a way to make these security holes more apparent and exploitable. MM	[reply] [d/l]
A reply falls below the community's threshold of quality. You may see it by logging in.