From dd5bb50fb53764ae69119468a7b6938cb40cc2b3 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 27 Mar 2018 14:28:06 +0000 Subject: [PATCH] Bug 14407: Follow up to add test case and clean noise Comment #28 has a /36 which is invalid CIDR. This triggers a crash and noise. This cleans up the crash and noise, and adds test cases to check for them. prove t/Auth.t -- before missing null case, and /36 case. -- after null case, and /36 with/without warnings. --- C4/Auth.pm | 19 +++++++++---------- t/Auth.t | 10 ++++++++-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 96f441e..f92e21a 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -2118,16 +2118,15 @@ Returns 1 if the remote address is in the provided ipset, or 0 otherwise. =cut sub in_ipset { - my ($ipset) = @_; - my @allowedipranges = split(' ', $ipset); - if (scalar @allowedipranges > 0) { - my @rangelist; - eval { @rangelist = Net::CIDR::range2cidr(@allowedipranges); }; return 0 if $@; - unless (Net::CIDR::cidrlookup($ENV{'REMOTE_ADDR'}, @rangelist)) { - return 0; - } - } - return 1; + my ($ipset) = @_; + my $result = 1; + my @allowedipranges = $ipset ? split(' ', $ipset) : (); + if (scalar @allowedipranges > 0) { + my @rangelist; + eval { @rangelist = Net::CIDR::range2cidr(@allowedipranges); }; return 0 if $@; + eval { $result = Net::CIDR::cidrlookup($ENV{'REMOTE_ADDR'}, @rangelist) } || ( $ENV{DEBUG} && warn 'cidrlookup failed for ' . join(' ',@rangelist) ); + } + return $result ? 1 : 0; } sub getborrowernumber { diff --git a/t/Auth.t b/t/Auth.t index 606a454..d904594 100644 --- a/t/Auth.t +++ b/t/Auth.t @@ -16,10 +16,10 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 10; +use Test::More tests => 13; +use Test::Warn; use C4::Auth qw / in_ipset /; -use warnings; $ENV{REMOTE_ADDR} = '192.168.1.30'; my $ipset1 = "192.168.1.30"; @@ -34,3 +34,9 @@ ok(in_ipset("192.168.1.10-192.168.1.30"), 'validly represented ip range with rem ok(in_ipset("127.0.0.1 192.168.1.30 192.168.2.10-192.168.2.25"), 'multiple ips and ranges, including the remote ip'); ok(!in_ipset("127.0.0.1 8.8.8.8 192.168.2.1/24 192.168.3.1/24 192.168.1.1-192.168.1.29"), "multiple ip and ip ranges, with the remote ip in none of them"); ok(in_ipset(""), "blank list given, no preference set - implies everything goes through."); +ok(in_ipset(), "no list given, no preference set - implies everything goes through."); +ok(in_ipset("192.168.1.1/36"), 'simple invalid ip range/36 with remote ip in it'); +$ENV{DEBUG} = 1; +warning_like { in_ipset("192.168.1.1/36") } + qr/cidrlookup failed for/, + 'noisy simple invalid ip range/36 with remote ip in it'; -- 2.1.4