Too many variables?

[MrCBofBCinTX]

After seeing more and more bandwidth on my server getting eaten up by more and more spam, etc.
I cobbled together a shell script with tidbits from the apache error log for a few weeks.

It got too long for my tastes so I did this perl script to do the work.
Works fine, but seems to me that it may have too many variables for such a simple script.

I'm not a real wiz with complex data structures, so I may not be seeing an obvious way to improve on this.

#!/bin/perl

my @list;
my @log;
my @fields;
my @logged;
my @crap;
my $doodoo;
my @matches = ('soapCaller', 'phpmyadmin', 'source\/mod\/', 'a\.asp', 'xmlrpc', 'viewitem', 'manager', ' ___', 'noexist', 'thisdoesnotexistahaha.php', '/mwf\
/index.php', 'cacti', 'admin.*js');
my $runs = $ARGV[0] ||= 1;  #Allows debug and one email per day from cronjob
do {
open LOG, "<", "/var/[URL unfurl="true"]www/logs/error_log";[/URL]
@fields = <LOG>;
close (LOG);
foreach my $match (@matches) {
@logged = grep(/$match/, @fields);
push (@log, @logged);
}

foreach my $crap (@log) {
my @duh = split (/ /, $crap);
my $duh = $duh[7];
$duh =~ s/]//;   #Removes ] from IP entry in log
push (@list, $duh);
}
my @uniq = keys %{{ map { $_ => 1 } @list }};
foreach my $poop (@uniq) {
$doodoo .= "$poop ";
}
print "$doodoo\n";
system "pfctl -t badhosts -T add $doodoo";
undef $doodoo;
sleep 10;    # Using perl to run more often than 1 minute from cron
} while ($runs > 0);

 

[Annihilannic]

I've managed to remove some of the faecal matter from your script (with which it was heavily laden!). Let's hope it keeps the $runs away...

No doubt my learned friends will find many more optimisations...

[gray]#!/bin/perl -w[/gray]
[url=http://perldoc.perl.org/functions/use.html][black][b]use[/b][/black][/url] [green]strict[/green][red];[/red]

[url=http://perldoc.perl.org/functions/my.html][black][b]my[/b][/black][/url] [blue]@list[/blue][red];[/red]
[black][b]my[/b][/black] [blue]@log[/blue][red];[/red]
[black][b]my[/b][/black] [blue]@fields[/blue][red];[/red]
[black][b]my[/b][/black] [blue]@logged[/blue][red];[/red]
[black][b]my[/b][/black] [blue]@matches[/blue] = [red]([/red]
        [red]'[/red][purple]soapCaller[/purple][red]'[/red],
        [red]'[/red][purple]phpmyadmin[/purple][red]'[/red],
        [red]'[/red][purple]source\/mod\/[/purple][red]'[/red],
        [red]'[/red][purple]a\.asp[/purple][red]'[/red],
        [red]'[/red][purple]xmlrpc[/purple][red]'[/red],
        [red]'[/red][purple]viewitem[/purple][red]'[/red],
        [red]'[/red][purple]manager[/purple][red]'[/red],
        [red]'[/red][purple] ___[/purple][red]'[/red],
        [red]'[/red][purple]noexist[/purple][red]'[/red],
        [red]'[/red][purple]thisdoesnotexistahaha.php[/purple][red]'[/red],
        [red]'[/red][purple]/mwf\[/purple]
[purple]        /index.php[/purple][red]'[/red], [red]'[/red][purple]cacti[/purple][red]'[/red], [red]'[/red][purple]admin.*js[/purple][red]'[/red]
[red])[/red][red];[/red]

[black][b]my[/b][/black] [blue]$runs[/blue] = [blue]$ARGV[/blue][red][[/red][fuchsia]0[/fuchsia][red]][/red] ||= [fuchsia]1[/fuchsia][red];[/red]  [gray][i]#Allows debug and one email per day from cronjob[/i][/gray]

[url=http://perldoc.perl.org/functions/do.html][black][b]do[/b][/black][/url] [red]{[/red]
        [url=http://perldoc.perl.org/functions/open.html][black][b]open[/b][/black][/url] LOG, [red]"[/red][purple]<[/purple][red]"[/red], [red]"[/red][purple]/var/www/logs/error_log[/purple][red]"[/red][red];[/red]
        [blue]@fields[/blue] = <LOG>[red];[/red]
        [url=http://perldoc.perl.org/functions/close.html][black][b]close[/b][/black][/url] [red]([/red]LOG[red])[/red][red];[/red]

        [olive][b]foreach[/b][/olive] [black][b]my[/b][/black] [blue]$match[/blue] [red]([/red][blue]@matches[/blue][red])[/red] [red]{[/red]
                [blue]@logged[/blue] = [url=http://perldoc.perl.org/functions/grep.html][black][b]grep[/b][/black][/url][red]([/red][red]/[/red][purple][blue]$match[/blue][/purple][red]/[/red], [blue]@fields[/blue][red])[/red][red];[/red]
                [url=http://perldoc.perl.org/functions/push.html][black][b]push[/b][/black][/url] [red]([/red][blue]@log[/blue], [blue]@logged[/blue][red])[/red][red];[/red]
        [red]}[/red]

        [olive][b]foreach[/b][/olive] [black][b]my[/b][/black] [blue]$crap[/blue] [red]([/red][blue]@log[/blue][red])[/red] [red]{[/red]
                [black][b]my[/b][/black] [blue]@duh[/blue] = [url=http://perldoc.perl.org/functions/split.html][black][b]split[/b][/black][/url] [red]([/red][red]/[/red][purple] [/purple][red]/[/red], [blue]$crap[/blue][red])[/red][red];[/red]
                [blue]$duh[/blue][red][[/red][fuchsia]7[/fuchsia][red]][/red] =~ [red]s/[/red][purple]][/purple][red]/[/red][purple][/purple][red]/[/red][red];[/red]   [gray][i]#Removes ] from IP entry in log[/i][/gray]
                [blue]$list[/blue][red]{[/red][blue]$duh[/blue][red][[/red][fuchsia]7[/fuchsia][red]][/red][red])[/red][red]}[/red]=[fuchsia]1[/fuchsia][red];[/red]
        [red]}[/red]

        [url=http://perldoc.perl.org/functions/system.html][black][b]system[/b][/black][/url] [red]"[/red][purple]pfctl -t badhosts -T add [/purple][red]"[/red] . [url=http://perldoc.perl.org/functions/join.html][black][b]join[/b][/black][/url][red]([/red][red]"[/red][purple] [/purple][red]"[/red],[url=http://perldoc.perl.org/functions/keys.html][black][b]keys[/b][/black][/url] [blue]@list[/blue][red])[/red][red];[/red]

        [url=http://perldoc.perl.org/functions/sleep.html][black][b]sleep[/b][/black][/url] [fuchsia]10[/fuchsia][red];[/red]    [gray][i]# Using perl to run more often than 1 minute from cron[/i][/gray]
[red]}[/red] [olive][b]while[/b][/olive] [red]([/red][blue]$runs[/blue] > [fuchsia]0[/fuchsia][red])[/red][red];[/red]

- no need to declare array @crap if you are only going to use scalar $crap.
- no need for $duh when you can refer to $duh[7] directly anyway.
- no need for @uniq when you can rely on the unique nature of hash keys.
- you can use join instead of foreach loop to concatenate array contents.

I can't vouch for the script's continued effectiveness without some test input data.

Annihilannic.

 

[Annihilannic]

Incidentally, what does this do? I'm not familiar with the ||= syntax?

my $runs = $ARGV[0] ||= 1;  #Allows debug and one email per day from cronjob

Annihilannic.

 

[Kirsle]

Generally ||= isn't used in the way it was used there. I've never seen it used like that too.

||= usually means "if it doesn't contain a true value, set it to this", i.e.

$runs = shift @_; # who knows if $runs was set right by the caller

# give it a default value in case it doesn't have one
$runs ||= 1;

If $runs is 0, undef, or blank, it would be set to 1 in this case; otherwise it's left alone (if it has a true value).

The way it was used here looks like it would cause a syntax error. If not, it does the ||= on $ARGV[0], setting $ARGV[0] to 1 if it's currently false, and then returning 1 to $runs... although a simple || should work fine here, unless the setting of $ARGV[0] is important for something else.

Cuvou.com | My personal homepage

perl -e '$|=$i=1;print" oo\n<|>\n_|_";x:sleep$|;print"\b",$i++%2?"/":"_";goto x;'

 

[MrCBofBCinTX]

Annihilannic,
yes that looks much better!

I have seen ||= used to set default values many times.
I need this to set $runs =1 normally.

I am running this script with >/dev/null 2>&1 &
This way I don't see any output for all these repeats.

However, I do want to get a daily total to know what's up.

So I run script from crontab crapper.pl -0
This makes script run just once and I get one email with output.

However I did find an important error!
I had a really bad memory leak from not clearing these arrays and variables at end of each loop!

Fixed now. I will make some of the changes above ,too.

 

[Annihilannic]

Thanks for the explanation kirsle, makes perfect sense when you think of it in similar terms to $a += $b and the like.

While looking at that on the perlre man page I found a little gem which takes me back to this thread...

Annihilannic.

 

[MrCBofBCinTX]

I wasn't using ||= correctly in this script.

I don't usually use it to work with zero as a value.
So I changed to use $ARGV[0] ||= 0;

Now it works correctly. I was entering -0, 0 didn't work as I thought it should.

I couldn't get all of above to work exactly as shown, but I still cleaned up using suggestions.

Thanks