From 30b07558c51ca7aa2f45967cbae00b35338c3665 Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Mon, 3 Jul 2023 15:50:35 +0200 Subject: [PATCH 01/21] Fix --test documentation --- lib/Zonemaster/CLI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index a789910f..ea93103e 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -192,7 +192,7 @@ has 'test' => ( isa => 'ArrayRef', required => 0, documentation => __( -'Specify test to run. Should be either the name of a module, or the name of a module and the name of a method in that module separated by a "/" character (Example: "Basic/basic1"). The method specified must be one that takes a zone object as its single argument. This switch can be repeated.' +'Specify test to run. Should be either the name of a module, or the name of a module and the name of a method in that module separated by a "/" character (Example: "Basic/basic01"). This switch can be repeated.' ) ); From f2acc5b326fc40a42adaf0e63fadf37b04b92964 Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Mon, 3 Jul 2023 15:48:07 +0200 Subject: [PATCH 02/21] Make --test option case insensitive --- lib/Zonemaster/CLI.pm | 4 ++-- script/zonemaster-cli | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index ea93103e..2940d48d 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -192,7 +192,7 @@ has 'test' => ( isa => 'ArrayRef', required => 0, documentation => __( -'Specify test to run. Should be either the name of a module, or the name of a module and the name of a method in that module separated by a "/" character (Example: "Basic/basic01"). This switch can be repeated.' +'Specify test to run case-insensitively. Should be either the name of a module, or the name of a module and the name of a method in that module separated by a "/" character (Example: "Basic/basic01"). This switch can be repeated.' ) ); @@ -622,7 +622,7 @@ sub run { eval { if ( $self->test and @{ $self->test } > 0 ) { foreach my $t ( @{ $self->test } ) { - my ( $module, $method ) = split( '/', $t, 2 ); + my ( $module, $method ) = split( '/', lc($t), 2 ); if ( $method ) { Zonemaster::Engine->test_method( $module, $method, Zonemaster::Engine->zone( $domain ) ); } diff --git a/script/zonemaster-cli b/script/zonemaster-cli index 00fbe6bf..d00268f9 100755 --- a/script/zonemaster-cli +++ b/script/zonemaster-cli @@ -202,6 +202,7 @@ Limit the testing suite to run only the specified tests. This can be the name of a testing module, in which case all test cases from that module will be run, or the name of a module followed by a slash and the name of a test case (test case identifier) in that module. +This option is case-insensitive. =item --stop_level=LEVEL, --stop-level=LEVEL From a325d6804dc4dc20d3543bfce2461e87f571d87a Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 12 Sep 2023 15:31:57 +0200 Subject: [PATCH 03/21] Update option 'list_tests' Remove POD extraction. This is no longer useful after the changes in zonemaster-engine#1277. --- lib/Zonemaster/CLI.pm | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 0f03c19b..7c96f334 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -30,7 +30,7 @@ use Zonemaster::Engine; use Zonemaster::Engine::Exception; use Zonemaster::Engine::Logger::Entry; use Zonemaster::Engine::Translator; -use Zonemaster::Engine::Util qw[parse_hints pod_extract_for]; +use Zonemaster::Engine::Util qw[parse_hints]; use Zonemaster::Engine::Zone; use Zonemaster::LDNS; @@ -841,19 +841,8 @@ sub print_test_list { foreach my $module ( sort keys %methods ) { say $module; - my $doc = pod_extract_for( $module ); foreach my $method ( sort @{ $methods{$module} } ) { - printf " %${maxlen}s ", $method; - if ( $doc and $doc->{$method} ) { - print reflow_string( - $doc->{$method}, - optimum => 65, - maximum => 75, - indent1 => ' ', - indent2 => ( ' ' x ( $maxlen + 6 ) ) - ); - } - print "\n"; + printf " %${maxlen}s\n", $method; } print "\n"; } From bde27c54acfe17801402ee54c4c03bf6f21d2b52 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Wed, 18 Oct 2023 16:37:25 +0200 Subject: [PATCH 04/21] Make use of Zonemaster::Engine::Normalization module for domain name normalization --- lib/Zonemaster/CLI.pm | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 40887375..cfd96669 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -11,7 +11,7 @@ use 5.014002; use strict; use warnings; -use version; our $VERSION = version->declare( "v6.0.3" ); +use version; our $VERSION = version->declare( "v6.1.0" ); use Locale::TextDomain 'Zonemaster-CLI'; use Moose; @@ -26,13 +26,15 @@ use Scalar::Util qw[blessed]; use Socket qw[AF_INET AF_INET6]; use Text::Reflow qw[reflow_string]; use Try::Tiny; + +use Zonemaster::LDNS; use Zonemaster::Engine; use Zonemaster::Engine::Exception; +use Zonemaster::Engine::Normalization qw[normalize_name]; use Zonemaster::Engine::Logger::Entry; use Zonemaster::Engine::Translator; use Zonemaster::Engine::Util qw[parse_hints]; use Zonemaster::Engine::Zone; -use Zonemaster::LDNS; our %numeric = Zonemaster::Engine::Logger::Entry->levels; our $JSON = JSON::XS->new->allow_blessed->convert_blessed->canonical; @@ -552,16 +554,18 @@ sub run { } my ( $domain ) = @{ $self->extra_argv }; + if ( not $domain ) { die __( "Must give the name of a domain to test.\n" ); } - if ( $domain =~ m/\.\./i ) { - die __( "The domain name contains consecutive dots.\n" ); - } + ( my $errors, $domain ) = normalize_name( $domain ); - $domain =~ s/\.$// unless $domain eq '.'; - $domain = $self->to_idn( $domain ); + if ( scalar @$errors > 0 ) { + my $err; + $err .= $_->string . "\n" for @$errors; + die $err; + } if ( defined $self->hints ) { my $hints_data; From 4ab0aaffe3d2df8344d14d292bbdf3f8dee3cb40 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Wed, 18 Oct 2023 17:47:15 +0200 Subject: [PATCH 05/21] Make use of Zonemaster::Engine::Normalization module for name server name normalization - Use Zonemaster::Engine::Normalization::normalize_name() in function add_fake_delegation() - Add conditional check to argument '--ns' in case of several slash characters --- lib/Zonemaster/CLI.pm | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index cfd96669..18a9c53e 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -756,23 +756,24 @@ sub add_fake_delegation { foreach my $pair ( @{ $self->ns } ) { my ( $name, $ip ) = split( '/', $pair, 2 ); - if ( not $name ) { + if ( $pair =~ tr/\/// > 1 or not $name ) { say STDERR __( "--ns must be a name or a name/ip pair." ); exit( 1 ); } - if ( $name =~ m/\.\./i ) { - say STDERR __x( "The name of the nameserver '{nsname}' contains consecutive dots.", nsname => $name ); - exit ( 1 ); - } + ( my $errors, $name ) = normalize_name( $name ); - $name =~ s/\.$// unless $name eq '.'; + if ( scalar @$errors > 0 ) { + my $err = "Invalid name in --ns argument:\n" ; + $err .= "\t" . $_->string . "\n" for @$errors; + die $err; + } if ($ip) { - push @{ $data{ $self->to_idn( $name ) } }, $ip; + push @{ $data{ $name } }, $ip; } else { - push @ns_with_no_ip, $self->to_idn($name); + push @ns_with_no_ip, $name; } } foreach my $ns ( @ns_with_no_ip ) { From 2974ebc819813216911414b66f1d8d5a21e73d93 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Wed, 18 Oct 2023 17:49:08 +0200 Subject: [PATCH 06/21] Remove unused Zonemaster::CLI::to_idn() method This is no longer needed following the switch to Zonemaster::Engine::Normalization. --- lib/Zonemaster/CLI.pm | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 18a9c53e..dd096169 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -11,7 +11,7 @@ use 5.014002; use strict; use warnings; -use version; our $VERSION = version->declare( "v6.1.0" ); +use version; our $VERSION = version->declare( "v6.0.3" ); use Locale::TextDomain 'Zonemaster-CLI'; use Moose; @@ -821,22 +821,6 @@ sub print_spinner { return; } -sub to_idn { - my ( $self, $str ) = @_; - - if ( $str =~ m/^[[:ascii:]]+$/ ) { - return $str; - } - - if ( Zonemaster::LDNS::has_idn() ) { - return Zonemaster::LDNS::to_idn( decode( $self->encoding, $str ) ); - } - else { - say STDERR __( "Warning: Zonemaster::LDNS not compiled with IDN support, cannot handle non-ASCII names correctly." ); - return $str; - } -} - sub print_test_list { my %methods = Zonemaster::Engine->all_methods; my $maxlen = max map { From f538e67ad1a842649ceb2ca8bd04ec0c544d8565 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 21 Nov 2023 17:28:02 +0100 Subject: [PATCH 07/21] Update after review https://github.com/zonemaster/zonemaster-cli/pull/357#pullrequestreview-1737064804 --- lib/Zonemaster/CLI.pm | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index dd096169..5c8ef30c 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -562,9 +562,11 @@ sub run { ( my $errors, $domain ) = normalize_name( $domain ); if ( scalar @$errors > 0 ) { - my $err; - $err .= $_->string . "\n" for @$errors; - die $err; + my $error_message; + foreach my $err ( @$errors ) { + $error_message .= $err->string . "\n"; + } + die $error_message; } if ( defined $self->hints ) { @@ -764,18 +766,21 @@ sub add_fake_delegation { ( my $errors, $name ) = normalize_name( $name ); if ( scalar @$errors > 0 ) { - my $err = "Invalid name in --ns argument:\n" ; - $err .= "\t" . $_->string . "\n" for @$errors; - die $err; + my $error_message = "Invalid name in --ns argument:\n" ; + foreach my $err ( @$errors ) { + $error_message .= "\t" . $err->string . "\n"; + } + die $error_message; } - if ($ip) { + if ( $ip ) { push @{ $data{ $name } }, $ip; } else { push @ns_with_no_ip, $name; } } + foreach my $ns ( @ns_with_no_ip ) { if ( not exists $data{ $ns } ) { $data{ $ns } = undef; From 46d0d97b59c86a8f8a930055fd81486e74c5b5b5 Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Thu, 25 May 2023 09:31:52 +0200 Subject: [PATCH 08/21] Extend --test option to pass only a test case Following commands are identical: zonemaster-cli --test delegation/delegation01 zonemaster-cli --test delegation01 Passing a trailing '/' assumes a module should be run. Credits to @marc-vanderwal for most of this code. --- lib/Zonemaster/CLI.pm | 21 ++++++++++++++++----- script/zonemaster-cli | 6 ++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 40887375..22c84fd6 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -192,7 +192,7 @@ has 'test' => ( isa => 'ArrayRef', required => 0, documentation => __( -'Specify test to run case-insensitively. Should be either the name of a module, or the name of a module and the name of a method in that module separated by a "/" character (Example: "Basic/basic01"). This switch can be repeated.' +'Specify one or several test cases to be run. Should be the case-insensitive name of a test module and/or a test case (examples: "Delegation", "delegation01", "Delegation/delegation01"). This switch can be repeated.' ) ); @@ -621,13 +621,24 @@ sub run { # Actually run tests! eval { if ( $self->test and @{ $self->test } > 0 ) { + my $zone = Zonemaster::Engine->zone( $domain ); foreach my $t ( @{ $self->test } ) { - my ( $module, $method ) = split( '/', lc($t), 2 ); - if ( $method ) { - Zonemaster::Engine->test_method( $module, $method, Zonemaster::Engine->zone( $domain ) ); + # The case does not matter + $t = lc( $t ); + # Fully qualified module and test case (e.g. Example/example12) + if (my ($module, $method) = $t =~ m#^ ( [a-z]+ ) / ( [a-z]+[0-9]{2} ) $#ix) { + Zonemaster::Engine->test_method( $module, $method, $zone ); } + # Just a test case (e.g. example12). Note the different capturing order. + elsif (($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix) { + Zonemaster::Engine->test_method( $module, $method, $zone ); + } + # Just a module name (e.g. Example) or something invalid. + # TODO: in case of invalid input, print a proper error message + # suggesting to use --list-tests for valid choices. else { - Zonemaster::Engine->test_module( $module, $domain ); + $t =~ s/\/$//; + Zonemaster::Engine->test_module( $t, $zone ); } } } diff --git a/script/zonemaster-cli b/script/zonemaster-cli index 1ccffed9..e8d22799 100755 --- a/script/zonemaster-cli +++ b/script/zonemaster-cli @@ -196,12 +196,14 @@ Default: on Print all test cases listed in the test modules, then exit. -=item --test=MODULE, --test=MODULE/TESTCASE +=item --test=MODULE, --test=MODULE/TESTCASE, --test=TESTCASE Limit the testing suite to run only the specified tests. This can be the name of a testing module, in which case all test cases from that module will be run, or the name of a module followed by a slash and the -name of a test case (test case identifier) in that module. +name of a test case (test case identifier) in that module, or the name of the +test case. +Can be specified multiple times. This option is case-insensitive. =item --stop_level=LEVEL, --stop-level=LEVEL From ce9b025c4ab4a913c0c53e99353bd5e202290d1e Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Thu, 23 Nov 2023 14:31:22 +0100 Subject: [PATCH 09/21] Use UTF8 decoding for domain and nameserver names --- lib/Zonemaster/CLI.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 5c8ef30c..d05d2416 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -559,7 +559,7 @@ sub run { die __( "Must give the name of a domain to test.\n" ); } - ( my $errors, $domain ) = normalize_name( $domain ); + ( my $errors, $domain ) = normalize_name( decode( 'utf8', $domain ) ); if ( scalar @$errors > 0 ) { my $error_message; @@ -763,7 +763,7 @@ sub add_fake_delegation { exit( 1 ); } - ( my $errors, $name ) = normalize_name( $name ); + ( my $errors, $name ) = normalize_name( decode( 'utf8', $name ) ); if ( scalar @$errors > 0 ) { my $error_message = "Invalid name in --ns argument:\n" ; From dfd20dd7114b574a0023d8e117be440593b8c184 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Thu, 23 Nov 2023 16:23:15 +0100 Subject: [PATCH 10/21] Use Net::IP::XS to validate input IP address in undelegated scenarios --- lib/Zonemaster/CLI.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index d05d2416..9099fa81 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -774,7 +774,8 @@ sub add_fake_delegation { } if ( $ip ) { - push @{ $data{ $name } }, $ip; + $ip = new Net::IP::XS ( $ip ) or die Net::IP::XS::Error(); + push @{ $data{ $name } }, $ip->short; } else { push @ns_with_no_ip, $name; From 02a1bcabe1ef1c4bb668edfc2be6752d665f9364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 23 Nov 2023 16:35:56 +0100 Subject: [PATCH 11/21] use new method argstr instead of string --- lib/Zonemaster/CLI.pm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 40887375..30db21ba 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -511,11 +511,10 @@ sub run { if ( $self->raw ) { $prefix .= $entry->tag; - my $message = $entry->string; - $message =~ s/^[A-Z0-9:_]+//; # strip MODULE:TAG, they're coming in $prefix instead + my $message = $entry->argstr; my @lines = split /\n/, $message; - printf "%s%s %s\n", $prefix, ' ', shift @lines; + printf "%s%s %s\n", $prefix, ' ', @lines ? shift @lines : ''; for my $line ( @lines ) { printf "%s%s %s\n", $prefix, '>', $line; } From 451039a17a0f06de9784a67aa8f28727bb9b08e8 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Thu, 23 Nov 2023 17:56:21 +0100 Subject: [PATCH 12/21] Further improve input IP address validation in undelegated scenarios Update IP address validation methodology - It is now primarly based on regex checks, but also leverages Net::IP::XS::Error (when applicable) for more detailed error messages Add Readonly and Net::IP::XS as direct dependencies --- Makefile.PL | 2 ++ lib/Zonemaster/CLI.pm | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Makefile.PL b/Makefile.PL index ce0968fa..b3f26a4f 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -18,6 +18,8 @@ tests_recursive( 't' ); # (see Zonemaster::LDNS below) requires( + 'Readonly' => 0, + 'Net::IP::XS' => 0, 'JSON::XS' => 0, 'Locale::TextDomain' => 1.23, 'MooseX::Getopt' => 0, diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 9099fa81..ff306349 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -18,6 +18,7 @@ use Moose; with 'MooseX::Getopt::GLD' => { getopt_conf => [ 'pass_through' ] }; use Encode; +use Readonly; use File::Slurp; use JSON::XS; use List::Util qw[max]; @@ -26,6 +27,7 @@ use Scalar::Util qw[blessed]; use Socket qw[AF_INET AF_INET6]; use Text::Reflow qw[reflow_string]; use Try::Tiny; +use Net::IP::XS; use Zonemaster::LDNS; use Zonemaster::Engine; @@ -39,6 +41,9 @@ use Zonemaster::Engine::Zone; our %numeric = Zonemaster::Engine::Logger::Entry->levels; our $JSON = JSON::XS->new->allow_blessed->convert_blessed->canonical; +Readonly our $IPV4_RE => qr/^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$/; +Readonly our $IPV6_RE => qr/^[0-9a-f:]*:[0-9a-f:]+(:[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})?$/i; + STDOUT->autoflush( 1 ); has 'version' => ( @@ -774,8 +779,16 @@ sub add_fake_delegation { } if ( $ip ) { - $ip = new Net::IP::XS ( $ip ) or die Net::IP::XS::Error(); - push @{ $data{ $name } }, $ip->short; + my $net_ip = Net::IP::XS->new( $ip ); + if ( ( $ip =~ /($IPV4_RE)/ && Net::IP::XS::ip_is_ipv4( $ip ) ) + or + ( $ip =~ /($IPV6_RE)/ && Net::IP::XS::ip_is_ipv6( $ip ) ) + ) { + push @{ $data{ $name } }, $ip; + } + else { + die Net::IP::XS::Error() ? "Invalid IP address in --ns argument:\n\t". Net::IP::XS::Error() ."\n" : "Invalid IP address in --ns argument.\n"; + } } else { push @ns_with_no_ip, $name; From a2c380c8e6d29de215cd2803a86085b50601f964 Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Tue, 28 Nov 2023 15:28:59 +0100 Subject: [PATCH 13/21] Update after review --- lib/Zonemaster/CLI.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 22c84fd6..39d02ee1 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -192,7 +192,7 @@ has 'test' => ( isa => 'ArrayRef', required => 0, documentation => __( -'Specify one or several test cases to be run. Should be the case-insensitive name of a test module and/or a test case (examples: "Delegation", "delegation01", "Delegation/delegation01"). This switch can be repeated.' +'Specify test case to be run. Should be the case-insensitive name of a test module (e.g. "Delegation") and/or a test case (e.g. "Delegation/delegation01" or "delegation01"). This switch can be repeated.' ) ); @@ -637,8 +637,8 @@ sub run { # TODO: in case of invalid input, print a proper error message # suggesting to use --list-tests for valid choices. else { - $t =~ s/\/$//; - Zonemaster::Engine->test_module( $t, $zone ); + $t =~ s{/$}{}; + Zonemaster::Engine->test_module( $t, $domain ); } } } From c5e4ca5a4f278e7e2460369d7dfc8c53fa1d77aa Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 28 Nov 2023 17:34:46 +0100 Subject: [PATCH 14/21] Retain the possibility to start a Test Case disabled in the profile https://github.com/zonemaster/zonemaster-engine/pull/1294 removes the possibility for Zonemaster-Engine to start a Test Case which is explicitly disabled in its profile. This commit retains the possibility by letting Zonemaster-CLI directly modify the profile, and notifying the user with a notice message. --- lib/Zonemaster/CLI.pm | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 19694018..33981095 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -416,7 +416,6 @@ sub run { Zonemaster::Engine::Profile->effective->set( q{net.ipv6}, 0+$self->ipv6 ); } - if ( $self->dump_profile ) { do_dump_profile(); } @@ -637,10 +636,20 @@ sub run { $t = lc( $t ); # Fully qualified module and test case (e.g. Example/example12) if (my ($module, $method) = $t =~ m#^ ( [a-z]+ ) / ( [a-z]+[0-9]{2} ) $#ix) { + if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { + say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); + Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); + } + Zonemaster::Engine->test_method( $module, $method, $zone ); } # Just a test case (e.g. example12). Note the different capturing order. elsif (($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix) { + if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { + say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); + Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); + } + Zonemaster::Engine->test_method( $module, $method, $zone ); } # Just a module name (e.g. Example) or something invalid. From a66d5592b01b85833e718a96190c6b98fb305f6b Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Wed, 29 Nov 2023 11:24:56 +0100 Subject: [PATCH 15/21] Small refactoring in --test --- lib/Zonemaster/CLI.pm | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 33981095..86fee2ac 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -630,27 +630,21 @@ sub run { # Actually run tests! eval { if ( $self->test and @{ $self->test } > 0 ) { - my $zone = Zonemaster::Engine->zone( $domain ); foreach my $t ( @{ $self->test } ) { # The case does not matter $t = lc( $t ); - # Fully qualified module and test case (e.g. Example/example12) - if (my ($module, $method) = $t =~ m#^ ( [a-z]+ ) / ( [a-z]+[0-9]{2} ) $#ix) { + # Fully qualified module and test case (e.g. Example/example12), or just a test case (e.g. example12). Note the different capturing order. + my ( $module, $method ); + if ( ( ($module, $method) = $t =~ m#^ ( [a-z]+ ) / ( [a-z]+[0-9]{2} ) $#ix ) + or + ( ($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix ) ) + { if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); } - Zonemaster::Engine->test_method( $module, $method, $zone ); - } - # Just a test case (e.g. example12). Note the different capturing order. - elsif (($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix) { - if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { - say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); - Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); - } - - Zonemaster::Engine->test_method( $module, $method, $zone ); + Zonemaster::Engine->test_method( $module, $method, $domain ); } # Just a module name (e.g. Example) or something invalid. # TODO: in case of invalid input, print a proper error message From 08c2a77d574ff1ab5c8850f5f1eb6357d2e406b7 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Mon, 4 Dec 2023 13:30:01 +0100 Subject: [PATCH 16/21] Update after review - refactoring in --test Move input checks earlier in the code Add input check for multiple slash characters Refactoring --- lib/Zonemaster/CLI.pm | 60 +++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 86fee2ac..a12b6a5a 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -407,6 +407,39 @@ sub run { Zonemaster::Engine::Profile->effective->merge( $profile ); } + my @testing_suite; + if ( $self->test and @{ $self->test } > 0 ) { + foreach my $t ( @{ $self->test } ) { + # There should be at most one slash character + if ( $t =~ tr/\/// > 1 ) { + die __( "Error: --test must have at most one slash ('/') character: $t"); + } + + # The case does not matter + $t = lc( $t ); + + my ( $module, $method ); + # Fully qualified module and test case (e.g. Example/example12), or just a test case (e.g. example12). Note the different capturing order. + if ( ( ($module, $method) = $t =~ m#^ ( [a-z]+ ) / ( [a-z]+[0-9]{2} ) $#ix ) + or + ( ($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix ) ) + { + push @testing_suite, "$module/$method"; + if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { + # Actual forcing will be done later in the code, i.e. just before calling Zonemaster::Engine->test_method() + say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); + } + } + # Just a module name (e.g. Example) or something invalid. + # TODO: in case of invalid input, print a proper error message + # suggesting to use --list-tests for valid choices. + else { + $t =~ s{/$}{}; + push @testing_suite, $t; + } + } + } + # These two must come after any profile from command line has been loaded # to make any IPv4/IPv6 option override the profile setting. if ( defined ($self->ipv4) ) { @@ -630,28 +663,16 @@ sub run { # Actually run tests! eval { if ( $self->test and @{ $self->test } > 0 ) { - foreach my $t ( @{ $self->test } ) { - # The case does not matter - $t = lc( $t ); - # Fully qualified module and test case (e.g. Example/example12), or just a test case (e.g. example12). Note the different capturing order. - my ( $module, $method ); - if ( ( ($module, $method) = $t =~ m#^ ( [a-z]+ ) / ( [a-z]+[0-9]{2} ) $#ix ) - or - ( ($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix ) ) - { - if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { - say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); - Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); - } - + foreach my $t ( @testing_suite ) { + # Either a module/method, or just a module + my ( $module, $method ) = split('/', $t); + if ( $method ) { + # Force Engine to include the requested test case in its profile + Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); Zonemaster::Engine->test_method( $module, $method, $domain ); } - # Just a module name (e.g. Example) or something invalid. - # TODO: in case of invalid input, print a proper error message - # suggesting to use --list-tests for valid choices. else { - $t =~ s{/$}{}; - Zonemaster::Engine->test_module( $t, $domain ); + Zonemaster::Engine->test_module( $module, $domain ); } } } @@ -659,6 +680,7 @@ sub run { Zonemaster::Engine->test_zone( $domain ); } }; + if ( not $self->raw and not $self->json ) { if ( not $printed_something ) { say __( "Looks OK." ); From f730d14bb054a82757ce0480c79f2fa473560536 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Mon, 4 Dec 2023 15:38:06 +0100 Subject: [PATCH 17/21] Update Travis CI - Bumped Perl supported versions - Bumped distribution version - Added Mail::SPF dependency. Zonemaster-Engine relies on Mail::SPF and there is a bug preventing its installation from cpanm. See Zonemaster-Engine commit `3ac72b0c` and/or pull request for more information on this. --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index a806ae36..f09e930b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,12 +1,11 @@ -dist: focal +dist: jammy language: perl perl: - - "5.32" - - "5.30.2" + - "5.36" + - "5.30" - "5.26" - - "5.14.4" addons: apt: @@ -26,6 +25,7 @@ addons: - liblist-moreutils-perl - liblocale-msgfmt-perl - libmail-rfc822-address-perl + - libmail-spf-perl - libmodule-find-perl - libnet-ip-perl - libpod-coverage-perl From 5c687644660db9e374b937a1ab242f79d13ed990 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 5 Dec 2023 14:37:44 +0100 Subject: [PATCH 18/21] Update after review - more refactoring of --test Add input validation, based on the content of 'Zonemaster::Engine->all_methods' Construct the 'test_cases' profile property from the combination of all --test switches Refactoring --- lib/Zonemaster/CLI.pm | 56 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index a12b6a5a..c2f9d1e3 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -409,10 +409,14 @@ sub run { my @testing_suite; if ( $self->test and @{ $self->test } > 0 ) { + my %existing_tests = Zonemaster::Engine->all_methods; + my @existing_test_modules = keys %existing_tests; + my @existing_test_cases = map { @{ $existing_tests{$_} } } @existing_test_modules; + foreach my $t ( @{ $self->test } ) { # There should be at most one slash character if ( $t =~ tr/\/// > 1 ) { - die __( "Error: --test must have at most one slash ('/') character: $t"); + die __( "Error: Invalid input '$t' in --test. There must be at most one slash ('/') character.\n"); } # The case does not matter @@ -424,20 +428,54 @@ sub run { or ( ($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix ) ) { - push @testing_suite, "$module/$method"; - if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { - # Actual forcing will be done later in the code, i.e. just before calling Zonemaster::Engine->test_method() - say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); + # Check that test module exists + if ( grep( /^$module$/, map { lc($_) } @existing_test_modules ) ) { + # Check that test case exists + if ( grep( /^$method$/, @existing_test_cases ) ) { + if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { + # Actual forcing will be done later in the code + say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); + } + + push @testing_suite, "$module/$method"; + } + else { + die __( "Error: Unrecognized test case '$method' in --test. Use --list-tests for a list of valid choices.\n" ); + } + } + else { + die __( "Error: Unrecognized test module '$module' in --test. Use --list-tests for a list of valid choices.\n" ); } } # Just a module name (e.g. Example) or something invalid. - # TODO: in case of invalid input, print a proper error message - # suggesting to use --list-tests for valid choices. else { $t =~ s{/$}{}; - push @testing_suite, $t; + # Check that test module exists + if ( grep( /^$t$/, map { lc($_) } @existing_test_modules ) ) { + push @testing_suite, $t; + } + else { + die __( "Error: Invalid input '$t' in --test.\n" ); + } + } + } + + my @actual_test_cases; + foreach my $t ( @testing_suite ) { + # Either a module/method, or just a module + my ( $module, $method ) = split('/', $t); + if ( $method ) { + push @actual_test_cases, $method; + } + else { + # Get the test module with the right case + ( $module ) = grep { lc( $module ) eq lc( $_ ) } @existing_test_modules; + push @actual_test_cases, @{ $existing_tests{$module} }; } } + + # Force Engine to include all of the requested test cases in the profile + Zonemaster::Engine::Profile->effective->set( 'test_cases', [ @actual_test_cases ] ); } # These two must come after any profile from command line has been loaded @@ -667,8 +705,6 @@ sub run { # Either a module/method, or just a module my ( $module, $method ) = split('/', $t); if ( $method ) { - # Force Engine to include the requested test case in its profile - Zonemaster::Engine::Profile->effective->set( 'test_cases', [ "$method" ] ); Zonemaster::Engine->test_method( $module, $method, $domain ); } else { From 583157ec5c0b4c586c611daa9189802fd02307b2 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 5 Dec 2023 15:01:06 +0100 Subject: [PATCH 19/21] Fix newline for --test in human readable output --- lib/Zonemaster/CLI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index c2f9d1e3..bfee26b3 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -618,7 +618,7 @@ sub run { } ); - if ( $self->profile ) { + if ( $self->profile or $self->test ) { # Separate initialization from main output in human readable output mode print "\n" if $fh_diag eq *STDOUT; } From d640b9cde80b55c3ac8b500ecec8b00b0f3634ff Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 5 Dec 2023 17:55:21 +0100 Subject: [PATCH 20/21] Respect profile configuration when there are already test cases of a requested module in the profile --- lib/Zonemaster/CLI.pm | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index bfee26b3..e1531994 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -21,7 +21,7 @@ use Encode; use Readonly; use File::Slurp; use JSON::XS; -use List::Util qw[max]; +use List::Util qw[max uniq]; use POSIX qw[setlocale LC_MESSAGES LC_CTYPE]; use Scalar::Util qw[blessed]; use Socket qw[AF_INET AF_INET6]; @@ -432,11 +432,6 @@ sub run { if ( grep( /^$module$/, map { lc($_) } @existing_test_modules ) ) { # Check that test case exists if ( grep( /^$method$/, @existing_test_cases ) ) { - if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) { - # Actual forcing will be done later in the code - say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); - } - push @testing_suite, "$module/$method"; } else { @@ -460,22 +455,40 @@ sub run { } } - my @actual_test_cases; + # Start with all profile-enabled test cases + my @actual_test_cases = @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) }; + + # Derive test module from each profile-enabled test case + my %actual_test_modules; + foreach my $t ( @actual_test_cases ) { + my ( $module ) = $t =~ m#^ ( [a-z]+ ) [0-9]{2} $#ix; + $actual_test_modules{$module} = 1; + } + + # Check if more test cases need to be included in the profile foreach my $t ( @testing_suite ) { # Either a module/method, or just a module my ( $module, $method ) = split('/', $t); if ( $method ) { - push @actual_test_cases, $method; + # Test case in not already in the profile, we add it explicitly and notify the user + if ( not grep( /^$method$/, @actual_test_cases ) ) { + say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing..."); + push @actual_test_cases, $method; + } } else { - # Get the test module with the right case - ( $module ) = grep { lc( $module ) eq lc( $_ ) } @existing_test_modules; - push @actual_test_cases, @{ $existing_tests{$module} }; + # No test case from this module is already in the profile, we can add them all + if ( not grep( /^$module$/, keys %actual_test_modules ) ) { + # Get the test module with the right case + ( $module ) = grep { lc( $module ) eq lc( $_ ) } @existing_test_modules; + # No need to bother to check for duplicates here + push @actual_test_cases, @{ $existing_tests{$module} }; + } } } - # Force Engine to include all of the requested test cases in the profile - Zonemaster::Engine::Profile->effective->set( 'test_cases', [ @actual_test_cases ] ); + # Configure Engine to include all of the required test cases in the profile + Zonemaster::Engine::Profile->effective->set( 'test_cases', [ uniq sort @actual_test_cases ] ); } # These two must come after any profile from command line has been loaded From 2e3eebb50eb8e36e916ce94229f6d2cb667ef8a6 Mon Sep 17 00:00:00 2001 From: Mats Dufberg Date: Sun, 17 Mar 2024 23:15:38 +0100 Subject: [PATCH 21/21] Sets new version, updates changes and updates dependencies --- Changes | 11 +++++++++++ Makefile.PL | 4 ++-- lib/Zonemaster/CLI.pm | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/Changes b/Changes index c4ba9288..c300b29d 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,17 @@ Release history for Zonemaster component Zonemaster-CLI +v6.1.0 2024-03-18 (public release version) + + [Features] + - Extends "--test" option to allow passing of test case only (#333) + - Updates "--list_tests" option (#354) + - Adds input name normalization (#357) + + [Fixes] + - Fixes the "--raw" output (#360) + + v6.0.3 2023-09-08 (public fix version) [Fixes] diff --git a/Makefile.PL b/Makefile.PL index b3f26a4f..30381d3f 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -25,8 +25,8 @@ requires( 'MooseX::Getopt' => 0, 'Text::Reflow' => 0, 'Try::Tiny' => 0, - 'Zonemaster::LDNS' => 3.002000, # v3.2.0 - 'Zonemaster::Engine' => 4.007003, # v4.7.3 + 'Zonemaster::LDNS' => 4.000000, # v4.0.0 + 'Zonemaster::Engine' => 5.000000, # v5.0.0 ); # Make all platforms include inc/Module/Install/External.pm diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index e1531994..addbb171 100755 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -11,7 +11,7 @@ use 5.014002; use strict; use warnings; -use version; our $VERSION = version->declare( "v6.0.3" ); +use version; our $VERSION = version->declare( "v6.1.0" ); use Locale::TextDomain 'Zonemaster-CLI'; use Moose;