You cannot know that it's opening the files correctly unless you check the return values of your open calls. It's entirely possible that the script opens the files properly when run via the command line (with your user permissions), and fails based on permissions when run by the webserver (who usually runs as 'nobody').

Use the ever-popular "or die "Couldn't open the file: $!";" construct after your opens to ensure things are hunky dory. See perlopentut for examples. Failing to check the return value of open is a very common mistake among beginners. It is, however, one of those absolutes in life: always always always check the return value of open. Even if you're sure the file will open, even if you don't care if it opens or not, check; develop the habit.

If the script dies once you check the return value of open, then you can at least check the server's error log to see what error is being reported.

It also might not hurt to print out all of the parameters captured by the CGI module, so that you can ensure that the params are what you think they are.

This piece of your code does not make sense to me:

   open(NEWS,"news.html");
    while(<>) {
        $pagetext .= $_;
    }
    close NEWS;
[download]

I guess what you want is: (otherwise why open/close the file?)

   open(NEWS,"news.html");
    while(<NEWS>) {
        $pagetext .= $_;
    }
    close NEWS;
[download]

I just wanted to follow-up a second time to point out something that I think is a lot more of a valuable lesson than any of the answers you've received, taken individually.

You've just witnessed a thought-process in action. Your script didn't work as you expected. We took a look at it and each came up with a different suggestion. But each was a suggestion that, applied in a more general sense, begins to answer the more important question of "how do I figure out what's going wrong, when something goes wrong?"

One answer is always check return values. Another answer is to develop your scripts under strictures (use strict;) and warnings (use warnings;). Another answer was to use CGI::Carp. Another answer (correctly) pointed you at the issue of opening a filehandle but reading from the <> diamond operator (sans named filehandle). And another was to look more closely at a means of simplifying the logic of your if-else statements.

Collectively these represent a process of programming in such a way that Perl tells you what's wrong, and in such a way that is less likely to develop hard to find errors. Many of the "why doesn't this work" questions posted here end up with a similar thought-process zeroing in on the problem. The implication is that many of the "why doesn't this work" questions a person has regarding his script, he can answer himself by applying the practices and thought processes that simplify debugging: Checking return values, simplifying logic, using strict, using warnings, using Carp (where applicable), using lexical filehandles (which are beneficial for many reasons not the least of which is the fact that a lexical filehandle must pass strictures), and so on. Tackle the bigger issue and the smaller ones fall into place.

This is just a thought that hopefully someone will find entertaining.... not quite worthy of being a meditation, but possibly worthy of mention. ;)

Wow. These are all great, helpful answers. Thank you, fellow Monks.
My Website

In addition to davido's advice I have found that CGI::Carp can be very valuable when hunting down errors in CGI scripts.

use strict;
use warnings;
[download]

After you got it to work, consider changing your 'if' and 'elsif' to:

my $query = get_param('node') || 'default';

my %nodes = (
   default  => \&default,
   news    => \&news,
   records => \&records, 
   quests => \&quests,
   help => \&help,
   _err    => \&invalid
);

eval { $nodes{ exists $nodes{$query} ? $query : '_err' }(); };
[download]

You certainly have a good point about using a hash as a dispatch table, but I utterly fail to understand why you would need to use eval. How about:

my $dispatch = $nodes{$query} || $nodes{_err}; 
$dispatch->();
[download]

Or if you really like to put everything in one line:

( $nodes{$query} || $nodes{_err} )->();
[download]

--
edan

Hi edan,

Thanks for pointing out. I missed out the catch error code below:

if ($@) {
  dienice();
}
[download]

Why suggest this when you could just use CGI::Application which does all of this, and more?

------
We are the carpenters and bricklayers of the Information Age.

Then there are Damian modules.... *sigh* ... that's not about being less-lazy -- that's about being on some really good drugs -- you know, there is no spoon. - flyingmoose

I shouldn't have to say this, but any code, unless otherwise stated, is untested

Hi dragonchild,

Spidy appears to be new to Perl CGI and is trying to get basic things to work. He's using a chain of 'if' and 'elsif' which can be improved with the hash director thingy without too much sweat.

I've heard of CGI::Application but have not used it before.