is there something wrong with this code?

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

Hi,

I have written this little piece of code to act as a sucessful-hit logger on my home-page and i cant for the life of me get it to work.

Here's the code if anyonw wouldnt mind taking a look:

#!/usr/bin/perl
#
# To use this just insert this line into any html file you
# want to hit:
#
#  <!--exec cgi="/cgi-bin/hitlogger.cgi"-->
#
print "Content-Type: text/html\n\n";
print "HELLO\n";

# Gather all of the log strings
$dateprog          = "/bin/date";
$server_protocol = $ENV{'SERVER_PROTOCOL'};
$remoth_host = $ENV{'HTTP_X_FORWARDED_FOR'} || $ENV{'REMOTE_ADDR'};
$user_agent      = $ENV{'HTTP_USER_AGENT'};
$request_method  = $ENV{'REQUEST_METHOD'};
$request_uri      = $ENV{'REQUEST_URI'};
$request          = "$request_method $request_uri $server_protocol";
$date              = `$dateprog`;
chomp $date;
$log_strig          = "$remote_host - - [$date] \"$request\" 200 -";

# open the log and output the log string
open( LOG, ">/home/exposu/logs/web_access.log.txt" ) || die "$!\n";
print LOG "$log_string\n";
close LOG;
[download]

Cheers,

Elfyn

Comment on is there something wrong with this code? Download Code

Replies are listed 'Best First'.
•Re: is there something wrong with this code? by merlyn (Sage) on May 18, 2002 at 21:22 UTC
There's something wrong with the code in principle: as in, hit counters are bad -- you should be grepping the logs instead, and even those are wrong because of proxies and caching. But specifically, this code: `open( LOG, ">/home/exposu/logs/web_access.log.txt" ) \|\| die "$!\n";` [download] makes this a "one hit wonder". {grin} Each new hit overwrites the previous file instead of appending it. Change your `>` to a `>>` there. -- Randal L. Schwartz, Perl hacker	[reply] [d/l] [select]
Re: •Re: is there something wrong with this code? by emcb (Beadle) on May 18, 2002 at 21:33 UTC
I realiased this and hit back to change it and re-submit after preview, but it did'nt work. And im ashamed that i left out the '-w'! Sorry the open has the '>>' but i cant edit it now, or can i?	[reply]
Re: is there something wrong with this code? by DamnDirtyApe (Curate) on May 18, 2002 at 22:14 UTC
Two comments here: `use strict;` would inform you of at least two typos in your variable names -- `$remoth_host` and `$log_strig`. Do the permissions on the dir/file you are attempting to write to allow write access to your web server? If not, no log. Hope that helps steer you in the right direction. _______________ D a m n D i r t y A p e Home Node \| Email	[reply]
Re: is there something wrong with this code? by perigeeV (Hermit) on May 19, 2002 at 11:34 UTC
`<!--exec cgi="/cgi-bin/hitlogger.cgi"-->` You're missing the hash before exec: `<!--`<bold>#</bold>`exec...` When you use the `exec` SSI, script output is not included into the document. I assume that's what you intend with `print "HELLO\n";`. For that you need to use `<!--#include virtual="/cgi-bin/hitlogger.cgi"-->` Here's the docs. There are plenty of log analysis tools out there, though.	[reply] [d/l] [select]
Re: is there something wrong with this code? by Parham (Friar) on May 19, 2002 at 13:46 UTC
i noticed this which doesn't seem right: `$log_strig = "$remote_host - - [$date] \"$request\" 200 -";` [download] you might want to add the 'n' in $log_strig, same goes for $remoth_host.	[reply] [d/l]


"be consistent"
	PerlMonks