From 0ea2f065137d200485b54d139da956a6ff1178cd Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 13 Jun 2024 00:41:43 -0400 Subject: [PATCH] get_ip: Don't mutate `$arg` This makes log messages easier to understand. --- ddclient.in | 21 +++++++-------------- t/builtinfw_query.pl | 19 +++---------------- 2 files changed, 10 insertions(+), 30 deletions(-) diff --git a/ddclient.in b/ddclient.in index 108148c..cf85f6e 100755 --- a/ddclient.in +++ b/ddclient.in @@ -216,17 +216,16 @@ sub query_cisco { $if =~ s%\/%\\\/%g; # Protect special HTML characters (like '?') $if =~ s/([\?&= ])/sprintf("%%%02x", ord($1))/ge; - my $url = ($asa) - ? "https://$fw/exec/show%20interface%20$if" - : "http://$fw/level/1/exec/show/ip/interface/brief/$if/CR"; my $reply = geturl( - url => $url, + url => ($asa) + ? "https://$fw/exec/show%20interface%20$if" + : "http://$fw/level/1/exec/show/ip/interface/brief/$if/CR", login => opt('fw-login', $h), password => opt('fw-password', $h), ignore_ssl_option => 1, ssl_validate => opt('fw-ssl-validate', $h), ) // ''; - return ($url, $reply); + return $reply; } our %builtinfw = ( @@ -2774,7 +2773,6 @@ sub get_ip { warning("'%s' is not a valid IPv4 or IPv6 address", $ip // ''); $ip = undef; } - $arg = 'ip'; } elsif ($use eq 'if') { $ip = get_ip_from_interface($arg); } elsif ($use eq 'cmd') { @@ -2791,7 +2789,6 @@ sub get_ip { $skip //= $biw->{skip}; $url = $biw->{url}; } - $arg = $url; if ($url) { $reply = geturl( proxy => opt('proxy', $h), @@ -2811,7 +2808,7 @@ sub get_ip { $skip //= $fw->{'skip'}; if (defined(my $query = $fw->{'query'})) { $url = undef; - ($arg, $reply) = $query->($h); + $reply = $query->($h); } else { $url = "http://$url$fw->{'url'}" unless $url =~ /\//; } @@ -3165,7 +3162,6 @@ sub get_ipv4 { warning("'%s' is not a valid IPv4", $ipv4 // ''); $ipv4 = undef; } - $arg = 'ipv4'; # For debug message at end of function } elsif ($usev4 eq 'ifv4') { ## Obtain IPv4 address from interface mamed in "ifv4=" $ipv4 = get_ip_from_interface($arg, 4); @@ -3185,7 +3181,6 @@ sub get_ipv4 { warning("'--webv4=$url' is deprecated! $biw->{deprecated}") if $biw->{deprecated}; $skip //= $biw->{skip}; $url = $biw->{url}; - $arg = $url; } if ($url) { $reply = geturl( @@ -3211,7 +3206,7 @@ sub get_ipv4 { $skip //= $fw->{'skip'}; if (defined(my $query = $fw->{'queryv4'})) { $url = undef; - ($arg, $reply) = $query->($h); + $reply = $query->($h); } else { $url = "http://$url$fw->{'url'}" unless $url =~ /\//; } @@ -3268,7 +3263,6 @@ sub get_ipv6 { warning("'%s' is not a valid IPv6", $ipv6 // ''); $ipv6 = undef; } - $arg = 'ipv6'; # For debug message at end of function } elsif ($usev6 eq 'ifv6' || $usev6 eq 'if') { ## Obtain IPv6 address from interface mamed in "ifv6=" if ($usev6 eq 'if') { @@ -3302,7 +3296,6 @@ sub get_ipv6 { warning("'--webv6=$url' is deprecated! $biw->{deprecated}") if $biw->{deprecated}; $skip //= $biw->{skip}; $url = $biw->{url}; - $arg = $url; } if ($url) { $reply = geturl( @@ -3318,7 +3311,7 @@ sub get_ipv6 { $skip = opt('fwv6-skip', $h) // $fw->{'skip'}; if ($fw && defined(my $query = $fw->{'queryv6'})) { $skip //= $fw->{'skip'}; - ($arg, $reply) = $query->($h); + $reply = $query->($h); } else { warning("'--usev6=%s' is not implemented and does nothing", $usev6); } diff --git a/t/builtinfw_query.pl b/t/builtinfw_query.pl index 6151fc6..a784a33 100644 --- a/t/builtinfw_query.pl +++ b/t/builtinfw_query.pl @@ -1,33 +1,22 @@ use Test::More; -eval { require Test::MockModule; } or plan(skip_all => $@); SKIP: { eval { require Test::Warnings; } or skip($@, 1); } eval { require 'ddclient'; } or BAIL_OUT($@); -my $debug_msg; -my $module = Test::MockModule->new('ddclient'); -# Note: 'mock' is used instead of 'redefine' because 'redefine' is not available in the versions of -# Test::MockModule distributed with old Debian and Ubuntu releases. -$module->mock('debug', sub { - my $msg = sprintf(shift, @_); - return unless ($msg =~ qr/^get_ip(v[46])?:/); - BAIL_OUT("debug already called") if defined($debug_msg); - $debug_msg = $msg; -}); my $got_host; my $builtinfw = 't/builtinfw_query.pl'; $ddclient::builtinfw{$builtinfw} = { name => 'dummy device for testing', query => sub { ($got_host) = @_; - return ($got_host, "192.0.2.1 skip1 192.0.2.2 skip2 192.0.2.3"); + return '192.0.2.1 skip1 192.0.2.2 skip2 192.0.2.3'; }, queryv4 => sub { ($got_host) = @_; - return ($got_host, "192.0.2.4 skip1 192.0.2.5 skip3 192.0.2.6"); + return '192.0.2.4 skip1 192.0.2.5 skip3 192.0.2.6'; }, queryv6 => sub { ($got_host) = @_; - return ($got_host, "2001:db8::1 skip1 2001:db8::2 skip4 2001:db8::3"); + return '2001:db8::1 skip1 2001:db8::2 skip4 2001:db8::3'; }, }; %ddclient::builtinfw if 0; # suppress spurious warning "Name used only once: possible typo" @@ -81,12 +70,10 @@ for my $tc (@test_cases) { %{$tc->{cfgxtra}}, }; %ddclient::config if 0; # suppress spurious warning "Name used only once: possible typo" - undef($debug_msg); undef($got_host); my $got = $tc->{getip}($builtinfw, $h); is($got_host, $h, "host is passed through"); is($got, $tc->{want}, "returned IP matches"); - like($debug_msg, qr/\b\Q$h\E\b/, "returned arg is properly handled"); }; }