Usability improvements

Changes

@@ -1,5 +1,11 @@
 Revision history for Perl module Net::Netmask
 
+2.0001 2021-03-29
+ - BUGFIX: Return a netmask length of 0 whenever parsing an invalid
+   digit.
+ - Add "safe_new()" method to make it more clear what new2() does. Note
+   that this executes the same code as new2().
+
 2.0000 2021-03-29
 
  - SECURITY: IPv4 addresses with leading zeros are no longer allowed.

lib/Net/Netmask.pm

@@ -156,12 +156,17 @@ sub new {
         $bits = 128;
     }
 
-    $ibase = ascii2int( ( $base || '::' ), $proto ) unless defined $ibase;
+    $ibase = ascii2int( ( $base || '::' ), $proto ) unless (defined $ibase or $error);
     unless ( defined($ibase) || defined($error) ) {
         $error = "could not parse $net";
         $error .= " $mask" if $mask;
     }
 
+    if ($error) {
+        $ibase = 0;
+        $bits  = 0;
+    }
+
     $ibase = i_getnet_addr( $ibase, $bits, $proto );
 
     return bless {
@@ -185,6 +190,10 @@ sub i_getnet_addr {
 }
 
 sub new2 {
+    goto &safe_new;
+}
+
+sub safe_new {
     local ($debug) = 0;
     my $net = new(@_);
     return if $error;

lib/Net/Netmask.pod

@@ -6,10 +6,12 @@
 
  use Net::Netmask;
 
- $block = Net::Netmask->new(network block)
- $block = Net::Netmask->new(network block, netmask)
+ $block = Net::Netmask->safe_new(network block)
+ $block = Net::Netmask->safe_new(network block, netmask)
  $block = Net::Netmask->new2(network block)
  $block = Net::Netmask->new2(network block, netmask)
+ $block = Net::Netmask->new(network block)   # Don't use in new code!
+ $block = Net::Netmask->new(network block, netmask)   # Don't use in new code!
 
  print $block;                      # a.b.c.d/bits or 1:2:3::4/bits
  print $block->base()
@@ -128,13 +130,17 @@ A /128 block.
 
 =back
 
-There are two constructor methods: C<new> and C<new2>.  C<new2>
-differs from C<new> in that it will return undef for invalid netmasks,
+There are two constructor methods: C<new> and C<safe_new> (also known as C<new2>).
+
+C<safe_new> differs from C<new> in that it will return undef for invalid netmasks,
 while C<new> will return a netmask object even if the constructor
 could not figure out what the network block should be.
 
 With C<new>, the error string can be found as $block->{'ERROR'}.  With
-C<new2> the error can be found as Net::Netmask::errstr or $Net::Netmask::error.
+C<safe_new> the error can be found as Net::Netmask::errstr or $Net::Netmask::error.
+
+IMPORTANT: You want to use C<safe_new> or C<new2> (C<new2> is a synonym
+for C<new>) in new code!
 
 As of version 2.000, the following abbreviated IPv4 netblocks are not accepted
 by default, but can be accepted with options.
@@ -162,27 +168,23 @@ Always a /8 block.
 To accept these, you can call the constructor with a "shortnet" option set
 to a true value.  Example:
 
-  my $block = Net::Netmask->new2("216.240/16", shortnet => 1);
-
-or:
-
-  my $block = Net::Netmask->new("216.240/16", shortnet => 1);
+  my $block = Net::Netmask->safe_new("216.240/16", shortnet => 1);
 
 For compatibility with older codebases, it's also possible to change the
 default to use the old behavior.  To do this, you can set the
 C<$Net::Netmask::SHORTNET_DEFAULT> variable to a true value. It is recommended
 that this be done by localizing the variable. Example:
 
   local $Net::Netmask::SHORTNET_DEFAULT = 1
-  my $block = Net::Netmask->new2("216.240/16");
+  my $block = Net::Netmask->safe_new("216.240/16");
 
 Please be aware that there are security implications to this as other Perl
 modules, system libraries, or utilities may not parse these addresses the
 same way.  This is why the default was changed.
 
 For instance:
 
-  perl -MNet::Netmask -E "say Net::Netmask->new("10.20", shortnet => 1)"
+  perl -MNet::Netmask -E "say Net::Netmask->safe_new("10.20", shortnet => 1)"
 
 Will print "10.2.0.0/16".  However:
 
@@ -401,7 +403,7 @@ number of IPs within this netmask.
 
 For instance,
 
-  Net::Netmask->new( '10.0.0.0/24' )->split(2)
+  Net::Netmask->safe_new( '10.0.0.0/24' )->split(2)
 
 is equivilent to
 

t/badnets.t

@@ -270,6 +270,11 @@ my @tests = (
         error => qr/^could not parse /,
         type  => 'ambiguous',
     },
+    {
+        input => [ '2001::/129' ],
+        error => qr/^illegal number of bits/,
+        type  => 'bad mask',
+    },
 );
 
 foreach my $test (@tests) {
@@ -278,15 +283,21 @@ foreach my $test (@tests) {
     my $name  = ( join ', ', @{ $test->{input} } );
     my $type  = $test->{type};
 
-    my $result = Net::Netmask->new2(@$input);
-
+    my $result = Net::Netmask->safe_new(@$input);
     is( $result, undef, "$name $type" );
     like( Net::Netmask->errstr, $err, "$name errstr mismatch" );
+
+    warns { $result = Net::Netmask->new(@$input) };
+    if ($result->{PROTOCOL} eq 'IPv4') {
+        is( "$result", "0.0.0.0/0", "result is 0.0.0.0/0" );
+    } else {
+        is( "$result", "::/0", "result is 0.0.0.0/0" );
+    }
 }
 
 # test whois numbers with space between dash (valid!)
-ok( Net::Netmask->new2('209.157.64.0 - 209.157.95.255'), "whois with single space around dash" );
-ok( Net::Netmask->new2('209.157.64.0   -   209.157.95.255'),
+ok( Net::Netmask->safe_new('209.157.64.0 - 209.157.95.255'), "whois with single space around dash" );
+ok( Net::Netmask->safe_new('209.157.64.0   -   209.157.95.255'),
     "whois with mulitple spaces around dash" );
 
 done_testing;

t/netmasks.t

@@ -708,12 +708,23 @@ MAIN: {
     $block77->storeNetblock();
     is( findNetblock( "10.2.1.0", $table77 ), undef );
 
+    my $table77 = {};
+    my $block77 = Net::Netmask->safe_new("10.1.2.0/24");
+    $block77->storeNetblock();
+    is( findNetblock( "10.2.1.0", $table77 ), undef );
+
     $table77 = {};
     $block77 = Net::Netmask->new2("2001:db8:cccc:1111::/64");
     is( $Net::Netmask::error, undef, 'No error' );
     $block77->storeNetblock();
     is( findNetblock( "2001:db8:cccc:2222::", $table77 ), undef );
 
+    $table77 = {};
+    $block77 = Net::Netmask->safe_new("2001:db8:cccc:1111::/64");
+    is( $Net::Netmask::error, undef, 'No error' );
+    $block77->storeNetblock();
+    is( findNetblock( "2001:db8:cccc:2222::", $table77 ), undef );
+
     {
         my $bl = Net::Netmask->new("192.168.0.0/23");
         my @t  = (