zonemaster-engine icon indicating copy to clipboard operation
zonemaster-engine copied to clipboard

t/util.t gives uncaught error

Open matsduf opened this issue 3 years ago • 9 comments

This is run on current develop branch, commit https://github.com/zonemaster/zonemaster-engine/commit/e2242922938a16476a91bed44a84344a9de69006:

$ PERL_DL_NONLAZY=1 "/usr/local/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(1, 'inc', 'blib/lib', 'blib/arch')" t/util.t 
t/util.t .. 
ok 1 - use Zonemaster::Engine::Util;
ok 2 - An object of class 'Zonemaster::Engine::Nameserver' isa 'Zonemaster::Engine::Nameserver'
ok 3 - An object of class 'Zonemaster::Engine::Logger::Entry' isa 'Zonemaster::Engine::Logger::Entry'
ok 4 - An object of class 'Zonemaster::Engine::DNSName' isa 'Zonemaster::Engine::DNSName'
ok 5 - A reference of type 'HASH' isa 'HASH'
ok 6 - At least four keys
ok 7 - Expected content.
# Subtest: parse_hints()
    ok 1 - Cropped IANA hints
unable to parse RR string at /usr/local/lib/perl5/site_perl/Net/DNS/ZoneFile.pm line 534.
 at /usr/home/matsd/.cpanm/work/1669629248.58017/Zonemaster-Engine-v4.5.1/blib/lib/Zonemaster/Engine/Util.pm line 179.
    ok 2 - Syntax error
    ok 3 - Forbidden $TTL
    ok 4 - Forbidden $INCLUDE
    ok 5 - Forbidden $ORIGIN
    ok 6 - Forbidden $GENERATE
    ok 7 - Forbidden CH class
    ok 8 - Forbidden RR type SOA
    ok 9 - Forbidden RR type TXT
    ok 10 - Wrong owner name
    ok 11 - Missing address record
    ok 12 - Orphan A record
    ok 13 - Orphan AAAA record
    ok 14 - Missing NS
    1..14
ok 8 - parse_hints()
1..8
ok
All tests successful.
Files=1, Tests=8,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.52 cusr  0.09 csys =  0.65 CPU)
Result: PASS

matsduf avatar Nov 28 '22 10:11 matsduf

I guess it is OK, but is looks like a problem with testing. This will stick out when you install. Is it possible to capture it by running it in eval or something?

matsduf avatar Nov 28 '22 10:11 matsduf

I tried resolving this but somehow I didn't manage to get it to work. For some reason, I cannot catch the exception:

$ git diff
diff --git a/lib/Zonemaster/Engine/Util.pm b/lib/Zonemaster/Engine/Util.pm
index 945328eb..42e851fe 100644
--- a/lib/Zonemaster/Engine/Util.pm
+++ b/lib/Zonemaster/Engine/Util.pm
@@ -117,10 +117,13 @@ sub parse_hints {
         die "Forbidden directive \$$1\n";
     }

-    my $rrs = Net::DNS::ZoneFile->parse( \$string );
-    if ( !defined $rrs ) {
+    my $rrs;
+    eval {
+        $rrs = Net::DNS::ZoneFile->parse( \$string );
+    } or do {
+        my $eval_error = $@ || "error";
         die "Unable to parse root hints\n";
-    }
+    };

     my %ns;
     my %glue;

$ prove t/util.t
t/util.t .. 1/? unable to parse RR string at /usr/share/perl5/Net/DNS/ZoneFile.pm line 534.
 at /home/tgreen/projects/zonemaster/zonemaster-engine/lib/Zonemaster/Engine/Util.pm line 122.
t/util.t .. ok
All tests successful.
Files=1, Tests=5,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.19 cusr  0.01 csys =  0.23 CPU)
Result: PASS

@mattias-p Maybe you have a clue on this?

tgreenx avatar Jul 17 '24 14:07 tgreenx

Is it possible to capture it by running it in eval or something?

For some reason, I cannot catch the exception.

The problem is that Net::DNS::ZoneFile->parse() has a stinky way of reporting errors: it emits the error message as a warning.

Theoretically we could override $SIG{__WARN__} and capture the warning. But then we need to consider the possibility of regular warnings being emitted through this channel, and we don't want to silence those. Sadly we don't have a robust way of telling the error message apart from regular warnings.

mattias-p avatar Jul 23 '24 14:07 mattias-p

The problem is that Net::DNS::ZoneFile->parse() has a stinky way of reporting errors: it emits the error message as a warning.

Should we find some other tool?

matsduf avatar Jul 23 '24 14:07 matsduf

I believe I looked around for alternatives when I introduced this dependency, but that was a couple of years ago. Maybe there is something better now.

mattias-p avatar Jul 23 '24 14:07 mattias-p

I found https://metacpan.org/pod/DNS::ZoneParse but that one hasn't been updated since 2010. Maybe it's perfect.

mattias-p avatar Jul 23 '24 14:07 mattias-p

Maybe https://metacpan.org/pod/Parse::DNS::Zone could be something.

I'll also see if I can report this issue upstream. They probably want to add a more reasonable interface.

mattias-p avatar Jul 23 '24 15:07 mattias-p

I had a closer look at the documentation of Net::DNS::ZoneFile and it seems there is an improved interface already. We can probably just use that.

mattias-p avatar Jul 23 '24 15:07 mattias-p

In the following environment no warning is seen on the command line when t/util.t is run:

$ lsb_release -d
Description:	Ubuntu 22.04.4 LTS
$ perl -v | grep version
This is perl 5, version 34, subversion 0 (v5.34.0) built for x86_64-linux-gnu-thread-multi
$ perl -E 'use Net::DNS::ZoneFile; say $Net::DNS::ZoneFile::VERSION'
1855

matsduf avatar Jul 23 '24 15:07 matsduf