From 2239b571019fbc76c04ec1a71cf090726c06babc Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 1 Aug 2024 19:08:29 -0400 Subject: [PATCH] dyndns2: Fix handling of multi-host response --- ChangeLog.md | 2 ++ ddclient.in | 83 ++++++++++++++++++++++++++++++---------------------- 2 files changed, 50 insertions(+), 35 deletions(-) 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/ddclient.in b/ddclient.in index 07b8ad6..1537efb 100755 --- a/ddclient.in +++ b/ddclient.in @@ -3876,56 +3876,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("empty response from $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); } }