diff --git a/ChangeLog.md b/ChangeLog.md index d0edbb0..13d1d10 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -148,6 +148,8 @@ repository history](https://github.com/ddclient/ddclient/commits/master). [#719](https://github.com/ddclient/ddclient/pull/719) * `gandi`: Fixed handling of error responses. [#721](https://github.com/ddclient/ddclient/pull/721) + * `dyndns2`: Fixed handling of responses for multi-host updates. + [#728](https://github.com/ddclient/ddclient/pull/728) ## 2023-11-23 v3.11.2 diff --git a/Makefile.am b/Makefile.am index eb2e77b..2ecf620 100644 --- a/Makefile.am +++ b/Makefile.am @@ -63,7 +63,6 @@ AM_PL_LOG_FLAGS = -Mstrict -w \ -MDevel::Autoflush handwritten_tests = \ t/builtinfw_query.pl \ - t/dnsexit2.pl \ t/get_ip_from_if.pl \ t/geturl_connectivity.pl \ t/geturl_response.pl \ @@ -75,6 +74,8 @@ handwritten_tests = \ t/is-and-extract-ipv6-global.pl \ t/logmsg.pl \ t/parse_assignments.pl \ + t/protocol_dnsexit2.pl \ + t/protocol_dyndns2.pl \ t/skip.pl \ t/ssl-validate.pl \ t/use_web.pl \ diff --git a/ddclient.in b/ddclient.in index cc637a5..42e0fd8 100755 --- a/ddclient.in +++ b/ddclient.in @@ -131,6 +131,7 @@ my $daemon_default = ($programd =~ /d$/) ? interval('5m') : undef; # Current Logger instance. To push a context prefix onto the context stack: # local _l = pushlogctx('additional context goes here'); our $_l = ddclient::Logger->new(); +our @_test_headers; $ENV{'PATH'} = (exists($ENV{PATH}) ? "$ENV{PATH}:" : "") . "/sbin:/usr/sbin:/bin:/usr/bin:/etc:/usr/lib:"; @@ -159,6 +160,14 @@ my $daemon; # Control how many times warning message logged for invalid IP addresses my (%warned_ip, %warned_ipv4, %warned_ipv6); +sub repr { + my $vals = @_ % 2 ? [shift] : []; + my %opts = @_; + my $d = Data::Dumper->new($vals)->Sortkeys(1)->Terse(!exists($opts{Names}))->Useqq(1); + $d->$_($opts{$_}) for keys(%opts); + return $d->Dump(); +} + sub T_ANY { 'any' } sub T_STRING { 'string' } sub T_EMAIL { 'e-mail address' } @@ -2674,7 +2683,6 @@ sub geturl { my $timeout = opt('timeout'); my $redirect = opt('redirect'); my @curlopt = (); - my @header_lines = (); ## canonify use_ssl, proxy and url if ($url =~ /^https:/) { @@ -2721,11 +2729,8 @@ sub geturl { push(@curlopt, "user=\"".escape_curl_param("${login}:${password}").'"') if (defined($login) && defined($password)); push(@curlopt, "proxy=\"".escape_curl_param("${protocol}://${proxy}").'"') if defined($proxy); push(@curlopt, "url=\"".escape_curl_param("${protocol}://${server}/${url}").'"'); - - # Each header line is added individually - @header_lines = ref($headers) eq 'ARRAY' ? @$headers : split('\n', $headers); - $_ = "header=\"".escape_curl_param($_).'"' for (@header_lines); - push(@curlopt, @header_lines); + push(@curlopt, map('header="' . escape_curl_param($_) . '"', @_test_headers, + ref($headers) eq 'ARRAY' ? @$headers : split('\n', $headers))); # Add in the data if any was provided (for POST/PATCH) push(@curlopt, "data=\"".escape_curl_param(${data}).'"') if ($data); @@ -3363,10 +3368,9 @@ sub group_hosts_by { @attrs = sort(keys(%attrs)); my %groups; my %cfgs; - my $d = Data::Dumper->new([])->Indent(0)->Sortkeys(1)->Terse(1)->Useqq(1); for my $h (@$hosts) { my %cfg = map({ ($_ => $config{$h}{$_}); } grep(exists($config{$h}{$_}), @attrs)); - my $sig = $d->Reset()->Values([\%cfg])->Dump(); + my $sig = repr(\%cfg, Indent => 0); push(@{$groups{$sig}}, $h); $cfgs{$sig} = \%cfg; } @@ -3819,17 +3823,17 @@ EoEXAMPLE ###################################################################### sub nic_dyndns2_update { my %errors = ( - 'badauth' => 'Bad authorization (username or password)', - 'badsys' => 'The system parameter given was not valid', - 'notfqdn' => 'A Fully-Qualified Domain Name was not provided', - 'nohost' => 'The hostname specified does not exist in the database', - '!yours' => 'The hostname specified exists, but not under the username currently being used', - '!donator' => 'The offline setting was set, when the user is not a donator', - '!active' => 'The hostname specified is in a Custom DNS domain which has not yet been activated.', - 'abuse', => 'The hostname specified is blocked for abuse; you should receive an email notification which provides an unblock request link. More info can be found on https://www.dyndns.com/support/abuse.html', - 'numhost' => 'System error: Too many or too few hosts found. Contact support@dyndns.org', - 'dnserr' => 'System error: DNS error encountered. Contact support@dyndns.org', - 'nochg' => 'No update required; unnecessary attempts to change to the current address are considered abusive', + 'badauth' => 'Bad authorization (username or password)', + 'badsys' => 'The system parameter given was not valid', + 'notfqdn' => 'A Fully-Qualified Domain Name was not provided', + 'nohost' => 'The hostname specified does not exist in the database', + '!yours' => 'The hostname specified exists, but not under the username currently being used', + '!donator' => 'The offline setting was set, when the user is not a donator', + '!active' => 'The hostname specified is in a Custom DNS domain which has not yet been activated.', + 'abuse' => 'The hostname specified is blocked for abuse; you should receive an email notification which provides an unblock request link. More info can be found on https://www.dyndns.com/support/abuse.html', + 'numhost' => 'System error: Too many or too few hosts found. Contact support@dyndns.org', + 'dnserr' => 'System error: DNS error encountered. Contact support@dyndns.org', + 'nochg' => 'No update required; unnecessary attempts to change to the current address are considered abusive', ); my @group_by_attrs = qw( backupmx @@ -3876,56 +3880,69 @@ sub nic_dyndns2_update { # updates too frequent) so the body of the response must also be checked. (my $body = $reply) =~ s/^.*?\n\n//s; my @reply = split(qr/\n/, $body); - if (!@reply) { - failed("Could not connect to $groupcfg{'server'}"); - next; - } # From : # # If updating multiple hostnames, hostname-specific return codes are given one per line, # in the same order as the hostnames were specified. Return codes indicating a failure # with the account or the system are given only once. # - # TODO: There is no mention of what happens if multiple IP addresses are supplied (e.g., - # IPv4 and IPv6) for a host. If one address fails to update and the other doesn't, is that - # one error status line? An error status line and a success status line? Or is an update - # considered to be all-or-nothing and the status applies to the operation as a whole? If - # the IPv4 address changes but not the IPv6 address does that result in a status of "good" - # because the set of addresses for a host changed even if a subset did not? + # If there is only one result for multiple hosts, this function assumes the one result + # applies to all hosts. According to the documentation quoted above this should only + # happen if the result is a failure. In case there is a single successful result, this + # code applies the success to all hosts (with a warning) to maximize potential + # compatibility with all DynDNS-like services. If there are zero results, or two or more + # results, any host without a corresponding result line is treated as a failure. # - # TODO: The logic below applies the last line's status to all hosts. Change it to apply - # each status to its corresponding host. - for my $line (@reply) { + # TODO: The DynDNS documentation does not mention what happens if multiple IP addresses are + # supplied (e.g., IPv4 and IPv6) for a host. If one address fails to update and the other + # doesn't, is that one error status line? An error status line and a success status line? + # Or is an update considered to be all-or-nothing and the status applies to the collection + # of addresses as a whole? If the IPv4 address changes but not the IPv6 address does that + # result in a status of "good" because the set of addresses for a host changed even if a + # subset did not? + my @statuses = map({ (my $l = $_) =~ s/ .*$//; $l; } @reply); + if (@statuses < @hosts && @statuses == 1) { + warning("service returned one successful result for multiple hosts; " . + "assuming the one success is intended to apply to all hosts") + if $statuses[0] =~ qr/^(?:good|nochg)$/; + @statuses = ($statuses[0]) x @hosts; + } + for (my $i = 0; $i < @hosts; ++$i) { + my $h = $hosts[$i]; + local $_l = $_l->{parent}; $_l = pushlogctx($h); + my $status = $statuses[$i] // 'unknown'; + if ($status eq 'nochg') { + warning("$status: $errors{$status}"); + $status = 'good'; + } + $config{$h}{'status-ipv4'} = $status if $ipv4; + $config{$h}{'status-ipv6'} = $status if $ipv6; + if ($status ne 'good') { + if (exists($errors{$status})) { + failed("$status: $errors{$status}"); + } elsif ($status eq 'unknown') { + failed('server did not return a success/fail result; assuming failure'); + } else { + # This case can only happen if there is a corresponding status line for this + # host or there was only one status line for all hosts. + failed("unexpected status: " . ($reply[$i] // $reply[0])); + } + next; + } # The IP address normally comes after the status, but we ignore it. We could compare # it with the expected address and mark the update as failed if it differs, but (1) # some services do not return the IP; and (2) comparison is brittle (e.g., # 192.000.002.001 vs. 192.0.2.1) and false errors could cause high load on the service # (an update attempt every min-error-interval instead of every max-interval). - (my $status = $line) =~ s/ .*$//; - if ($status eq 'nochg') { - warning("$status: $errors{$status}"); - $status = 'good'; - } - for my $h (@hosts) { - $config{$h}{'status-ipv4'} = $status if $ipv4; - $config{$h}{'status-ipv6'} = $status if $ipv6; - } - if ($status ne 'good') { - if (exists($errors{$status})) { - failed("$status: $errors{$status}"); - } else { - failed("unexpected status: $line"); - } - next; - } - for my $h (@hosts) { - $config{$h}{'ipv4'} = $ipv4 if $ipv4; - $config{$h}{'ipv6'} = $ipv6 if $ipv6; - $config{$h}{'mtime'} = $now; - } + $config{$h}{'ipv4'} = $ipv4 if $ipv4; + $config{$h}{'ipv6'} = $ipv6 if $ipv6; + $config{$h}{'mtime'} = $now; success("IPv4 address set to $ipv4") if $ipv4; success("IPv6 address set to $ipv6") if $ipv6; } + warning("unexpected extra lines after per-host update status lines:\n" . + join("\n", @reply[@hosts..$#reply])) + if (@reply > @hosts); } } diff --git a/t/dnsexit2.pl b/t/protocol_dnsexit2.pl similarity index 100% rename from t/dnsexit2.pl rename to t/protocol_dnsexit2.pl diff --git a/t/protocol_dyndns2.pl b/t/protocol_dyndns2.pl new file mode 100644 index 0000000..055d9dc --- /dev/null +++ b/t/protocol_dyndns2.pl @@ -0,0 +1,303 @@ +use Test::More; +use Scalar::Util qw(blessed); +use MIME::Base64; +eval { require ddclient::Test::Fake::HTTPD; } or plan(skip_all => $@); +SKIP: { eval { require Test::Warnings; } or skip($@, 1); } +eval { require 'ddclient'; } or BAIL_OUT($@); + +my $httpd = ddclient::Test::Fake::HTTPD->new(); +$httpd->run(sub { + my ($req) = @_; + diag('=============================================================================='); + diag("Test server received request:\n" . $req->as_string()); + my $headers = ['content-type' => 'text/plain; charset=utf-8']; + my $wantauthn = 'Basic ' . encode_base64('username:password', ''); + return [401, [@$headers, 'www-authenticate' => 'Basic realm="realm", charset="UTF-8"'], + ['authentication required']] if ($req->header('authorization') // '') ne $wantauthn; + return [400, $headers, ['invalid method: ' . $req->method()]] if $req->method() ne 'GET'; + return [400, $headers, ['unexpected request: ' . $req->uri() . "\n", + 'want: ' . $req->header('want-req')]] + if $req->uri() ne $req->header('want-req'); + return [200, $headers, [map("$_\n", $req->header('line'))]]; +}); +diag("started IPv4 HTTP server running at " . $httpd->endpoint()); + +{ + package Logger; + BEGIN { push(our @ISA, qw(ddclient::Logger)); } + sub new { + my ($class, $parent) = @_; + my $self = $class->SUPER::new(undef, $parent); + $self->{logs} = []; + return $self; + } + sub _log { + my ($self, $args) = @_; + push(@{$self->{logs}}, $args) + if ($args->{label} // '') =~ qr/^(?:WARNING|FATAL|SUCCESS|FAILED)$/; + return $self->SUPER::_log($args); + } +} + +my @test_cases = ( + { + desc => 'IPv4, single host, good', + cfg => {h1 => {wantipv4 => '192.0.2.1'}}, + resp => ['good'], + wantquery => 'hostname=h1&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + ], + }, + { + desc => 'IPv4, single host, nochg', + cfg => {h1 => {wantipv4 => '192.0.2.1'}}, + resp => ['nochg'], + wantquery => 'hostname=h1&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'WARNING', ctx => ['h1'], msg => qr/nochg/}, + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + ], + }, + { + desc => 'IPv4, single host, bad', + cfg => {h1 => {wantipv4 => '192.0.2.1'}}, + resp => ['nohost'], + wantquery => 'hostname=h1&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'nohost'}, + }, + wantlogs => [ + {label => 'FAILED', ctx => ['h1'], msg => qr/nohost/}, + ], + }, + { + desc => 'IPv4, single host, unexpected', + cfg => {h1 => {wantipv4 => '192.0.2.1'}}, + resp => ['WAT'], + wantquery => 'hostname=h1&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'WAT'}, + }, + wantlogs => [ + {label => 'FAILED', ctx => ['h1'], msg => qr/unexpected.*WAT/}, + ], + }, + { + desc => 'IPv4, multiple hosts, multiple good', + cfg => { + h1 => {wantipv4 => '192.0.2.1'}, + h2 => {wantipv4 => '192.0.2.1'}, + }, + resp => [ + 'good 192.0.2.1', + 'good', + ], + wantquery => 'hostname=h1,h2&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + {label => 'SUCCESS', ctx => ['h2'], msg => qr/IPv4/}, + ], + }, + { + desc => 'IPv4, multiple hosts, mixed success', + cfg => { + h1 => {wantipv4 => '192.0.2.1'}, + h2 => {wantipv4 => '192.0.2.1'}, + h3 => {wantipv4 => '192.0.2.1'}, + }, + resp => [ + 'good', + 'nochg', + 'dnserr', + ], + wantquery => 'hostname=h1,h2,h3&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h3 => {'status-ipv4' => 'dnserr'}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + {label => 'WARNING', ctx => ['h2'], msg => qr/nochg/}, + {label => 'SUCCESS', ctx => ['h2'], msg => qr/IPv4/}, + {label => 'FAILED', ctx => ['h3'], msg => qr/dnserr/}, + ], + }, + { + desc => 'IPv6, single host, good', + cfg => {h1 => {wantipv6 => '2001:db8::1'}}, + resp => ['good'], + wantquery => 'hostname=h1&myip=2001:db8::1', + wantstatus => { + h1 => {'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv6/}, + ], + }, + { + desc => 'IPv4 and IPv6, single host, good', + cfg => {h1 => {wantipv4 => '192.0.2.1', wantipv6 => '2001:db8::1'}}, + resp => ['good'], + wantquery => 'hostname=h1&myip=192.0.2.1,2001:db8::1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', + 'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', + 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv6/}, + ], + }, + { + desc => 'excess status line', + cfg => { + h1 => {wantipv4 => '192.0.2.1'}, + h2 => {wantipv4 => '192.0.2.1'}, + }, + resp => [ + 'good', + 'good', + 'WAT', + ], + wantquery => 'hostname=h1,h2&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + {label => 'SUCCESS', ctx => ['h2'], msg => qr/IPv4/}, + {label => 'WARNING', ctx => ['h1,h2'], msg => qr/unexpected.*\nWAT$/}, + ], + }, + { + desc => 'multiple hosts, single failure', + cfg => { + h1 => {wantipv4 => '192.0.2.1'}, + h2 => {wantipv4 => '192.0.2.1'}, + }, + resp => ['abuse'], + wantquery => 'hostname=h1,h2&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'abuse'}, + h2 => {'status-ipv4' => 'abuse'}, + }, + wantlogs => [ + {label => 'FAILED', ctx => ['h1'], msg => qr/abuse/}, + {label => 'FAILED', ctx => ['h2'], msg => qr/abuse/}, + ], + }, + { + desc => 'multiple hosts, single success', + cfg => { + h1 => {wantipv4 => '192.0.2.1'}, + h2 => {wantipv4 => '192.0.2.1'}, + }, + resp => ['good'], + wantquery => 'hostname=h1,h2&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + }, + wantlogs => [ + {label => 'WARNING', ctx => ['h1,h2'], msg => qr//}, + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + {label => 'SUCCESS', ctx => ['h2'], msg => qr/IPv4/}, + ], + }, + { + desc => 'multiple hosts, fewer results', + cfg => { + h1 => {wantipv4 => '192.0.2.1'}, + h2 => {wantipv4 => '192.0.2.1'}, + h3 => {wantipv4 => '192.0.2.1'}, + }, + resp => [ + 'good', + 'nochg', + ], + wantquery => 'hostname=h1,h2,h3&myip=192.0.2.1', + wantstatus => { + h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, + h3 => {'status-ipv4' => 'unknown'}, + }, + wantlogs => [ + {label => 'SUCCESS', ctx => ['h1'], msg => qr/IPv4/}, + {label => 'WARNING', ctx => ['h2'], msg => qr/nochg/}, + {label => 'SUCCESS', ctx => ['h2'], msg => qr/IPv4/}, + {label => 'FAILED', ctx => ['h3'], msg => qr/assuming failure/}, + ], + }, +); + +for my $tc (@test_cases) { + diag('=============================================================================='); + diag("Starting test: $tc->{desc}"); + diag('=============================================================================='); + local $ddclient::globals{debug} = 1; + local $ddclient::globals{verbose} = 1; + my $l = Logger->new($ddclient::_l); + local %ddclient::config; + $ddclient::config{$_} = { + login => 'username', + password => 'password', + server => $httpd->endpoint(), + script => '/nic/update', + %{$tc->{cfg}{$_}}, + } for keys(%{$tc->{cfg}}); + { + local @ddclient::_test_headers = ( + "want-req: /nic/update?$tc->{wantquery}", + map("line: $_", @{$tc->{resp}}), + ); + local $ddclient::_l = $l; + ddclient::nic_dyndns2_update(sort(keys(%{$tc->{cfg}}))); + } + # These are the properties in %ddclient::config to check against $tc->{wantstatus}. + my %statuskeys = map(($_ => undef), qw(atime ip ipv4 ipv6 mtime status status-ipv4 status-ipv6 + wantip wantipv4 wantipv6 wtime)); + my %gotstatus; + for my $h (keys(%ddclient::config)) { + $gotstatus{$h} = {map(($_ => $ddclient::config{$h}{$_}), + grep(exists($statuskeys{$_}), keys(%{$ddclient::config{$h}})))}; + } + is_deeply(\%gotstatus, $tc->{wantstatus}, "$tc->{desc}: status") + or diag(ddclient::repr(\%ddclient::config, Names => ['*ddclient::config'])); + $tc->{wantlogs} //= []; + subtest("$tc->{desc}: logs" => sub { + my @got = @{$l->{logs}}; + my @want = @{$tc->{wantlogs}}; + for my $i (0..$#want) { + last if $i >= @got; + my $got = $got[$i]; + my $want = $want[$i]; + subtest("log $i" => sub { + is($got->{label}, $want->{label}, "label matches"); + is_deeply($got->{ctx}, $want->{ctx}, "context matches"); + like($got->{msg}, $want->{msg}, "message matches"); + }) or diag(ddclient::repr(Values => [$got, $want], Names => ['*got', '*want'])); + } + my @unexpected = @got[@want..$#got]; + ok(@unexpected == 0, "no unexpected logs") + or diag(ddclient::repr(\@unexpected, Names => ['*unexpected'])); + my @missing = @want[@got..$#want]; + ok(@missing == 0, "no missing logs") + or diag(ddclient::repr(\@missing, Names => ['*missing'])); + }); +} + +done_testing();