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

G'day all I have been trying to write a script which takes a user's input from a form on the submission page (addnews.html) and adds it to the main page (index.html). The plan is to store the addnews.html file in a .htaccess protected directory and have a static un-updatable password to stop everyone and everbody adding news. My script currently is: 
#!/usr/bin/perl

# news.pl
# Attempt #3 at a news script.

# Set up the standard HTML header.

    print "Content-type: text/html\n";

# Read in the data from the "addnews.html" file.

    $data_length = $ENV{'CONTENT_LENGTH'};
    $bytes_read = read (STDIN, $temp_data, $data_length);

# Store the 'temp_data' in an array called '@temp_name_value' for mani
+pulation.

    @temp_name_value = split (/&/, $temp_data);

    foreach $temp_name_value (@temp_name_value)
    {

    # Split up the 'name=value' pairs.

        ($name, $value) = split (/=/, $temp_name_value);

    # Replace '+'s with ' 's.

        $name =~ tr/+/ /;
        $value =~ tr/+/ /;

    # Translate hexidecimal characters back into their original charac
+ters.

        $name =~ s/%(..)/pack("C",hex($1))/eg;
        $value =~ s/%(..)/pack("C",hex($1))/eg;

    # Create an associative array using the '$name" and '$value' varia
+bles.
    # NOTE: If there is no '$name' specified '$value' is used instead.

        if ($form_data{$name})
        {
            $form_data{$name} .= "\t$value";
        }
        else
        {
            $form_data{$name} = $value;
        }
    }

# Create a variable to store the location of the display page, "index.
+html".

    $news_data = "/index.html";

# Open the '$news_data' file to read the current data or die with an e
+rror message.

    open (NEWS, ">$news_data") || die "Unable to locate $news_data, th
+e news data file.";

# Read the data form '$news_data' into an array called '@news'.

    @news = <NEWS>;

# Close the '$news_data' file after use.

    close (NEWS);

# Re-open the '$news_data' file for the input of the new data.

    open (NEWS, "<$news_data") || die "Unable to locate $news_data, th
+e news data file a second time.";

# Write the data back into the '$news_data' file with the new data inc
+luded where appropriate.

# Loop through each line of the '$news_data' file and write it back.
    
    foreach $line (@news)
    {

        # Search for the '<!--latest_news-->' marker.

        if ($line =~ /<!--latest_news-->/i)
        {
            # Write itself back for future use.

            print NEWS "<!--latest_news-->\n";

            # Add the 'nickname' of the poster.

            print NEWS "From: $form_data{'nickname'}</TD>\n<TD>";

            # Add the 'headline' of the post.

            print NEWS "<B>$form_data{'headline'}</B></TD>\n<TD>";

            # Add the 'news_text' underneath the 'headline'.

            print NEWS "$form_data{'news_text'}";

        }
        else
        {
            die "Unable to find the position header in $news_data, the
+ news data file.";
        }

    }

# Close the '$news_data' file after use.

    close (NEWS);

# Provide the poster with a link to their news item.

    print << "EOF";
    <H2 ALIGN="center">Thank you for your news item</H2>
    <P ALIGN="center">Click <A HREF="../index.html">here</A> to view y
+our news item.<BR>
    Note that you may have to reload the file before your post is visi
+ble.</P>
EOF

# EOF news.pl

[download]
Could someone please give me some ideas on where to go from here, in terms of making it functional..? Thanks David

Replies are listed 'Best First'.
Re: Writing a simple posting script which can be used via the web
by ajt (Prior) on Sep 29, 2001 at 16:45 UTC
    You don't fully explain all the details, but here are some suggestions - I'm sure you'll get lots more too from more learned monks.
    • Use Strict - see perlman:lib:strict - Basically it will help avoid typos, and other silly errors that bite you when you least expect them.
    • Turn -w on - see perlman:lib:warn - Helps you track down lots of errors
    • Turn -T. You can't trust user input, and on a server, you have to be doubly sure that they are inputting what you want them to.
    • Go and look at the perlman:lib:CGI module, it will save you so much pain to use this, it's the stanard way of doing all things CGI. For reasons why you should do this see use CGI or die; amongst others.

    Starting from the above you can get rid of most of your CGI parsing code which will make your script a lot easier to work with. Your script will start to look more like: 

    #!/usr/bin/perl -wT
# news.pl

use Strict;
use CGI qw(:standard);

    [download]

    Now that you have done that, the CGI moudule will take care of all your form processing, for example to put the value of the foo paramater into a variable you can use: my $bar = param("foo"); much easier than doing it yourself.

    CGI also handles the output too, so to print the HTTP header, and your HTML you can now do something more like: 

    print header(),
      start_html,
      hr, h1("Heading"), hr,
      end_html;

    [download]

    and this will send the following to the browser: 

    Content-Type: text/html; charset=ISO-8859-1

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html
        PUBLIC "-//W3C//DTD XHTML Basic 1.0//EN"
        "http://www.w3.org/TR/xhtml-basic/xhtml-basic10.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="en-US"><head><title>U
+ntitled Document</title&gt;
</head><body><hr /><h1>Heading</h1><hr /></body></html>

    [download]

    This will no doubt make your life a lot easier... If you want to find out more, browse this site you may wish to look at some of the books in the Book Reviews section, have a look at Writing CGI Applications with Perl amongst others.

    Anyhow my comments are a small start, others will no doubt add more.

    Good luck.

[reply]
[d/l]
[select]
Re: Writing a simple posting script which can be used via the web
by wog (Curate) on Sep 29, 2001 at 18:00 UTC
    Ignoring that you should use CGI and strict and warnings as mentioned previously (which would fix one problem -- you don't output proper headers), you have a few logic errors in your script: 

    # Open the '$news_data' file to read the current data or die with an e
+rror message.

    open (NEWS, ">$news_data") || die "Unable to locate $news_data, th
+e news data file."

    [download]

    The problem here is that you are actually opening the $news_data file for writing here. Not what you want to do. Your code is simillary reversed for writing the file. Additionally the file not being found is not the only reason the open could fail. Try including $! in your error message to give a better idea what's wrong. (Note that unless you do use CGI::Carp 'fatalsToBrowser' or similar (often not reccommended in production code) die messages will typically go to the error log, not the browser.)

    Also, note that opening a file, storing what's in it, and then closeing it before reopening it again opens you up to a race condition. That is, if two people try to add a news article at the same time you may suffer data loss. You should open the file once for update (both read and write, see the docs for open), and use seek (so you can read the file, and then rewind to the beginning to re-write it) and flock to avoid this.

    Another problem is in your foreach loop. You check, for every line in the file you've read, if it matches <!--latest news-->, but for every line you throw an error if it doesn't. (On a sidenote you may want to consider what happens if someone puts <!--latest news--> inside the text of a news item.) Furthermore you do not output the lines in the file as you iterate through them, meaning that only your new news item will be outputed.

    update: s/Crap/Carp/ (... thanks jlongino)

[reply]
[d/l]
[select]
      I think wog means  use CGI::Carp not  Crap :)
[reply]
[d/l]
[select]
Thanks and further advice
by lagrenouille (Acolyte) on Sep 29, 2001 at 19:42 UTC
    G'day it's me again (I've registered BTW) Thanks for the advice regarding switches and so on... I have a book but i'm REALLY doubting its reliability It's full of typos in code AND text (to the point where I had to debug the first example to get it to work!) so any advice on suitable replacements would be most appreciated too. It didn't mention anyhing about a CGI module, what exactly is this and how does it work? (obviously the syntax is in the replies) And thanx for the information on the HTML headers, it explains why I keep getting "insufficient header information" errors. I'm trying to run this script on my linux box using Apache and the counter script I wrote worked, even with the suspect HTML headers. I will give your advice a try when I next get on my linux box and let you know how it goes. I have read\heard that code setting out etc. is very important in perl and cgi scripts, not for functionality but for debuging and portability does my code look ok or are there some things to improve. It does look a bit suspect centred though. Thank you very much for your help and I think I may call on you in the future. La_Grenouille lagrenouille@cyberarmy.com
[reply]
      I have a book but i'm REALLY doubting its reliability It's full of typos in code AND text (to the point where I had to debug the first example to get it to work!) so any advice on suitable replacements would be most appreciated too.
      Name of the book please, so we can learn to avoid it?

      As for good books, the Mouse (2nd edition) is a decent book on CGI/Perl. And I've got about 30 examples of using CGI at my columns.

      -- Randal L. Schwartz, Perl hacker

[reply]

Back to Seekers of Perl Wisdom