Advice for moving forward with modules

[newrisedesigns]

Thanks to tachyon's tutorial, I was able to come up with a few modules. This module is used to block access to some of my CGI programs based on IP address.

If you have the time, please take a look it. I'm updating some code for work, and I'd like all the core functions to be in a module. Before I get deeper into it, I'd like to know any pointers you'd be willing to offer. What's good to have in a module and what's not? How can I make OO modules and when should/shouldn't I use OO? Any insight or information you have is greatly appreciated.

#!/usr/bin/perl -w

package IPBlock;

use strict;
use Exporter;

use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);

use constant BLOCKED_LIST => './data/ip_block.dat';

$VERSION     = 1.00;
@ISA         = qw(Exporter);
@EXPORT      = qw(&checkIP);
@EXPORT_OK   = qw(&checkIP &blockIP &unblockIP &returnIP);
%EXPORT_TAGS = (DEFAULT => \@EXPORT,
        ALL    => [qw(&checkIP &blockIP &unblockIP &returnIP)]);

sub checkIP {

    # returns 1 if blocked
    # returns 0 if not blocked


    my $ip = shift;

    open(my $fh, BLOCKED_LIST) or die("Cannot open r BLOCKED_LIST $!")
+;
    flock($fh, 4);

    my $returnvalue = 0;

    while(<$fh>){
        chomp;
        if($_ =~ /\A$ip/){
            $returnvalue = 1;
            last;
        }        
    }

    close($fh);

    return $returnvalue;

}

sub blockIP {

    my $ip = shift;

    open(my $fh, '+<', BLOCKED_LIST) or die("Cannot open r/w BLOCKED_L
+IST $!");
    flock($fh, 2);
    my @block = <$fh>;

    chomp(@block);

    my $returnvalue = 1;

    if( grep(/\A$ip/, @block) ){
        $returnvalue = 0;
    }

    if($returnvalue == 1){
        push(@block, $ip);

        seek($fh, 0, 0);
        truncate($fh, 0);

        print $fh "$_\n" foreach(@block);
    }

    close($fh);

    return $returnvalue;

}

sub unblockIP {

    my $ip = shift;

    open(my $fh, '+<', BLOCKED_LIST) or die("Cannot open r/w BLOCKED_L
+IST $!");
    flock($fh, 2);
    my @block = <$fh>;

    chomp(@block);

    my $returnvalue = 0;

    if( grep(/^$ip/, @block) ){
        @block = grep(!/\A$ip/, @block);
        $returnvalue = 1;
    }

    seek($fh, 0, 0);
    truncate($fh, 0);

    print $fh "$_\n" foreach(@block);
    close($fh);

    return $returnvalue;

}
sub returnIP{

    open(my $fh, '+<', BLOCKED_LIST) or die("Cannot open r/w BLOCKED_L
+IST $!");
    flock($fh, 4);
    my @block = <$fh>;
    close($fh);
    chomp(@block);

    return \@block;

}

1;

Many thanks,
John J Reiser
newrisedesigns.com

[jeffa]

When you call die from inside a module, you should probably be using croak instead (and carp instead of warn). These two methods are available from Carp.pm. Try this little exercise, save the following as Foo.pm:

package Foo;

use Carp qw(croak);
use base qw(Exporter);
our @EXPORT_OK = qw(gonna_croak gonna_die);

sub gonna_die   { die "ya got me!" }
sub gonna_croak { croak "ya got me!" }

1;

Now use that module with the following one-liners:

$ perl -MFoo=gonna_die -le gonna_die
ya got me! at Foo.pm line 7.

$ perl -MFoo=gonna_croak -le gonna_croak
ya got me! at -e line 1

die reports the error from the actual line it was executed on, which might be many packages deep. croak, on the other hand, reports the error from the perpestive of the caller, which is usually more useful to the end user. The end user usually cares more about which line of their code caused the problem, not which line in the module it affected.

jeffa

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

[tachyon]

I like the full confession myself rather than croak. Sure you get some cake you may not want to eat but you get the whole cake!

use Carp;

hello();

sub hello { again() }
sub again { my_friend() }
sub my_friend { I() }
sub I { called() }
sub called { to_say()}
sub to_say { confess('Goodbye, I died') }

__DATA__
Goodbye, I died at script line 10
    main::to_say() called at script line 9 
    main::called() called at script line 8 
    main::I() called at script line 7
    main::my_friend() called at script line 6
    main::again() called at script line 5
    main::hello() called at script line 3

cheers

tachyon

s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

[chromatic]

The nice thing about using objects here is that you wouldn't have to hardcode the block list file. You could pass it to the constructor. As well, you could keep the contents of the file in the object and you wouldn't need to keep reading and writing the file.

Of course, if you want to have multiple processes consulting the file, you'd want to check if it was updated since you last read it, so keep a timestamp around as object data as well. Then you could open, lock, and write the file only if you changed the status of an IP address.

[mojotoad]

I don't know how many ip's you're anticipating to block...but if it gets very large you might consider using a DBM file for storing them -- that way you can randomly access them rather than grepping through a flat file.

Another thought is that you might want to also maintain a collection of block patterns so you can nail whole subnets or networks when desired.

Matt

[zengargoyle]

my favorite way to block people has lot's of watcher daemons sending block requests to a database, and a blocker daemon that updates firewall-filters at the border. no muss, no fuss. (it's a current project, can't 'ya tell? =)

Net::Netmask has everything you need for blocking whole ranges of ip's, but it might not be easy to serialize to/from a file.

if i were you, i would have done something with one of the Cache::Cache modules:

# untested, but close.
$blocked = Cache::FileCache->new(
  cache_root => '/wherever/blocked_hosts',
  default_expires_in => '30 days',
) or die "no cache!\n";

sub block {   # block($ip,$howlong)
  $blocked->set($_[0], 1, $_[1]);  
}

sub is_blocked {  # is_blocked($ip) -> undef or 1
  $blocked->get($_[0]);
}

sub unblock {  # unblock($ip)
  block($_[0], 'now');  # expire 'now', $ip go poof!
}


block('192.168.254.1');   # 30 days
block('192.168.254.12','6 months');
deny_access if is_blocked('192.168.254.1');
unblock('192.168.254.1');
block('192.168.254.13', 'never');  # block forever!

the auto expire can come in quite handy.

or if you can, a decent database, DBI and some SQL will let you easily access the blocked info from anywhere w/o worrying about locking and such, plus you can store the IP and Mask in the database as Integer values and then build a SELECT that will match ranges using SQL's math operators.

$is_blocked = $dbh->prepare('
SELECT 1 FROM blocks WHERE
( ? & mask ) == ( ip & mask )
');

deny_access if ($is_blocked->execute(ip2int('192.168.254.1')));

# blocks
#  ip integer,
#  mask integer
1.0.0.0  == 16777216
1.1.0.0  == 16842752
1.1.1.1  == 16843009
255.255.0.0 == 4294901760

16777216 & 4294901760 == 16777216
16842752 & 4294901760 == 16842752
16843009 & 4294901760 == 16842752

so... blocks
ip          mask
16842752    4294901760   (1.1.0.0 255.255.0.0)

will block everything under 1.1.0.0/16

or since TMTOWTDI i'm pretty sure CPAN has something that will tie a hash to a file/db

tie %blocks, 'Tie::Foo', 'the_block_file';
sub block { $blocks{shift} = 1 }
sub unblock { delete $blocks{shift} }
sub is_blocked { exists $blocks{shift} }