[#16336565] Replace Net::CIDR::Compare with Net::IP

Net::CIDR::Compare has buggy output with complaints about an invalid DESTROY method. Replacing it with Net::IP to eliminate concern that this could cause issues.
liblime · Jan 12, 2012 · 683a373 · 683a373
1 parent a9e70b9
commit 683a373
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 35 deletions.
diff --git a/C4/Auth.pm b/C4/Auth.pm
@@ -20,7 +20,6 @@ package C4::Auth;
 use strict;
 use warnings;
 use Carp;
-use Net::CIDR::Compare;
 use Digest::MD5 qw(md5_base64);
 use CGI::Session;
 
@@ -1593,26 +1592,11 @@ sub getborrowernumber {
 =cut
 
 sub IsIpInLibrary {
-    my ( $params ) = @_;
-
-    my $collection = Net::CIDR::Compare->new();
-
-    # Seed a list of CIDR blocks with the requestor's IP
+    my $params = shift;
     my $client_ip = $params->{ip}
                     // $ENV{HTTP_X_FORWARDED_FOR}
                     // $ENV{REMOTE_ADDR};
-    my $client_cidr = $collection->new_list();
-    $collection->add_range($client_cidr, $client_ip, 0);
-
-    # Seed a list of CIDR blocks with branch details
-    my $branch = GetBranchDetail($params->{branchcode});
-    croak "Cannot retrieve details for branch '$params->{branchcode}'" if not $branch;
-    my $library_cidr = $collection->new_list();
-    map {$collection->add_range($library_cidr, $_, 0)} split(/\n/, $branch->{branchip});
-
-    # Finally find the (possibly null) intersection and return our answer
-    $collection->process_intersection();
-    return ($collection->get_next_intersection_range()) ? 1 : 0;
+    return (C4::Branch::GetBranchByIp($client_ip) eq $params->{branchcode}) ? 1 : 0;
 }
 
 sub _uniq {

diff --git a/C4/Branch.pm b/C4/Branch.pm
@@ -20,7 +20,7 @@ use strict;
 use warnings;
 use Carp;
 use List::MoreUtils qw(uniq);
-use Net::CIDR::Compare;
+use Net::IP;
 
 require Exporter;
 
@@ -607,24 +607,25 @@ sub GetBranchCodeFromName {
 # 
 # Returns the branches.branchcode for the first match or undef if no match.
 sub GetBranchByIp {
-    my ($ip) = shift
+    my $client_ip = Net::IP->new(shift
                // $ENV{HTTP_X_FORWARDED_FOR}
-               // $ENV{REMOTE_ADDR};
-
-    my $collection = Net::CIDR::Compare->new();
-
-    my $client_cidr = $collection->new_list();
-    $collection->add_range($client_cidr, $ip, 0);
+               // $ENV{REMOTE_ADDR});
 
-    foreach my $branch (values %{GetBranches()}) {
-        my $library_cidr = $collection->new_list();
-        map {$collection->add_range($library_cidr, $_, 0)} split(/\n/, $branch->{branchip});
-        $collection->process_intersection();
-        return $branch->{branchcode} if ($collection->get_next_intersection_range());
-        $collection->remove_list($library_cidr);
+    for my $branch (values %{GetBranches()}) {
+        next unless $branch->{branchip};
+        my $raw = $branch->{branchip};
+        $raw =~ s{[^0-9./*]+}{\n}g;
+        for my $ip (split /\n/, $raw) {
+            $ip =~ s{^\*$}{0.0.0.0/0};
+            $ip =~ s{^(\d+)\.\*$}{$1.0.0.0/8};
+            $ip =~ s{^(\d+\.\d+)\.\*$}{$1.0.0/16};
+            $ip =~ s{^(\d+\.\d+\.\d+)\.\*$}{$1.0/24};
+            my $branch_ip = Net::IP->new($ip);
+            next unless $branch_ip;
+            return $branch->{branchcode} if $branch_ip->overlaps($client_ip);
+        }
     }
-
-    return;
+    return undef;
 }
 
 1;

diff --git a/Makefile.PL b/Makefile.PL
@@ -597,7 +597,7 @@ WriteMakefile(
                             'Mail::Sendmail'                   => 0.79,
                             'Memoize::Memcached'               => 0.03, # optional
                             'Modern::Perl'                     => 1.03,
-                            'Net::CIDR::Compare'               => 0.03,
+                            'Net::IP'                          => 1.25,
                             'Net::LDAP'                        => 0.33, # optional
                             'Net::LDAP::Filter'                => 0.14, # optional
                             'Net::Server::PreFork'             => 0.97,