From 13825aa55bf729ab9f668cff62ce5e210db9aa81 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Wed, 19 Jul 2023 15:31:33 +0200 Subject: [PATCH] Update resolver.defaults.retry - #1070 This commit updates "resolver.defaults.retry" profile parameter by making it more proper to its name: - Make it exclude the initial query. Note that the actual value of "resolver.defaults.retry" is silently incremented before being passed to Zonemaster-LDNS. This is needed because LDNS does count the initial query as a try with this parameter. - Update default values - Update documentation - Update unitary tests --- lib/Zonemaster/Engine/Nameserver.pm | 13 +++++++++---- lib/Zonemaster/Engine/Profile.pm | 9 +++++---- share/profile.json | 2 +- share/profile.yaml | 2 +- t/profiles.t | 8 ++++---- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/Zonemaster/Engine/Nameserver.pm b/lib/Zonemaster/Engine/Nameserver.pm index d6d5b2c32..f5962f8d8 100644 --- a/lib/Zonemaster/Engine/Nameserver.pm +++ b/lib/Zonemaster/Engine/Nameserver.pm @@ -217,7 +217,7 @@ sub _build_dns { $res->dnssec( 0 ); $res->edns_size( $UDP_EDNS_QUERY_DEFAULT ); - $res->retry( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retry} ) ); + $res->retry( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retry} ) + 1 ); $res->retrans( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retrans} ) ); $res->usevc( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.usevc} ) ); $res->igntc( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.igntc} ) ); @@ -452,7 +452,7 @@ sub _query { ); # Make sure we have a value for each flag - $flags{q{retry}} = $href->{q{retry}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retry} ); + $flags{q{retry}} = $href->{q{retry}} + 1 // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retry} ) + 1; $flags{q{retrans}} = $href->{q{retrans}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retrans} ); $flags{q{dnssec}} = $href->{q{dnssec}} // 0; $flags{q{usevc}} = $href->{q{usevc}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.usevc} ); @@ -537,7 +537,12 @@ sub _query { # Reset to defaults foreach my $flag ( keys %flags ) { - $self->dns->$flag( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.}.$flag ) ); + if ( $flag eq 'retry' ) { + $self->dns->$flag( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.}.$flag ) + 1 ); + } + else { + $self->dns->$flag( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.}.$flag ) ); + } } if ( $res ) { @@ -864,7 +869,7 @@ Set the timeout for the outgoing sockets. May or may not be observed by the unde =item retry -Set the number of times the query is tried. +Set the number of times a query is re-tried. This does not include the initial query. =item igntc diff --git a/lib/Zonemaster/Engine/Profile.pm b/lib/Zonemaster/Engine/Profile.pm index 97495b433..9d462ba1e 100644 --- a/lib/Zonemaster/Engine/Profile.pm +++ b/lib/Zonemaster/Engine/Profile.pm @@ -47,8 +47,8 @@ my %profile_properties_details = ( }, q{resolver.defaults.retry} => { type => q{Num}, - min => 1, - max => 255 + min => 0, + max => 254 }, q{resolver.defaults.usevc} => { type => q{Bool} @@ -681,8 +681,9 @@ This should almost certainly be kept false. =head2 resolver.defaults.retry -An integer between 1 and 255 inclusive. -The number of times a query is sent before we give up. Default 2. +An integer between 0 and 254. +The number of times a query is re-sent before giving up. This does not include the initial query. +Default 1. =head2 resolver.defaults.igntc diff --git a/share/profile.json b/share/profile.json index 2450d701d..2f0db9d63 100644 --- a/share/profile.json +++ b/share/profile.json @@ -20,7 +20,7 @@ "fallback" : true, "recurse" : false, "retrans" : 3, - "retry" : 2, + "retry" : 1, "usevc" : false, "timeout": 5 } diff --git a/share/profile.yaml b/share/profile.yaml index b423c32ee..92a3eb06a 100644 --- a/share/profile.yaml +++ b/share/profile.yaml @@ -20,7 +20,7 @@ resolver: igntc: false recurse: false retrans: 3 - retry: 2 + retry: 1 timeout: 5 usevc: false test_cases: diff --git a/t/profiles.t b/t/profiles.t index 8ac84b355..34b7a907a 100644 --- a/t/profiles.t +++ b/t/profiles.t @@ -295,8 +295,8 @@ subtest 'from_json() dies on illegal values' => sub { dies_ok { Zonemaster::Engine::Profile->from_json( '{"net":{"ipv4":1}}' ); } "checks type of net.ipv4"; dies_ok { Zonemaster::Engine::Profile->from_json( '{"net":{"ipv6":0}}' ); } "checks type of net.ipv6"; dies_ok { Zonemaster::Engine::Profile->from_json( '{"no_network":1}' ); } "checks type of no_network"; - dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retry":0}}}' ); } "checks lower bound of resolver.defaults.retry"; - dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retry":256}}}' ); } "checks upper bound of resolver.defaults.retry"; + dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retry":-1}}}' ); } "checks lower bound of resolver.defaults.retry"; + dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retry":255}}}' ); } "checks upper bound of resolver.defaults.retry"; dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retry":1.5}}}' ); } "checks type of resolver.defaults.retry"; dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retrans":0}}}' ); } "checks lower bound of resolver.defaults.retrans"; dies_ok { Zonemaster::Engine::Profile->from_json( '{"resolver":{"defaults":{"retrans":256}}}' ); } "checks upper bound of resolver.defaults.retrans"; @@ -546,8 +546,8 @@ subtest 'set() dies if the given property name is invalid' => sub { subtest 'set() dies on illegal value' => sub { my $profile = Zonemaster::Engine::Profile->new; - dies_ok { $profile->set( 'resolver.defaults.retry', 0 ); } 'checks lower bound of resolver.defaults.retry'; - dies_ok { $profile->set( 'resolver.defaults.retry', 256 ); } 'checks upper bound of resolver.defaults.retry'; + dies_ok { $profile->set( 'resolver.defaults.retry', -1 ); } 'checks lower bound of resolver.defaults.retry'; + dies_ok { $profile->set( 'resolver.defaults.retry', 255 ); } 'checks upper bound of resolver.defaults.retry'; dies_ok { $profile->set( 'resolver.defaults.retry', 1.5 ); } 'checks type of resolver.defaults.retry'; dies_ok { $profile->set( 'resolver.defaults.retrans', 0 ); } 'checks lower bound of resolver.defaults.retrans'; dies_ok { $profile->set( 'resolver.defaults.retrans', 256 ); } 'checks upper bound of resolver.defaults.retrans';