From a136ba4cdc89226327671ce1d2c0afe17bdaa667 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 18 Aug 2024 00:26:49 -0400 Subject: [PATCH 01/32] tests: Fix `ssl` option for `dnsexit2` protocol tests The `ssl` option is a global option, not a per-host option. This commit could instead do: local $ddclient::globals{ssl} = 0; but it's more straightforward to include `http://` in the `server` option, and it tests that `server` supports the inclusion of the scheme. --- t/protocol_dnsexit2.pl | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/t/protocol_dnsexit2.pl b/t/protocol_dnsexit2.pl index b32c688..dd24b1d 100644 --- a/t/protocol_dnsexit2.pl +++ b/t/protocol_dnsexit2.pl @@ -66,7 +66,6 @@ sub get_requests { subtest 'Testing nic_dnsexit2_update' => sub { my %config = ( 'host.my.zone.com' => { - 'ssl' => 'no', 'verbose' => 'yes', 'usev4' => 'ipv4', 'wantipv4' => '8.8.4.4', @@ -75,7 +74,7 @@ subtest 'Testing nic_dnsexit2_update' => sub { 'protocol' => 'dnsexit2', 'password' => 'mytestingpassword', 'zone' => 'my.zone.com', - 'server' => $httpd->host_port(), + 'server' => $httpd->endpoint(), 'path' => '/update', 'ttl' => 5 }); @@ -111,13 +110,12 @@ subtest 'Testing nic_dnsexit2_update' => sub { subtest 'Testing nic_dnsexit2_update without a zone set' => sub { my %config = ( 'myhost.zone.com' => { - 'ssl' => 'yes', 'verbose' => 'yes', 'usev4' => 'ipv4', 'wantipv4' => '8.8.4.4', 'protocol' => 'dnsexit2', 'password' => 'anotherpassword', - 'server' => $httpd->host_port(), + 'server' => $httpd->endpoint(), 'path' => '/update-alt', 'ttl' => 10 }); @@ -143,24 +141,22 @@ subtest 'Testing nic_dnsexit2_update without a zone set' => sub { subtest 'Testing nic_dnsexit2_update with two hostnames, one with a zone and one without' => sub { my %config = ( 'host1.zone.com' => { - 'ssl' => 'yes', 'verbose' => 'yes', 'usev4' => 'ipv4', 'wantipv4' => '8.8.4.4', 'protocol' => 'dnsexit2', 'password' => 'testingpassword', - 'server' => $httpd->host_port(), + 'server' => $httpd->endpoint(), 'path' => '/update', 'ttl' => 5 }, 'host2.zone.com' => { - 'ssl' => 'yes', 'verbose' => 'yes', 'usev6' => 'ipv6', 'wantipv6' => '2001:4860:4860::8888', 'protocol' => 'dnsexit2', 'password' => 'testingpassword', - 'server' => $httpd->host_port(), + 'server' => $httpd->endpoint(), 'path' => '/update', 'ttl' => 10, 'zone' => 'zone.com' From 0c094f6ee89bb094ca52d87c0c8c9139a563d32a Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 18 Aug 2024 00:32:33 -0400 Subject: [PATCH 02/32] tests: Fix `verbose` option for `dnsexit2` protocol tests The `verbose` option is a global option, not a per-host option. --- t/protocol_dnsexit2.pl | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/t/protocol_dnsexit2.pl b/t/protocol_dnsexit2.pl index dd24b1d..ec5137d 100644 --- a/t/protocol_dnsexit2.pl +++ b/t/protocol_dnsexit2.pl @@ -34,6 +34,8 @@ $httpd->run(sub { diag(sprintf("started IPv4 server running at %s", $httpd->endpoint())); +local $ddclient::globals{verbose} = 1; + my $ua = LWP::UserAgent->new; sub test_nic_dnsexit2_update { @@ -66,7 +68,6 @@ sub get_requests { subtest 'Testing nic_dnsexit2_update' => sub { my %config = ( 'host.my.zone.com' => { - 'verbose' => 'yes', 'usev4' => 'ipv4', 'wantipv4' => '8.8.4.4', 'usev6' => 'ipv6', @@ -110,7 +111,6 @@ subtest 'Testing nic_dnsexit2_update' => sub { subtest 'Testing nic_dnsexit2_update without a zone set' => sub { my %config = ( 'myhost.zone.com' => { - 'verbose' => 'yes', 'usev4' => 'ipv4', 'wantipv4' => '8.8.4.4', 'protocol' => 'dnsexit2', @@ -141,7 +141,6 @@ subtest 'Testing nic_dnsexit2_update without a zone set' => sub { subtest 'Testing nic_dnsexit2_update with two hostnames, one with a zone and one without' => sub { my %config = ( 'host1.zone.com' => { - 'verbose' => 'yes', 'usev4' => 'ipv4', 'wantipv4' => '8.8.4.4', 'protocol' => 'dnsexit2', @@ -151,7 +150,6 @@ subtest 'Testing nic_dnsexit2_update with two hostnames, one with a zone and one 'ttl' => 5 }, 'host2.zone.com' => { - 'verbose' => 'yes', 'usev6' => 'ipv6', 'wantipv6' => '2001:4860:4860::8888', 'protocol' => 'dnsexit2', From 0b79e3bc95a61ff195b3c6a88f9da2f427fb41fe Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 2 Jul 2024 02:00:31 -0400 Subject: [PATCH 03/32] godaddy: Delete redundant condition --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index 4b4e1b7..095aa0e 100755 --- a/ddclient.in +++ b/ddclient.in @@ -5418,7 +5418,7 @@ sub nic_godaddy_update { if ($code eq "400") { $msg = 'GoDaddy API URL ($url) was malformed.'; } elsif ($code eq "401") { - if ($config{$h}{'login'} && $config{$h}{'login'}) { + if ($config{$h}{'login'}) { $msg = 'login or password option incorrect.'; } else { $msg = 'login or password option missing.'; From ab2e0d79993495cbd91cf48bd8974c825a9be11c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 2 Jul 2024 02:08:27 -0400 Subject: [PATCH 04/32] hetzner: Quote interpolated value in regex to prevent metacharacter issues. --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index 095aa0e..cb1bbe1 100755 --- a/ddclient.in +++ b/ddclient.in @@ -5896,7 +5896,7 @@ sub nic_hetzner_update { my $headers = "Auth-API-Token: $config{$domain}{'password'}\n"; $headers .= "Content-Type: application/json"; - (my $hostname = $domain) =~ s/\.$config{$domain}{zone}$//; + (my $hostname = $domain) =~ s/\Q.$config{$domain}{zone}\E$//; my $ipv4 = delete $config{$domain}{'wantipv4'}; my $ipv6 = delete $config{$domain}{'wantipv6'}; From bd688e97506c86720a9f0bbdac4fd996f8838f3e Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 24 Jun 2024 17:55:15 -0400 Subject: [PATCH 05/32] Add TODO comments for problematic bits of code --- ddclient.in | 73 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 5 deletions(-) diff --git a/ddclient.in b/ddclient.in index cb1bbe1..b5d110c 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1540,6 +1540,11 @@ sub write_recap { my ($file) = @_; for my $h (keys %config) { + # nic_updateable (called from update_nics for each host) sets `$config{$h}{update}` + # according to whether an update was attempted. + # + # TODO: Why are different variables saved to the recap depending on whether an update was + # attempted or not? Shouldn't the same variables be saved every time? if (!exists $recap{$h} || $config{$h}{'update'}) { my $vars = $protocols{$config{$h}{protocol}}{variables}; for my $v (keys(%$vars)) { @@ -1602,6 +1607,8 @@ sub read_recap { for my $h (keys(%recap)) { next if !exists($config->{$h}); + # TODO: Why is this limited to this set of variables? Why not copy every recap var + # defined for the host's protocol? for (qw(atime mtime wtime ip ipv4 ipv6 status status-ipv4 status-ipv6)) { # TODO: Isn't $config equal to \%recap here? If so, this is a no-op. What was the # original intention behind this? To copy %recap values into %config? If so, is @@ -1760,6 +1767,8 @@ sub _read_config { } ## remove comments + # TODO: This makes it impossible to include '#' in keys or values except as permitted by + # the special password parsing above. s/#.*//; ## Handle continuation lines @@ -1778,6 +1787,8 @@ sub _read_config { s/^\s+//; # remove leading white space s/\s+$//; # remove trailing white space + # TODO: This makes it impossible to include multiple consecutive spaces, tabs, etc. in keys + # or values. s/\s+/ /g; # canonify next if /^$/; @@ -1799,6 +1810,7 @@ sub _read_config { # Set the value to the value of the environment variable $locals{$1} = $ENV{$locals{$k}}; # Remove the '_env' suffix from the key + # TODO: Shouldn't the *_env entry be deleted from %locals? $k = $1; } @@ -1808,6 +1820,8 @@ sub _read_config { delete $locals{$k}; next; } + # TODO: This might grab an arbitrary protocol-specific variable definition, which could + # cause surprising behavior. my $def = $variables{'merged'}{$k}; my $value = check_value($locals{$k}, $def); if (!defined($value)) { @@ -1872,24 +1886,36 @@ sub init_config { ## infer the IP strategy if possible if (!$opt{'use'}) { + # TODO: I don't think these have any effect. The value is not copied into the + # host-specific %config settings, and $config{$h}{use} is initialized to the variable + # default ("ip"), not set to undef, so opt() won't fall back to this. $opt{'use'} = 'web' if ($opt{'web'}); $opt{'use'} = 'if' if ($opt{'if'}); $opt{'use'} = 'ip' if ($opt{'ip'}); } ## infer the IPv4 strategy if possible if (!$opt{'usev4'}) { + # TODO: I don't think these have any effect. The value is not copied into the + # host-specific %config settings, and $config{$h}{usev4} is initialized to the variable + # default ("disabled"), not set to undef, so opt() won't fall back to this. $opt{'usev4'} = 'webv4' if ($opt{'webv4'}); $opt{'usev4'} = 'ifv4' if ($opt{'ifv4'}); $opt{'usev4'} = 'ipv4' if ($opt{'ipv4'}); } ## infer the IPv6 strategy if possible if (!$opt{'usev6'}) { + # TODO: I don't think these have any effect. The value is not copied into the + # host-specific %config settings, and $config{$h}{usev6} is initialized to the variable + # default ("disabled"), not set to undef, so opt() won't fall back to this. $opt{'usev6'} = 'webv6' if ($opt{'webv6'}); $opt{'usev6'} = 'ifv6' if ($opt{'ifv6'}); $opt{'usev6'} = 'ipv6' if ($opt{'ipv6'}); } ## sanity check + # TODO: I don't think these have any effect. The values are not copied into the host-specific + # %config settings, and $config{$h}{$opt} is initialized to the non-undef variable default so + # opt() won't fall back to these. $opt{'max-interval'} = min(interval(opt('max-interval')), interval(default('max-interval'))); $opt{'min-interval'} = max(interval(opt('min-interval')), interval(default('min-interval'))); $opt{'min-error-interval'} = max(interval(opt('min-error-interval')), interval(default('min-error-interval'))); @@ -1911,6 +1937,7 @@ sub init_config { warning("'$name' is deprecated and does nothing"); next; } + # TODO: Shouldn't *_env options be processed like _read_config does? $options{$name} = $var; } ## determine hosts specified with --host @@ -1928,20 +1955,20 @@ sub init_config { delete $options{'host'}; } ## merge options into host definitions or globals + # TODO: Keys and values should be validated before mutating %config or %globals. if (@hosts) { for my $h (@hosts) { $config{$h} = {%{$config{$h} // {}}, %options, 'host' => $h}; } $opt{'host'} = join(',', @hosts); } else { + # TODO: Why not merge the values into %opt? %globals = (%globals, %options); } } ## override global options with those on the command-line. for my $o (keys %opt) { - # TODO: Isn't $opt{$o} guaranteed to be defined? Otherwise $o wouldn't appear in the keys - # of %opt, right? # TODO: Why is this limited to $variables{'global-defaults'}? Why not # $variables{'merged'}? if (defined $opt{$o} && exists $variables{'global-defaults'}{$o}) { @@ -1949,6 +1976,9 @@ sub init_config { # %opt doesn't have a value, so this shouldn't be necessary. $globals{$o} = $opt{$o}; } + # TODO: Why aren't host configs updated with command-line values (except for $opt{options} + # handled above)? Shouldn't command-line values always override config file values (even + # if they are not associated with a host via `--host=` or `--options=host=`)? } ## sanity check @@ -1962,7 +1992,9 @@ sub init_config { if (opt('host')) { @hosts = split_by_comma($opt{'host'}); } - # TODO: This function is called before the recap file is read. How is this supposed to work? + # TODO: The first two times init_config() is called the cache file has not been read yet, so + # this will not filter out any hosts and thus updates will not be limited to non-good hosts as + # intended. if (opt('retry')) { @hosts = grep(($recap{$_}{'status'} // '') ne 'good', keys(%recap)); } @@ -1972,11 +2004,19 @@ sub init_config { map { $hosts{$_} = undef } @hosts; map { delete $config{$_} unless exists $hosts{$_} } keys %config; + # TODO: Why aren't the hosts specified by --host added to %config except when --options is also + # given? + ## sanity check.. + # TODO: The code below doesn't look like a mere "sanity check", so I'm guessing the above + # comment is out of date. Figure out what this code is actually doing and refactor to improve + # the readability so that a comment isn't necessary. ## make sure config entries have all defaults and they meet minimums ## first the globals... for my $k (keys %globals) { # Make sure any _env suffixed variables look at their original entry + # TODO: Didn't _read_config already handle *_env? Or is this needed to handle + # the --options command-line option whose values were merged into %globals above? $k = $1 if $k =~ /^(.*)_env$/; # TODO: This might grab an arbitrary protocol-specific variable, which could cause @@ -1990,9 +2030,12 @@ sub init_config { # TODO: Isn't $globals{$k} guaranteed to be defined here? Otherwise $k wouldn't appear in # %globals. my $ovalue = $globals{$k} // $def->{'default'}; - # TODO: Didn't _read_config already check the value? Or is the purpose of this to check - # the value of command-line options ($opt{$k}) which were merged into %globals above? + # _read_config already checked any value from the config file, so the purpose of this check + # is to validate command-line options which were merged into %globals above. + # TODO: Move this check to where the command-line options are actually processed. my $value = check_value($ovalue, $def); + # TODO: If the variable is not required, the value is set to undef but no warning is + # logged. Is that intentional? if ($def->{'required'} && !defined $value) { # TODO: What's the point of this? The opt() function will fall back to the default # value if $globals{$k} is undefined. @@ -2018,13 +2061,26 @@ sub init_config { my $svars = $protocols{$proto}{'variables'}; my $conf = {'host' => $h, 'protocol' => $proto}; + # TODO: This silently ignores unknown options passed via --options. for my $k (keys %$svars) { # Make sure any _env suffixed variables look at their original entry $k = $1 if $k =~ /^(.*)_env$/; my $def = $svars->{$k}; + # TODO: Why doesn't this try %opt and %globals before falling back to the variable + # default? By ignoring %opt and %globals, `--options` will not have any effect on any + # hosts that are not specified on the command line (via `--host=a,b` and/or + # `--options=host=c,d`). + # TODO: Why not just leave $conf->{$k} undef? Then opt() would automatically fall back + # to %opt, %globals, or the variable default. my $ovalue = $config{$h}{$k} // $def->{'default'}; + # _read_config already checked any value from the config file, so the purpose of this + # check is to validate command-line options from --options which were merged into + # $config{$h} above. + # TODO: Move this check to where --options is actually processed. my $value = check_value($ovalue, $def); + # TODO: If the variable is not required, the value is set to undef but no warning is + # logged. Is that intentional? if ($def->{'required'} && !defined $value) { $ovalue //= '(not set)'; warning("skipping host $h: invalid $def->{type} variable value '$k=$ovalue'"); @@ -2321,17 +2377,24 @@ sub split_by_comma { sub default { my $v = shift; return undef if !defined($variables{'merged'}{$v}); + # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause + # surprising behavior. return $variables{'merged'}{$v}{'default'}; } sub minimum { my $v = shift; return undef if !defined($variables{'merged'}{$v}); + # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause + # surprising behavior. return $variables{'merged'}{$v}{'minimum'}; } sub opt { my $v = shift; my $h = shift; return $config{$h}{$v} if defined($h) && defined($config{$h}{$v}); + # TODO: Why check %opt before %globals? Valid variables from %opt are merged into %globals by + # init_config(), so it shouldn't be necessary. Also, it runs the risk of collision with a + # non-variable command line option like `--version`, `--help`, etc. return $opt{$v} // $globals{$v} // default($v); } sub min { From b4c4b5dc5473ffcf1a1d80c2a6f228f58cdc376a Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 24 Jun 2024 02:14:18 -0400 Subject: [PATCH 06/32] Move usage generation to a separate function --- ddclient.in | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/ddclient.in b/ddclient.in index b5d110c..2e96a0d 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1248,12 +1248,12 @@ my @opt = ( ); sub main { - my $opt_usage = process_args(@opt); + process_args(@opt); $saved_recap = ''; %saved_opt = %opt; $result = 'OK'; if (opt('help')) { - printf "%s\n", $opt_usage; + print(usage(@opt), "\n"); $opt{'version'}('', ''); } @@ -2093,18 +2093,12 @@ sub init_config { } } -###################################################################### -## process_args - -###################################################################### -sub process_args { - my @spec = (); +sub usage { my $usage = ""; for (@_) { if (ref $_) { my ($key, $specifier, $arg_usage) = @$_; my $value = default($key); - push @spec, $key . $specifier; - $opt{$key} //= undef; next unless $arg_usage; $usage .= " $arg_usage"; if (defined($value) && $value ne '') { @@ -2123,10 +2117,23 @@ sub process_args { } $usage .= "\n"; } + return $usage; +} + +###################################################################### +## process_args - +###################################################################### +sub process_args { + my @spec = (); + for (@_) { + next if !ref($_); + my ($key, $specifier) = @$_; + push @spec, $key . $specifier; + $opt{$key} //= undef; + } if (!GetOptions(\%opt, @spec)) { $opt{"help"} = 1; } - return $usage; } ###################################################################### From c83dc67039e96465d0f7699c2227721b43b2897f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 24 Jun 2024 17:17:29 -0400 Subject: [PATCH 07/32] Remove pointless `help` setting This does not affect the `--help` command-line argument. The `help` setting didn't do anything useful, and it didn't make sense to set `help=1` in the config file (or pass `--options=help=1`), so this removal is unlikely to affect anyone. If the setting does exist, the user will get a warning and the setting will be ignored. --- ddclient.in | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ddclient.in b/ddclient.in index 2e96a0d..99edd71 100755 --- a/ddclient.in +++ b/ddclient.in @@ -614,7 +614,6 @@ our %variables = ( 'debug' => setv(T_BOOL, 0, 0, 0, undef), 'verbose' => setv(T_BOOL, 0, 0, 0, undef), 'quiet' => setv(T_BOOL, 0, 0, 0, undef), - 'help' => setv(T_BOOL, 0, 0, 0, undef), 'test' => setv(T_BOOL, 0, 0, 0, undef), 'postscript' => setv(T_POSTS, 0, 0, undef, undef), @@ -1291,11 +1290,6 @@ sub main { $now = time; $result = 'OK'; %opt = %saved_opt; - if (opt('help')) { - *STDERR = *STDOUT; - printf("Help found"); - } - read_config($opt{'file'} // default('file'), \%config, \%globals); init_config(); read_recap(opt('cache'), \%recap); From 9e659a18ebdd5d7f0e57111403197b718f38fbd8 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 24 Jun 2024 03:15:14 -0400 Subject: [PATCH 08/32] Move `--help` processing to `%opt` --- ddclient.in | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ddclient.in b/ddclient.in index 99edd71..bd96d39 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1245,17 +1245,16 @@ my @opt = ( "", nic_examples(), ); +$opt{'help'} = sub { + print(usage(@opt), "\n"); + $opt{'version'}('', ''); +}; sub main { process_args(@opt); $saved_recap = ''; %saved_opt = %opt; $result = 'OK'; - if (opt('help')) { - print(usage(@opt), "\n"); - $opt{'version'}('', ''); - } - ## read config file because 'daemon' mode may be defined there. read_config($opt{'file'} // default('file'), \%config, \%globals); init_config(); @@ -2126,7 +2125,7 @@ sub process_args { $opt{$key} //= undef; } if (!GetOptions(\%opt, @spec)) { - $opt{"help"} = 1; + $opt{'help'}('', ''); } } From 05dbe7a9840fdfd05d34ad51014e6cc62078d088 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 24 Jun 2024 15:38:30 -0400 Subject: [PATCH 09/32] Delete confusing and unnecessary `T_OFQDN` type Variable declarations already have a `required` flag, which makes the type confusing. What would it mean for a variable to be a required `T_OFQDN`? And how would an optional `T_FQDN` differ from an optional `T_OFQDN`? --- ddclient.in | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ddclient.in b/ddclient.in index bd96d39..a4d7434 100755 --- a/ddclient.in +++ b/ddclient.in @@ -177,7 +177,6 @@ sub T_LOGIN { 'login' } sub T_PASSWD { 'password' } sub T_BOOL { 'boolean value' } sub T_FQDN { 'fully qualified host name' } -sub T_OFQDN { 'optional fully qualified host name' } sub T_FILE { 'file name' } sub T_FQDNP { 'fully qualified host name and optional port number' } sub T_PROTO { 'protocol' } @@ -704,7 +703,7 @@ our %variables = ( }, 'dyndns-common-defaults' => { 'backupmx' => setv(T_BOOL, 0, 1, 0, undef), - 'mx' => setv(T_OFQDN, 0, 1, undef, undef), + 'mx' => setv(T_FQDN, 0, 1, undef, undef), 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), }, ); @@ -734,7 +733,7 @@ our %protocols = ( 'backupmx' => setv(T_BOOL, 0, 1, 0, undef), 'login' => setv(T_LOGIN, 0, 0, 'token', undef), 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'mx' => setv(T_OFQDN, 0, 1, undef, undef), + 'mx' => setv(T_FQDN, 0, 1, undef, undef), 'server' => setv(T_FQDNP, 0, 0, 'api.cloudflare.com/client/v4', undef), 'static' => setv(T_BOOL, 0, 1, 0, undef), 'ttl' => setv(T_NUMBER, 0, 0, 1, undef), @@ -861,7 +860,7 @@ our %protocols = ( # From : "You need to wait at least 10 # minutes between updates." 'min-interval' => setv(T_DELAY, 0, 0, interval('10m'), 0), - 'mx' => setv(T_OFQDN, 0, 1, undef, undef), + 'mx' => setv(T_FQDN, 0, 1, undef, undef), 'server' => setv(T_FQDNP, 0, 0, 'api.cp.easydns.com', undef), 'script' => setv(T_STRING, 0, 1, '/dyn/generic.php', undef), 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), @@ -1017,7 +1016,7 @@ our %protocols = ( 'password' => undef, 'apikey' => setv(T_PASSWD, 1, 0, undef, undef), 'secretapikey' => setv(T_PASSWD, 1, 0, undef, undef), - 'root-domain' => setv(T_OFQDN, 0, 0, undef, undef), + 'root-domain' => setv(T_FQDN, 0, 0, undef, undef), 'on-root-domain' => setv(T_BOOL, 0, 0, 0, undef), }, }, @@ -1046,7 +1045,7 @@ our %protocols = ( %{$variables{'protocol-common-defaults'}}, 'min-interval' => setv(T_DELAY, 0, 0, interval('10m'), 0), 'server' => setv(T_FQDNP, 0, 0, 'dynamic.zoneedit.com', undef), - 'zone' => setv(T_OFQDN, 0, 0, undef, undef), + 'zone' => setv(T_FQDN, 0, 0, undef, undef), }, }, 'keysystems' => { @@ -2587,9 +2586,9 @@ sub check_value { } else { return undef; } - } elsif ($type eq T_FQDN || $type eq T_OFQDN && $value ne '') { + } elsif ($type eq T_FQDN) { $value = lc $value; - return undef if $value !~ /[^.]\.[^.]/; + return undef if ($value ne '' || $required) && $value !~ /[^.]\.[^.]/; } elsif ($type eq T_FQDNP) { $value = lc $value; From 70858e659fd96b8f5ea02f234040d67c8e05383f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 26 Jun 2024 19:41:15 -0400 Subject: [PATCH 10/32] Also warn about non-required values that are invalid Before, only required values that were invalid produced a warning. Non-required values were quietly ignored. --- ddclient.in | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/ddclient.in b/ddclient.in index a4d7434..bc859af 100755 --- a/ddclient.in +++ b/ddclient.in @@ -2026,13 +2026,14 @@ sub init_config { # is to validate command-line options which were merged into %globals above. # TODO: Move this check to where the command-line options are actually processed. my $value = check_value($ovalue, $def); - # TODO: If the variable is not required, the value is set to undef but no warning is - # logged. Is that intentional? - if ($def->{'required'} && !defined $value) { - # TODO: What's the point of this? The opt() function will fall back to the default - # value if $globals{$k} is undefined. - $value = default($k); - warning("'%s=%s' is an invalid %s. (using default of %s)", $k, $ovalue, $def->{'type'}, $value); + if (!defined($value)) { + $ovalue //= '(not set)'; + warning("ignoring invalid $def->{type} variable value '$k=$ovalue'"); + if ($def->{'required'}) { + # TODO: What's the point of this? The opt() function will fall back to the default + # value if $globals{$k} is undefined. + $value = default($k); + } } $globals{$k} = $value; } @@ -2071,13 +2072,15 @@ sub init_config { # $config{$h} above. # TODO: Move this check to where --options is actually processed. my $value = check_value($ovalue, $def); - # TODO: If the variable is not required, the value is set to undef but no warning is - # logged. Is that intentional? - if ($def->{'required'} && !defined $value) { + if (!defined($value)) { $ovalue //= '(not set)'; - warning("skipping host $h: invalid $def->{type} variable value '$k=$ovalue'"); - delete $config{$h}; - next HOST; + if ($def->{'required'}) { + warning("skipping host $h: invalid $def->{type} variable value '$k=$ovalue'"); + delete $config{$h}; + next HOST; + } else { + warning("host $h: ignoring invalid $def->{type} variable value '$k=$ovalue'"); + } } $conf->{$k} = $value; } From 4d3dcdc7de8e11441302b9907b9316cb7cea6d93 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 13 Jun 2024 03:07:52 -0400 Subject: [PATCH 11/32] Move option normalization from usage to load This avoids bugs if a usage forgets to normalize the value. --- ddclient.in | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/ddclient.in b/ddclient.in index bc859af..da78b59 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1372,15 +1372,12 @@ sub update_nics { for my $h (sort keys %config) { local $_l = pushlogctx($h); - next if $config{$h}{'protocol'} ne lc($p); + next if $config{$h}{'protocol'} ne $p; $examined{$h} = 1; # we only do this once per 'use' and argument combination - my $use = opt('use', $h) // 'disabled'; - my $usev4 = opt('usev4', $h) // 'disabled'; - my $usev6 = opt('usev6', $h) // 'disabled'; - $use = 'disabled' if ($use eq 'no'); # backward compatibility - $usev6 = 'disabled' if ($usev6 eq 'no'); # backward compatibility - $use = 'disabled' if ($usev4 ne 'disabled') || ($usev6 ne 'disabled'); + my $use = opt('use', $h); + my $usev4 = opt('usev4', $h); + my $usev6 = opt('usev6', $h); my $arg_ip = opt('ip', $h) // ''; my $arg_ipv4 = opt('ipv4', $h) // ''; my $arg_ipv6 = opt('ipv6', $h) // ''; @@ -2041,6 +2038,8 @@ sub init_config { ## now the host definitions... HOST: for my $h (keys %config) { + $config{$h}{use} = 'disabled' + if opt('usev4', $h) ne 'disabled' || opt('usev6', $h) ne 'disabled'; my $proto = opt('protocol', $h); load_sha1_support($proto) if (grep($_ eq $proto, ("freedns", "nfsn"))); load_json_support($proto) if (grep($_ eq $proto, ("1984", "cloudflare", "digitalocean", "directnic", "gandi", "godaddy", "hetzner", "yandex", "nfsn", "njalla", "porkbun", "dnsexit2"))); @@ -2606,6 +2605,7 @@ sub check_value { } elsif ($type eq T_USE) { $value = lc $value; + $value = 'disabled' if $value eq 'no'; # backwards compatibility return undef if !exists $ip_strategies{$value}; } elsif ($type eq T_USEV4) { @@ -2614,6 +2614,7 @@ sub check_value { } elsif ($type eq T_USEV6) { $value = lc $value; + $value = 'disabled' if $value eq 'no'; # backwards compatibility return undef if !exists $ipv6_strategies{$value}; } elsif ($type eq T_FILE) { @@ -2842,9 +2843,7 @@ sub geturl { ## get_ip ###################################################################### sub get_ip { - my $use = lc shift; - $use = 'disabled' if ($use eq 'no'); # backward compatibility - my $h = shift; + my ($use, $h) = @_; my ($ip, $reply, $url, $skip) = (undef, ''); my $argvar = $use; # Note that --use=firewallname uses --fw=arg, not --firewallname=arg. @@ -3239,8 +3238,7 @@ sub get_ip_from_interface { ## get_ipv4 ###################################################################### sub get_ipv4 { - my $usev4 = lc(shift); ## Method to obtain IP address - my $h = shift; ## Host/service making the request + my ($usev4, $h) = @_; my $ipv4 = undef; ## Found IPv4 address my $reply = ''; ## Text returned from various methods my $url = ''; ## URL of website or firewall @@ -3349,9 +3347,7 @@ sub get_ipv4 { ## get_ipv6 ###################################################################### sub get_ipv6 { - my $usev6 = lc(shift); ## Method to obtain IP address - $usev6 = 'disabled' if ($usev6 eq 'no'); # backward compatibility - my $h = shift; ## Host/service making the request + my ($usev6, $h) = @_; my $ipv6 = undef; ## Found IPv6 address my $reply = ''; ## Text returned from various methods my $url = ''; ## URL of website or firewall @@ -3561,12 +3557,9 @@ sub nic_updateable { my $ip = $config{$host}{'wantip'}; my $ipv4 = $config{$host}{'wantipv4'}; my $ipv6 = $config{$host}{'wantipv6'}; - my $use = opt('use', $host) // 'disabled'; - my $usev4 = opt('usev4', $host) // 'disabled'; - my $usev6 = opt('usev6', $host) // 'disabled'; - $use = 'disabled' if ($use eq 'no'); # backward compatibility - $usev6 = 'disabled' if ($usev6 eq 'no'); # backward compatibility - $use = 'disabled' if ($usev4 ne 'disabled') || ($usev6 ne 'disabled'); + my $use = opt('use', $host); + my $usev4 = opt('usev4', $host); + my $usev6 = opt('usev6', $host); my $inv_ip_warn_count = opt('max-warn'); my $previp = $recap{$host}{'ip'} || ''; my $previpv4 = $recap{$host}{'ipv4'} || ''; From 2f8a4ba00a75ca0b1cbee64718cc15a893c65233 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 20 Jun 2024 23:57:32 -0400 Subject: [PATCH 12/32] Use `opt()` instead of accessing `%opt` or `%globals` directly This is for consistency, and to ensure that all possible ways of configuring are respected. --- ddclient.in | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/ddclient.in b/ddclient.in index da78b59..7bd3867 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1255,7 +1255,7 @@ sub main { %saved_opt = %opt; $result = 'OK'; ## read config file because 'daemon' mode may be defined there. - read_config($opt{'file'} // default('file'), \%config, \%globals); + read_config(opt('file'), \%config, \%globals); init_config(); test_possible_ip() if opt('query'); @@ -1288,19 +1288,17 @@ sub main { $now = time; $result = 'OK'; %opt = %saved_opt; - read_config($opt{'file'} // default('file'), \%config, \%globals); + read_config(opt('file'), \%config, \%globals); init_config(); read_recap(opt('cache'), \%recap); print_info() if opt('debug') && opt('verbose'); fatal("invalid argument '--use=%s'; possible values are:\n%s", - $opt{'use'}, join("\n", ip_strategies_usage())) + opt('use'), join("\n", ip_strategies_usage())) if defined(opt('use')) && !$ip_strategies{lc(opt('use'))}; - if (defined($opt{'usev6'})) { - fatal("invalid argument '--usev6=%s'; possible values are:\n%s", - $opt{'usev6'}, join("\n", ipv6_strategies_usage())) - unless exists $ipv6_strategies{lc opt('usev6')}; - } + fatal("invalid argument '--usev6=%s'; possible values are:\n%s", + opt('usev6'), join("\n", ipv6_strategies_usage())) + if defined(opt('usev6')) && !$ipv6_strategies{lc(opt('usev6'))}; $daemon = opt('daemon'); @@ -1347,12 +1345,12 @@ sub main { sub runpostscript { my ($ip) = @_; - if (defined $globals{postscript}) { - my @postscript = split(/\s+/, $globals{postscript}); + if (defined(my $ps = opt('postscript'))) { + my @postscript = split(/\s+/, $ps); if (-x $postscript[0]) { - system("$globals{postscript} $ip &"); + system("$ps $ip &"); } else { - warning("Can not execute post script: %s", $globals{postscript}); + warning("Can not execute post script: %s", $ps); } } } @@ -1971,7 +1969,7 @@ sub init_config { } ## sanity check - if (defined $opt{'host'} && defined $opt{'retry'}) { + if (defined(opt('host')) && opt('retry')) { fatal("options --retry and --host (or --option host=..) are mutually exclusive"); } fatal("options --retry and --daemon cannot be used together") if (opt('retry') && opt('daemon')); @@ -2769,7 +2767,7 @@ sub geturl { $use_ssl = 1; } elsif ($url =~ /^http:/) { $use_ssl = 0; - } elsif ($globals{'ssl'} && !($params{ignore_ssl_option} // 0)) { + } elsif (opt('ssl') && !($params{ignore_ssl_option} // 0)) { $use_ssl = 1; } else { $use_ssl = 0; @@ -3573,7 +3571,7 @@ sub nic_updateable { $warned_ipv4{$host} = 0 if $usev4 ne 'disabled' && $ipv4; $warned_ipv6{$host} = 0 if $usev6 ne 'disabled' && $ipv6; - if ($opt{'force'}) { + if (opt('force')) { info("update forced via 'force' option"); $update = 1; From ed2afde72d88ef85dedf2dbf4a0d8a17cb0f0f5d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 26 Jun 2024 19:00:45 -0400 Subject: [PATCH 13/32] check_value: `die` if the value is invalid This makes it possible to convey details about why the value was deemed invalid. It also allows `undef` to be treated as a valid value. --- ddclient.in | 63 ++++++++++++++++++++++++------------------------ t/check_value.pl | 16 ++++++++---- 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/ddclient.in b/ddclient.in index 7bd3867..f4f4ca3 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1810,13 +1810,11 @@ sub _read_config { # TODO: This might grab an arbitrary protocol-specific variable definition, which could # cause surprising behavior. my $def = $variables{'merged'}{$k}; - my $value = check_value($locals{$k}, $def); - if (!defined($value)) { - warning("Invalid Value for keyword '%s' = '%s'", $k, $locals{$k}); + if (!eval { $locals{$k} = check_value($locals{$k}, $def); 1; }) { + warning("invalid variable value '$k=$locals{$k}': $@"); delete $locals{$k}; next; } - $locals{$k} = $value; } %passwords = (); if (exists($locals{'host'})) { @@ -2020,17 +2018,17 @@ sub init_config { # _read_config already checked any value from the config file, so the purpose of this check # is to validate command-line options which were merged into %globals above. # TODO: Move this check to where the command-line options are actually processed. - my $value = check_value($ovalue, $def); - if (!defined($value)) { + if (!eval { $globals{$k} = check_value($ovalue, $def); 1; }) { $ovalue //= '(not set)'; - warning("ignoring invalid $def->{type} variable value '$k=$ovalue'"); + warning("ignoring invalid variable value '$k=$ovalue': $@"); if ($def->{'required'}) { # TODO: What's the point of this? The opt() function will fall back to the default # value if $globals{$k} is undefined. - $value = default($k); + $globals{$k} = default($k); + } else { + $globals{$k} = undef; } } - $globals{$k} = $value; } ## now the host definitions... @@ -2068,18 +2066,17 @@ sub init_config { # check is to validate command-line options from --options which were merged into # $config{$h} above. # TODO: Move this check to where --options is actually processed. - my $value = check_value($ovalue, $def); - if (!defined($value)) { + if (!eval { $conf->{$k} = check_value($ovalue, $def); 1; }) { $ovalue //= '(not set)'; if ($def->{'required'}) { - warning("skipping host $h: invalid $def->{type} variable value '$k=$ovalue'"); + warning("skipping host $h: invalid variable value '$k=$ovalue': $@"); delete $config{$h}; next HOST; } else { - warning("host $h: ignoring invalid $def->{type} variable value '$k=$ovalue'"); + warning("host $h: ignoring invalid variable value '$k=$ovalue': $@"); + $conf->{$k} = undef; } } - $conf->{$k} = $value; } $config{$h} = $conf; } @@ -2558,7 +2555,8 @@ sub interval_expired { ## check_value ###################################################################### sub check_value { - my ($value, $def) = @_; + my ($orig, $def) = @_; + my $value = $orig; my $type = $def->{'type'}; my $min = $def->{'minimum'}; my $required = $def->{'required'}; @@ -2568,14 +2566,14 @@ sub check_value { } elsif (!defined($value) && $required) { # None of the types have 'undef' as a valid value, so check definedness once here for # convenience. - return undef; + die("$type is required\n"); } elsif ($type eq T_DELAY) { $value = interval($value); $value = $min if defined($value) && defined($min) && $value < $min; } elsif ($type eq T_NUMBER) { - return undef if $value !~ /^\d+$/; + die("invalid $type: $orig\n") if $value !~ /^\d+$/; $value = $min if defined($min) && $value < $min; } elsif ($type eq T_BOOL) { @@ -2584,57 +2582,58 @@ sub check_value { } elsif ($value =~ /^(n(o)?|f(alse)?|0)$/i) { $value = 0; } else { - return undef; + die("invalid $type: $orig\n"); } } elsif ($type eq T_FQDN) { $value = lc $value; - return undef if ($value ne '' || $required) && $value !~ /[^.]\.[^.]/; + die("invalid $type: $orig\n") if ($value ne '' || $required) && $value !~ /[^.]\.[^.]/; } elsif ($type eq T_FQDNP) { $value = lc $value; - return undef if $value !~ /[^.]\.[^.].*(:\d+)?$/; + die("invalid $type: $orig\n") if $value !~ /[^.]\.[^.].*(:\d+)?$/; } elsif ($type eq T_PROTO) { $value = lc $value; - return undef if !exists $protocols{$value}; + die("invalid $type: $orig\n") if !exists $protocols{$value}; } elsif ($type eq T_URL) { - return undef if $value !~ qr{^(?i:https?://)?[^./]+(\.[^./]+)+(:\d+)?(/[^/]+)*/?$}; + die("invalid $type: $orig\n") + if $value !~ qr{^(?i:https?://)?[^./]+(\.[^./]+)+(:\d+)?(/[^/]+)*/?$}; } elsif ($type eq T_USE) { $value = lc $value; $value = 'disabled' if $value eq 'no'; # backwards compatibility - return undef if !exists $ip_strategies{$value}; + die("invalid $type: $orig\n") if !exists($ip_strategies{$value}); } elsif ($type eq T_USEV4) { $value = lc $value; - return undef if !exists $ipv4_strategies{$value}; + die("invalid $type: $orig\n") if !exists($ipv4_strategies{$value}); } elsif ($type eq T_USEV6) { $value = lc $value; $value = 'disabled' if $value eq 'no'; # backwards compatibility - return undef if !exists $ipv6_strategies{$value}; + die("invalid $type: $orig\n") if !exists($ipv6_strategies{$value}); } elsif ($type eq T_FILE) { - return undef if $value eq ""; + die("invalid $type: $orig\n") if $value eq ""; } elsif ($type eq T_IF) { - return undef if $value !~ /^[a-zA-Z0-9:._-]+$/; + die("invalid $type: $orig\n") if $value !~ /^[a-zA-Z0-9:._-]+$/; } elsif ($type eq T_PROG) { - return undef if $value eq ""; + die("invalid $type: $orig\n") if $value eq ""; } elsif ($type eq T_LOGIN) { - return undef if $value eq ""; + die("invalid $type: $orig\n") if $value eq ""; } elsif ($type eq T_IP) { - return undef if !is_ipv4($value) && !is_ipv6($value); + die("invalid $type: $orig\n") if !is_ipv4($value) && !is_ipv6($value); } elsif ($type eq T_IPV4) { - return undef if !is_ipv4($value); + die("invalid $type: $orig\n") if !is_ipv4($value); } elsif ($type eq T_IPV6) { - return undef if !is_ipv6($value); + die("invalid $type: $orig\n") if !is_ipv6($value); } return $value; diff --git a/t/check_value.pl b/t/check_value.pl index 02a0cde..d430851 100644 --- a/t/check_value.pl +++ b/t/check_value.pl @@ -12,7 +12,7 @@ my @test_cases = ( { type => ddclient::T_FQDN(), input => 'example', - want => undef, + want_invalid => 1, }, { type => ddclient::T_URL(), @@ -32,16 +32,22 @@ my @test_cases = ( { type => ddclient::T_URL(), input => 'ftp://bad.protocol/', - want => undef, + want_invalid => 1, }, { type => ddclient::T_URL(), input => 'bad-url', - want => undef, + want_invalid => 1, }, ); for my $tc (@test_cases) { - my $got = ddclient::check_value($tc->{input}, ddclient::setv($tc->{type}, 0, 0, undef, undef)); - is($got, $tc->{want}, "$tc->{type}: $tc->{input}"); + my $got; + my $got_invalid = !(eval { + $got = ddclient::check_value($tc->{input}, + ddclient::setv($tc->{type}, 0, 0, undef, undef)); + 1; + }); + is($got_invalid, !!$tc->{want_invalid}, "$tc->{type}: $tc->{input}: validity"); + is($got, $tc->{want}, "$tc->{type}: $tc->{input}: normalization") if !$tc->{want_invalid}; } done_testing(); From 19848852a486a6c1fc64e843068ca693a0eb9081 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 26 Jun 2024 19:33:17 -0400 Subject: [PATCH 14/32] check_value: Mention supported values if given an invalid value --- ddclient.in | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ddclient.in b/ddclient.in index f4f4ca3..2b18017 100755 --- a/ddclient.in +++ b/ddclient.in @@ -2594,7 +2594,8 @@ sub check_value { } elsif ($type eq T_PROTO) { $value = lc $value; - die("invalid $type: $orig\n") if !exists $protocols{$value}; + die("invalid $type: $orig\nSupported values: ", join(' ', sort(keys(%protocols))), "\n") + if !exists $protocols{$value}; } elsif ($type eq T_URL) { die("invalid $type: $orig\n") @@ -2603,16 +2604,19 @@ sub check_value { } elsif ($type eq T_USE) { $value = lc $value; $value = 'disabled' if $value eq 'no'; # backwards compatibility - die("invalid $type: $orig\n") if !exists($ip_strategies{$value}); + die(map(($_, "\n"), "invalid $type: $orig", 'Supported values:', ip_strategies_usage())) + if !exists($ip_strategies{$value}); } elsif ($type eq T_USEV4) { $value = lc $value; - die("invalid $type: $orig\n") if !exists($ipv4_strategies{$value}); + die(map(($_, "\n"), "invalid $type: $orig", 'Supported values:', ipv4_strategies_usage())) + if !exists($ipv4_strategies{$value}); } elsif ($type eq T_USEV6) { $value = lc $value; $value = 'disabled' if $value eq 'no'; # backwards compatibility - die("invalid $type: $orig\n") if !exists($ipv6_strategies{$value}); + die(map(($_, "\n"), "invalid $type: $orig", 'Supported values:', ipv6_strategies_usage())) + if !exists($ipv6_strategies{$value}); } elsif ($type eq T_FILE) { die("invalid $type: $orig\n") if $value eq ""; From 4c7634855b6f702e4134a10966899455b59d600d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 27 Jun 2024 00:41:45 -0400 Subject: [PATCH 15/32] Move `*_env` processing to `parse_assignment` This makes the behavior transparent to the rest of ddclient, which will make it possible for a future commit to simplify option processing. --- ddclient.in | 24 +++++++++--------------- t/parse_assignments.pl | 8 ++++++++ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/ddclient.in b/ddclient.in index 2b18017..380a617 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1652,6 +1652,15 @@ sub parse_assignment { } $rest = substr($rest,1); } + if ($name =~ qr/^(.*)_env$/) { + $name = $1; + debug("Loading value for $name from environment variable $value"); + if (!exists($ENV{$value})) { + warning("Environment variable '$value' not set for keyword '$name' (ignored)"); + return parse_assignment($rest); + } + $value = $ENV{$value}; + } } warning("assignment to '%s' ended with the escape character (\\)", $name) if $escape; warning("assignment to '%s' ended with an unterminated quote (%s)", $name, $quote) if $quote; @@ -1786,21 +1795,6 @@ sub _read_config { ## verify that keywords are valid...and check the value for my $k (keys %locals) { - # Handle '_env' keyword suffix - if ($k =~ /(.*)_env$/) { - debug("Loading value for $1 from environment variable $locals{$k}."); - if (!exists($ENV{$locals{$k}})) { - warning("Environment variable '$locals{$k}' not set for keyword '$k' (ignored)"); - delete $locals{$k}; - next; - } - # Set the value to the value of the environment variable - $locals{$1} = $ENV{$locals{$k}}; - # Remove the '_env' suffix from the key - # TODO: Shouldn't the *_env entry be deleted from %locals? - $k = $1; - } - $locals{$k} = $passwords{$k} if defined $passwords{$k}; if (!exists $variables{'merged'}{$k}) { warning("unrecognized keyword '%s' (ignored)", $k); diff --git a/t/parse_assignments.pl b/t/parse_assignments.pl index d595459..d71f4a8 100644 --- a/t/parse_assignments.pl +++ b/t/parse_assignments.pl @@ -44,8 +44,16 @@ my @test_cases = ( tc('unquoted escaped backslash', "a=\\\\", { a => "\\" }, ""), tc('squoted escaped squote', "a='\\''", { a => "'" }, ""), tc('dquoted escaped dquote', "a=\"\\\"\"", { a => '"' }, ""), + tc('env: empty', "a_env=", {}, ""), + tc('env: unset', "a_env=UNSET", {}, ""), + tc('env: set', "a_env=TEST", { a => 'val' }, ""), + tc('env: single quoted', "a_env='TEST'", { a => 'val' }, ""), ); +delete($ENV{''}); +delete($ENV{UNSET}); +$ENV{TEST} = 'val'; + for my $tc (@test_cases) { my ($got_rest, %got_vars) = ddclient::parse_assignments($tc->{input}); subtest $tc->{name} => sub { From 270a82dd58fc4e54f0a310b493d25bdfa63b8d68 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 27 Jun 2024 01:02:37 -0400 Subject: [PATCH 16/32] parse_assignments: Support newlines Allow newlines to be in values, but stop searching for assignments once an unescaped/unquoted newline is discovered. This is preparation to using `parse_assignments` to process the `--options` command-line argument, which might have embedded newlines. --- ddclient.in | 9 +++++++-- t/parse_assignments.pl | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 380a617..6b6268a 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1610,6 +1610,9 @@ sub read_recap { ###################################################################### ## parse_assignments(string) return (rest, %variables) ## parse_assignment(string) return (name, value, rest) +# +# Parsing stops upon encountering non-assignment text (e.g., hostname after the assignments) or an +# unquoted/unescaped newline. ###################################################################### sub parse_assignments { my ($rest) = @_; @@ -1617,7 +1620,7 @@ sub parse_assignments { while (1) { (my $name, my $value, $rest) = parse_assignment($rest); - $rest =~ s/^[,\s]+//; + $rest =~ s/^(?:[^\S\n]|,)+//; # Remove leading commas and non-newline whitespace. return ($rest, %variables) if !defined($name); if ($name eq 'fw-banlocal' || $name eq 'if-skip') { warning("'$name' is deprecated and does nothing"); @@ -1631,7 +1634,9 @@ sub parse_assignment { my ($name, $value); my ($escape, $quote) = (0, ''); - if ($rest =~ /^[,\s]*([a-z][0-9a-z_-]*)=(.*)/i) { + # Ignore leading commas and non-newline whitespace. (An unquoted/unescaped newline terminates + # the assignment search.) + if ($rest =~ qr/^(?:[^\S\n]|,)*([a-z][0-9a-z_-]*)=(.*)/is) { ($name, $rest, $value) = ($1, $2, ''); while (length(my $c = substr($rest, 0, 1))) { diff --git a/t/parse_assignments.pl b/t/parse_assignments.pl index d71f4a8..ab965b9 100644 --- a/t/parse_assignments.pl +++ b/t/parse_assignments.pl @@ -48,6 +48,10 @@ my @test_cases = ( tc('env: unset', "a_env=UNSET", {}, ""), tc('env: set', "a_env=TEST", { a => 'val' }, ""), tc('env: single quoted', "a_env='TEST'", { a => 'val' }, ""), + tc('newline: quoted value', "a='1\n2'", { a => "1\n2" }, ""), + tc('newline: escaped value', "a=1\\\n2", { a => "1\n2" }, ""), + tc('newline: between vars', "a=1 \n b=2", { a => '1' }, "\n b=2"), + tc('newline: terminating', "a=1 \n", { a => '1' }, "\n"), ); delete($ENV{''}); From bbf98dd031590c29e762494498d4c1473e342d4d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 27 Jun 2024 01:02:58 -0400 Subject: [PATCH 17/32] Use `parse_assignments` to process `--options` This simplifies the code and enables quoting of special characters. --- ddclient.in | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/ddclient.in b/ddclient.in index 6b6268a..36cefc4 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1913,17 +1913,12 @@ sub init_config { ## define or modify host options specified on the command-line if (defined($opt{'options'})) { - ## collect cmdline configuration options. - my %options = (); - for my $opt (split_by_comma($opt{'options'})) { - my ($name, $var) = split /\s*=\s*/, $opt; - if ($name eq 'fw-banlocal' || $name eq 'if-skip') { - warning("'$name' is deprecated and does nothing"); - next; - } - # TODO: Shouldn't *_env options be processed like _read_config does? - $options{$name} = $var; - } + # TODO: Perhaps the --options argument should be processed like the contents of the config + # file: each line (after removing any comments or continuations) either specifies global + # values or host-specific settings. For now, non-value newlines and end-of-line host + # declarations are rejected. + my ($rest, %options) = parse_assignments($opt{'options'}); + fatal("unexpected content in '--options' argument: $rest") if $rest ne ''; ## determine hosts specified with --host my @hosts = (); if (exists $opt{'host'}) { @@ -1998,11 +1993,6 @@ sub init_config { ## make sure config entries have all defaults and they meet minimums ## first the globals... for my $k (keys %globals) { - # Make sure any _env suffixed variables look at their original entry - # TODO: Didn't _read_config already handle *_env? Or is this needed to handle - # the --options command-line option whose values were merged into %globals above? - $k = $1 if $k =~ /^(.*)_env$/; - # TODO: This might grab an arbitrary protocol-specific variable, which could cause # surprising behavior. my $def = $variables{'merged'}{$k}; @@ -2050,9 +2040,6 @@ sub init_config { # TODO: This silently ignores unknown options passed via --options. for my $k (keys %$svars) { - # Make sure any _env suffixed variables look at their original entry - $k = $1 if $k =~ /^(.*)_env$/; - my $def = $svars->{$k}; # TODO: Why doesn't this try %opt and %globals before falling back to the variable # default? By ignoring %opt and %globals, `--options` will not have any effect on any From 775b7fcbfe5fed3243042e0dd523991bd2de7671 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 26 Jun 2024 01:59:33 -0400 Subject: [PATCH 18/32] Validate and normalize all command-line arguments --- ddclient.in | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/ddclient.in b/ddclient.in index 36cefc4..67279ba 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1292,14 +1292,6 @@ sub main { init_config(); read_recap(opt('cache'), \%recap); print_info() if opt('debug') && opt('verbose'); - - fatal("invalid argument '--use=%s'; possible values are:\n%s", - opt('use'), join("\n", ip_strategies_usage())) - if defined(opt('use')) && !$ip_strategies{lc(opt('use'))}; - fatal("invalid argument '--usev6=%s'; possible values are:\n%s", - opt('usev6'), join("\n", ipv6_strategies_usage())) - if defined(opt('usev6')) && !$ipv6_strategies{lc(opt('usev6'))}; - $daemon = opt('daemon'); update_nics(); @@ -1864,6 +1856,15 @@ sub _read_config { ###################################################################### sub init_config { %opt = %saved_opt; + # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause + # surprising behavior. + for my $var (keys(%{$variables{'merged'}})) { + # TODO: Also validate $opt{'options'}. + next if !defined($opt{$var}) || ref($opt{$var}); + if (!eval { $opt{$var} = check_value($opt{$var}, $variables{'merged'}{$var}); 1; }) { + fatal("invalid argument '--$var=$opt{$var}': $@"); + } + } ## $opt{'quiet'} = 0 if opt('verbose'); @@ -1904,13 +1905,6 @@ sub init_config { $opt{'min-interval'} = max(interval(opt('min-interval')), interval(default('min-interval'))); $opt{'min-error-interval'} = max(interval(opt('min-error-interval')), interval(default('min-error-interval'))); - $opt{'timeout'} = 0 if opt('timeout') < 0; - - ## parse an interval expression (such as '5m') into number of seconds - $opt{'daemon'} = interval(opt('daemon')) if defined($opt{'daemon'}); - ## make sure the interval isn't too short - $opt{'daemon'} = minimum('daemon') if opt('daemon') && opt('daemon') < minimum('daemon'); - ## define or modify host options specified on the command-line if (defined($opt{'options'})) { # TODO: Perhaps the --options argument should be processed like the contents of the config @@ -2347,7 +2341,6 @@ sub sendmail { ###################################################################### ## split_by_comma ## default -## minimum ## opt ###################################################################### sub split_by_comma { @@ -2363,13 +2356,6 @@ sub default { # surprising behavior. return $variables{'merged'}{$v}{'default'}; } -sub minimum { - my $v = shift; - return undef if !defined($variables{'merged'}{$v}); - # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause - # surprising behavior. - return $variables{'merged'}{$v}{'minimum'}; -} sub opt { my $v = shift; my $h = shift; From fe1768316a395d576b3b1e55977f2e5b28e3f89f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 30 Jun 2024 23:40:46 -0400 Subject: [PATCH 19/32] Use protocol-specific default when known Different protocols can have different default values for a particular variable. Grab the protocol-specific variable definition if given a hostname when looking up the variable's default. --- ddclient.in | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 67279ba..e0f6c9a 100755 --- a/ddclient.in +++ b/ddclient.in @@ -2350,7 +2350,12 @@ sub split_by_comma { return (); } sub default { - my $v = shift; + my ($v, $h) = @_; + if (defined($h) && $config{$h}) { + my $proto = $protocols{$config{$h}{'protocol'}}; + my $var = $proto->{variables}{$v} if $proto; + return $var->{default} if $var; + } return undef if !defined($variables{'merged'}{$v}); # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause # surprising behavior. @@ -2363,7 +2368,7 @@ sub opt { # TODO: Why check %opt before %globals? Valid variables from %opt are merged into %globals by # init_config(), so it shouldn't be necessary. Also, it runs the risk of collision with a # non-variable command line option like `--version`, `--help`, etc. - return $opt{$v} // $globals{$v} // default($v); + return $opt{$v} // $globals{$v} // default($v, $h); } sub min { my $min = shift; From 7fde55c1882f05889271baa30422e9530e0e2e25 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 30 Jun 2024 23:44:54 -0400 Subject: [PATCH 20/32] Don't initialize `%opt` entries to `undef` There's no need -- if the key doesn't exist the value returned is already `undef`. This prevents debug output from being littered with `undef` lines. --- ddclient.in | 1 - 1 file changed, 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index e0f6c9a..e82af89 100755 --- a/ddclient.in +++ b/ddclient.in @@ -2098,7 +2098,6 @@ sub process_args { next if !ref($_); my ($key, $specifier) = @$_; push @spec, $key . $specifier; - $opt{$key} //= undef; } if (!GetOptions(\%opt, @spec)) { $opt{'help'}('', ''); From 478f517d533d2fe3b174ab26e528538fd0656ce1 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 14:34:05 -0400 Subject: [PATCH 21/32] Delete no-effect option normalization The alternative is to "fix" the code to match the original intention, but users haven't been complaining so it's better to avoid the risk of introducing a new bug in the fix. --- ddclient.in | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/ddclient.in b/ddclient.in index e82af89..53db6ed 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1869,42 +1869,6 @@ sub init_config { ## $opt{'quiet'} = 0 if opt('verbose'); - ## infer the IP strategy if possible - if (!$opt{'use'}) { - # TODO: I don't think these have any effect. The value is not copied into the - # host-specific %config settings, and $config{$h}{use} is initialized to the variable - # default ("ip"), not set to undef, so opt() won't fall back to this. - $opt{'use'} = 'web' if ($opt{'web'}); - $opt{'use'} = 'if' if ($opt{'if'}); - $opt{'use'} = 'ip' if ($opt{'ip'}); - } - ## infer the IPv4 strategy if possible - if (!$opt{'usev4'}) { - # TODO: I don't think these have any effect. The value is not copied into the - # host-specific %config settings, and $config{$h}{usev4} is initialized to the variable - # default ("disabled"), not set to undef, so opt() won't fall back to this. - $opt{'usev4'} = 'webv4' if ($opt{'webv4'}); - $opt{'usev4'} = 'ifv4' if ($opt{'ifv4'}); - $opt{'usev4'} = 'ipv4' if ($opt{'ipv4'}); - } - ## infer the IPv6 strategy if possible - if (!$opt{'usev6'}) { - # TODO: I don't think these have any effect. The value is not copied into the - # host-specific %config settings, and $config{$h}{usev6} is initialized to the variable - # default ("disabled"), not set to undef, so opt() won't fall back to this. - $opt{'usev6'} = 'webv6' if ($opt{'webv6'}); - $opt{'usev6'} = 'ifv6' if ($opt{'ifv6'}); - $opt{'usev6'} = 'ipv6' if ($opt{'ipv6'}); - } - - ## sanity check - # TODO: I don't think these have any effect. The values are not copied into the host-specific - # %config settings, and $config{$h}{$opt} is initialized to the non-undef variable default so - # opt() won't fall back to these. - $opt{'max-interval'} = min(interval(opt('max-interval')), interval(default('max-interval'))); - $opt{'min-interval'} = max(interval(opt('min-interval')), interval(default('min-interval'))); - $opt{'min-error-interval'} = max(interval(opt('min-error-interval')), interval(default('min-error-interval'))); - ## define or modify host options specified on the command-line if (defined($opt{'options'})) { # TODO: Perhaps the --options argument should be processed like the contents of the config From 5a66efe79ed90384b30e4bb28278578cd874112a Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 16:12:20 -0400 Subject: [PATCH 22/32] Delete unnecessary `defined()` check --- ddclient.in | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ddclient.in b/ddclient.in index 53db6ed..7d244db 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1959,15 +1959,11 @@ sub init_config { delete($globals{$k}); next; } - # TODO: Isn't $globals{$k} guaranteed to be defined here? Otherwise $k wouldn't appear in - # %globals. - my $ovalue = $globals{$k} // $def->{'default'}; # _read_config already checked any value from the config file, so the purpose of this check # is to validate command-line options which were merged into %globals above. # TODO: Move this check to where the command-line options are actually processed. - if (!eval { $globals{$k} = check_value($ovalue, $def); 1; }) { - $ovalue //= '(not set)'; - warning("ignoring invalid variable value '$k=$ovalue': $@"); + if (!eval { $globals{$k} = check_value($globals{$k}, $def); 1; }) { + warning("ignoring invalid variable value '$k=$globals{$k}': $@"); if ($def->{'required'}) { # TODO: What's the point of this? The opt() function will fall back to the default # value if $globals{$k} is undefined. From 912bc6291ab804ba1c42c223922745e0bd952b5b Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 18 Aug 2024 01:14:51 -0400 Subject: [PATCH 23/32] group_hosts_by: Treat `undef` as unset for consistency with `opt` The `opt` function falls back to global/default if the value is undefined, even if it is explicitly set to `undef`. `group_hosts_by` should behave the same. --- ddclient.in | 2 +- t/group_hosts_by.pl | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ddclient.in b/ddclient.in index 7d244db..d8fbbf8 100755 --- a/ddclient.in +++ b/ddclient.in @@ -3381,7 +3381,7 @@ sub group_hosts_by { my %groups; my %cfgs; for my $h (@$hosts) { - my %cfg = map({ ($_ => $config{$h}{$_}); } grep(exists($config{$h}{$_}), @attrs)); + my %cfg = map({ ($_ => $config{$h}{$_}); } grep(defined($config{$h}{$_}), @attrs)); my $sig = repr(\%cfg, Indent => 0); push(@{$groups{$sig}}, $h); $cfgs{$sig} = \%cfg; diff --git a/t/group_hosts_by.pl b/t/group_hosts_by.pl index 4e2c29f..4cf25a1 100644 --- a/t/group_hosts_by.pl +++ b/t/group_hosts_by.pl @@ -72,7 +72,9 @@ my @test_cases = ( want => [ {cfg => {falsy => 0}, hosts => [$h1]}, {cfg => {falsy => ''}, hosts => [$h2]}, - {cfg => {falsy => undef}, hosts => [$h3]}, + # undef intentionally becomes unset because undef always means "fall back to global or + # default". + {cfg => {}, hosts => [$h3]}, ], }, { @@ -80,8 +82,9 @@ my @test_cases = ( groupby => [qw(maybeunset)], want => [ {cfg => {maybeunset => 'unique'}, hosts => [$h1]}, - {cfg => {maybeunset => undef}, hosts => [$h2]}, - {cfg => {}, hosts => [$h3]}, + # undef intentionally becomes unset because undef always means "fall back to global or + # default". + {cfg => {}, hosts => [$h2, $h3]}, ], }, { From 2b65aff56b5b5a1f70854dcb17f0053318fff460 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 2 Jul 2024 01:26:52 -0400 Subject: [PATCH 24/32] Use `opt()` instead of accessing `%config` directly This makes it possible to leave `$config{$h}{$var}` undefined to have it fall back to `%opt`, `%globals`, or the variable's default value. --- ddclient.in | 290 ++++++++++++++++++++++++++-------------------------- 1 file changed, 146 insertions(+), 144 deletions(-) diff --git a/ddclient.in b/ddclient.in index d8fbbf8..7ba3c47 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1362,7 +1362,7 @@ sub update_nics { for my $h (sort keys %config) { local $_l = pushlogctx($h); - next if $config{$h}{'protocol'} ne $p; + next if opt('protocol', $h) ne $p; $examined{$h} = 1; # we only do this once per 'use' and argument combination my $use = opt('use', $h); @@ -1479,7 +1479,7 @@ sub update_nics { local $_l = pushlogctx($h); if (!exists $examined{$h}) { failed("not updated because protocol is not supported: " . - $config{$h}{'protocol'} // ''); + opt('protocol', $h) // ''); } } write_recap(opt('cache')); @@ -1525,14 +1525,14 @@ sub write_recap { # TODO: Why are different variables saved to the recap depending on whether an update was # attempted or not? Shouldn't the same variables be saved every time? if (!exists $recap{$h} || $config{$h}{'update'}) { - my $vars = $protocols{$config{$h}{protocol}}{variables}; + my $vars = $protocols{opt('protocol', $h)}{variables}; for my $v (keys(%$vars)) { - next if !$vars->{$v}{recap} || !defined($config{$h}{$v}); - $recap{$h}{$v} = $config{$h}{$v}; + next if !$vars->{$v}{recap} || !defined(opt($v, $h)); + $recap{$h}{$v} = opt($v, $h); } } else { for my $v (qw(atime wtime status status-ipv4 status-ipv6)) { - $recap{$h}{$v} = $config{$h}{$v}; + $recap{$h}{$v} = opt($v, $h); } } } @@ -2311,7 +2311,7 @@ sub split_by_comma { sub default { my ($v, $h) = @_; if (defined($h) && $config{$h}) { - my $proto = $protocols{$config{$h}{'protocol'}}; + my $proto = $protocols{opt('protocol', $v eq 'protocol' ? undef : $h)}; my $var = $proto->{variables}{$v} if $proto; return $var->{default} if $var; } @@ -2475,14 +2475,13 @@ sub interval { return $value; } sub interval_expired { - my ($host, $time, $interval) = @_; - - return 0 if ($config{$host}{$interval} // 0) == 'inf'; + my ($host, $time, $interval_opt) = @_; + my $interval = opt($interval_opt, $host); + return 0 if ($interval // 0) == 'inf'; return 1 if !exists $recap{$host}; return 1 if !exists $recap{$host}{$time} || !$recap{$host}{$time}; - return 1 if !exists $config{$host}{$interval} || !$config{$host}{$interval}; - - return $now > ($recap{$host}{$time} + $config{$host}{$interval}); + return 1 if !$interval; + return $now > ($recap{$host}{$time} + $interval); } @@ -3381,7 +3380,7 @@ sub group_hosts_by { my %groups; my %cfgs; for my $h (@$hosts) { - my %cfg = map({ ($_ => $config{$h}{$_}); } grep(defined($config{$h}{$_}), @attrs)); + my %cfg = map({ ($_ => opt($_, $h)); } grep(defined(opt($_, $h)), @attrs)); my $sig = repr(\%cfg, Indent => 0); push(@{$groups{$sig}}, $h); $cfgs{$sig} = \%cfg; @@ -3489,7 +3488,7 @@ EoEXAMPLE ###################################################################### sub nic_updateable { my ($host) = @_; - my $force_update = $protocols{$config{$host}{protocol}}{force_update}; + my $force_update = $protocols{opt('protocol', $host)}{force_update}; my $update = 0; my $ip = $config{$host}{'wantip'}; my $ipv4 = $config{$host}{'wantipv4'}; @@ -3503,7 +3502,7 @@ sub nic_updateable { my $previpv6 = $recap{$host}{'ipv6'} || ''; my %prettyt = map({ ($_ => $recap{$host}{$_} ? prettytime($recap{$host}{$_}) : ''); } qw(atime mtime wtime)); - my %prettyi = map({ ($_ => prettyinterval($config{$host}{$_})); } + my %prettyi = map({ ($_ => prettyinterval(opt($_, $host))); } qw(max-interval min-error-interval min-interval)); $warned_ip{$host} = 0 if $use ne 'disabled' && $ip; @@ -3611,7 +3610,7 @@ sub nic_updateable { } elsif (defined($force_update) && $force_update->($host)) { $update = 1; - } elsif (my @changed = grep({ my $rv = $recap{$host}{$_}; my $cv = $config{$host}{$_}; + } elsif (my @changed = grep({ my $rv = $recap{$host}{$_}; my $cv = opt($_, $host); defined($rv) && defined($cv) && $rv ne $cv; } qw(static wildcard mx backupmx))) { info("update forced because options changed: " . join(', ', @changed)); @@ -3740,22 +3739,22 @@ sub nic_dyndns1_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$h}{'server'}/nic/"; - $url .= ynu($config{$h}{'static'}, 'statdns', 'dyndns', 'dyndns'); + $url = 'https://' . opt('server', $h) . '/nic/'; + $url .= ynu(opt('static', $h), 'statdns', 'dyndns', 'dyndns'); $url .= "?action=edit&started=1&hostname=YES&host_id=$h"; $url .= "&myip="; $url .= $ip if $ip; - $url .= "&wildcard=ON" if ynu($config{$h}{'wildcard'}, 1, 0, 0); - if ($config{$h}{'mx'}) { - $url .= "&mx=$config{$h}{'mx'}"; - $url .= "&backmx=" . ynu($config{$h}{'backupmx'}, 'YES', 'NO'); + $url .= "&wildcard=ON" if ynu(opt('wildcard', $h), 1, 0, 0); + if (opt('mx', $h)) { + $url .= '&mx=' . opt('mx', $h); + $url .= "&backmx=" . ynu(opt('backupmx', $h), 'YES', 'NO'); } my $reply = geturl( proxy => opt('proxy'), url => $url, - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ); next if !header_ok($reply); @@ -3769,7 +3768,7 @@ sub nic_dyndns1_update { if ($return_code ne 'NOERROR' || $error_code ne 'NOERROR' || !$title) { $config{$h}{'status'} = 'failed'; - $title = "incomplete response from $config{$h}{server}" unless $title; + $title = 'incomplete response from ' . opt('server', $h) unless $title; warning("SENT: %s", $url) unless opt('verbose'); warning("REPLIED: %s", $reply); failed($title); @@ -4001,7 +4000,7 @@ sub nic_dnsexit2_update { # The DNSExit API does not support updating hosts with different zones at the same time, # handling update per host. for my $h (@_) { - $config{$h}{'zone'} //= $h; + $config{$h}{'zone'} = $h if !defined(opt('zone', $h)); dnsexit2_update_host($h); } } @@ -4013,10 +4012,11 @@ sub dnsexit2_update_host { # Remove the zone suffix from $name. If the zone eq $name, $name can be left alone or # set to the empty string; both have identical semantics. For consistency, always # remove the zone even if it means $name becomes the empty string. - if ($name =~ s/(?:^|\.)\Q$config{$h}{'zone'}\E$//) { + my $zone = opt('zone', $h); + if ($name =~ s/(?:^|\.)\Q$zone\E$//) { # The zone was successfully trimmed from $name. } else { - fatal("hostname does not end with the zone: $config{$h}{'zone'}"); + fatal("hostname does not end with the zone: " . opt('zone', $h)); } # The IPv4 and IPv6 addresses must be updated together in a single API call. my %ips; @@ -4030,10 +4030,10 @@ sub dnsexit2_update_host { name => $name, type => ($ipv eq '6') ? 'AAAA' : 'A', content => $ip, - ttl => $config{$h}{'ttl'}, + ttl => opt('ttl', $h), }); }; - my $url = $config{$h}{'server'} . $config{$h}{'path'}; + my $url = opt('server', $h) . opt('path', $h); my $reply = geturl( proxy => opt('proxy'), url => $url, @@ -4043,8 +4043,8 @@ sub dnsexit2_update_host { ], method => 'POST', data => encode_json({ - apikey => $config{$h}{'password'}, - domain => $config{$h}{'zone'}, + apikey => opt('password', $h), + domain => $zone, update => \@updates, }), ); @@ -4267,8 +4267,8 @@ sub nic_dslreports1_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$h}{'server'}/nic/"; - $url .= ynu($config{$h}{'static'}, 'statdns', 'dyndns', 'dyndns'); + $url = 'https://' . opt('server', $h) . '/nic/'; + $url .= ynu(opt('static', $h), 'statdns', 'dyndns', 'dyndns'); $url .= "?action=edit&started=1&hostname=YES&host_id=$h"; $url .= "&myip="; $url .= $ip if $ip; @@ -4276,11 +4276,11 @@ sub nic_dslreports1_update { my $reply = geturl( proxy => opt('proxy'), url => $url, - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ) // ''; if ($reply eq '') { - failed("request to $config{$h}{'server'} failed"); + failed("request to " . opt('server', $h) . " failed"); next; } @@ -4341,9 +4341,9 @@ sub nic_domeneshop_update { info("setting IPv$ipv address to $ip"); my $reply = geturl( proxy => opt('proxy'), - url => "$config{$h}{'server'}/v0/dyndns/update?hostname=$h&myip=$ip", - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + url => opt('server', $h) . "/v0/dyndns/update?hostname=$h&myip=$ip", + login => opt('login', $h), + password => opt('password', $h), ); next if !header_ok($reply); $config{$h}{"ipv$ipv"} = $ip; @@ -4524,20 +4524,20 @@ sub nic_easydns_update { my $ip = delete $config{$h}{"wantipv$ipv"} or next; info("setting IPv$ipv address to $ip"); #'https://api.cp.easydns.com/dyn/generic.php?hostname=test.burry.ca&myip=10.20.30.40&wildcard=ON' - my $url = "https://$config{$h}{'server'}$config{$h}{'script'}?hostname=$h&myip=$ip"; - $url .= "&wildcard=" . ynu($config{$h}{'wildcard'}, 'ON', 'OFF', 'OFF') - if defined($config{$h}{'wildcard'}); - $url .= "&mx=$config{$h}{'mx'}&backmx=" . ynu($config{$h}{'backupmx'}, 'YES', 'NO') - if $config{$h}{'mx'}; + my $url = "https://" . opt('server', $h) . opt('script', $h) . "?hostname=$h&myip=$ip"; + $url .= "&wildcard=" . ynu(opt('wildcard', $h), 'ON', 'OFF', 'OFF') + if defined(opt('wildcard', $h)); + $url .= "&mx=" . opt('mx', $h) . "&backmx=" . ynu(opt('backupmx', $h), 'YES', 'NO') + if opt('mx', $h); my $reply = geturl( proxy => opt('proxy'), url => $url, - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ); next if !header_ok($reply); (my $body = $reply) =~ s/^.*?\n\n//s or do { - failed("Could not connect to $config{$h}{'server'}"); + failed("could not connect to " . opt('server', $h)); next; }; my $resultcode_re = join('|', map({quotemeta} 'NOERROR', keys(%errors))); @@ -4608,13 +4608,13 @@ sub nic_namecheap_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$h}{'server'}/update"; - my $domain = $config{$h}{'login'}; + $url = 'https://' . opt('server', $h) . '/update'; + my $domain = opt('login', $h); my $host = $h; $host =~ s/(.*)\.$domain(.*)/$1$2/; $url .= "?host=$host"; $url .= "&domain=$domain"; - $url .= "&password=$config{$h}{'password'}"; + $url .= '&password=' . opt('password', $h); $url .= "&ip="; $url .= $ip if $ip; @@ -4687,7 +4687,7 @@ sub nic_nfsn_gen_auth_header { ## In this header, login is the member login name of the user ## making the API request. my $auth_header = 'X-NFSN-Authentication: '; - $auth_header .= $config{$h}{'login'} . ';'; + $auth_header .= opt('login', $h) . ';'; ## timestamp is the standard 32-bit unsigned Unix timestamp ## value. @@ -4705,10 +4705,10 @@ sub nic_nfsn_gen_auth_header { ## hash is a SHA1 hash of a string in the following format: ## login;timestamp;salt;api-key;request-uri;body-hash - my $hash_string = $config{$h}{'login'} . ';' . + my $hash_string = opt('login', $h) . ';' . $timestamp . ';' . $salt . ';' . - $config{$h}{'password'} . ';'; + opt('password', $h) . ';'; ## The request-uri value is the path portion of the requested URL ## (i.e. excluding the protocol and hostname). @@ -4736,7 +4736,7 @@ sub nic_nfsn_make_request { my $method = shift // 'GET'; my $body = shift // ''; - my $base_url = "https://$config{$h}{'server'}"; + my $base_url = 'https://' . opt('server', $h); my $url = $base_url . $path; my $header = nic_nfsn_gen_auth_header($h, $path, $body); if ($method eq 'POST' && $body ne '') { @@ -4788,7 +4788,7 @@ sub nic_nfsn_update { ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); - my $zone = $config{$h}{'zone'}; + my $zone = opt('zone', $h); my $name; if ($h eq $zone) { @@ -4822,7 +4822,7 @@ sub nic_nfsn_update { next; } - my $rr_ttl = $config{$h}{'ttl'}; + my $rr_ttl = opt('ttl', $h); if (ref($list) eq 'ARRAY' && defined $list->[0]->{'data'}) { my $rr_data = $list->[0]->{'data'}; @@ -4903,11 +4903,11 @@ sub nic_njalla_update { # Read input params my $ipv4 = delete $config{$h}{'wantipv4'}; my $ipv6 = delete $config{$h}{'wantipv6'}; - my $quietreply = $config{$h}{'quietreply'}; + my $quietreply = opt('quietreply', $h); my $ip_output = ''; # Build url - my $url = "https://$config{$h}{'server'}/update/?h=$h&k=$config{$h}{'password'}"; + my $url = 'https://' . opt('server', $h) . "/update/?h=$h&k=" . opt('password', $h); my $auto = 1; for my $ip ($ipv4, $ipv6) { next if (!$ip); @@ -4944,7 +4944,7 @@ sub nic_njalla_update { $response = eval {decode_json(${^MATCH})}; # No response, declare as failed if (!defined($reply) || !$reply) { - failed("could not connect to $config{$h}{'server'}"); + failed("could not connect to " . opt('server', $h)); } else { # Strip header if ($response->{status} == 401 && $response->{message} =~ /invalid host or key/) { @@ -5011,10 +5011,10 @@ sub nic_sitelutions_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$h}{'server'}/dnsup"; + $url = 'https://' . opt('server', $h) . '/dnsup'; $url .= "?id=$h"; - $url .= "&user=$config{$h}{'login'}"; - $url .= "&pass=$config{$h}{'password'}"; + $url .= '&user=' . opt('login', $h); + $url .= '&pass=' . opt('password', $h); $url .= "&ip="; $url .= $ip if $ip; @@ -5097,8 +5097,8 @@ sub nic_freedns_update { # address type. my %recs_ipv4; my %recs_ipv6; - my $url_tmpl = "https://$config{$_[0]}{'server'}/api/?action=getdyndns&v=2&sha="; - my $creds = sha1_hex("$config{$_[0]}{'login'}|$config{$_[0]}{'password'}"); + my $url_tmpl = 'https://' . opt('server', $_[0]) . '/api/?action=getdyndns&v=2&sha='; + my $creds = sha1_hex(opt('login', $_[0]) . '|' . opt('password', $_[0])); (my $url = $url_tmpl) =~ s//$creds/; my $reply = geturl(proxy => opt('proxy'), @@ -5223,8 +5223,8 @@ sub nic_1984_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$host}{'server'}/1.0/freedns/"; - $url .= "?apikey=$config{$host}{'password'}"; + $url = 'https://' . opt('server', $host) . '/1.0/freedns/'; + $url .= '?apikey=' . opt('password', $host); $url .= "&domain=$host"; $url .= "&ip=$ip"; @@ -5299,7 +5299,7 @@ sub nic_changeip_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$h}{'server'}/nic/update"; + $url = 'https://' . opt('server', $h) . '/nic/update'; $url .= "?hostname=$h"; $url .= "&ip="; $url .= $ip if $ip; @@ -5307,8 +5307,8 @@ sub nic_changeip_update { my $reply = geturl( proxy => opt('proxy'), url => $url, - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ); next if !header_ok($reply); @@ -5371,31 +5371,31 @@ EoEXAMPLE sub nic_godaddy_update { for my $h (@_) { local $_l = pushlogctx($h); - my $zone = $config{$h}{'zone'}; + my $zone = opt('zone', $h); (my $hostname = $h) =~ s/\.\Q$zone\E$//; for my $ipv ('4', '6') { my $ip = delete($config{$h}{"wantipv$ipv"}) or next; info("setting IPv$ipv address to $ip"); my $rrset_type = ($ipv eq '6') ? 'AAAA' : 'A'; - my $url = "https://$config{$h}{'server'}/$zone/records/$rrset_type/$hostname"; + my $url = "https://" . opt('server', $h) . "/$zone/records/$rrset_type/$hostname"; my $reply = geturl( proxy => opt('proxy'), url => $url, headers => [ 'Content-Type: application/json', 'Accept: application/json', - "Authorization: sso-key $config{$h}{'login'}:$config{$h}{'password'}", + "Authorization: sso-key " . opt('login', $h) . ":" . opt('password', $h), ], method => 'PUT', data => encode_json([{ data => $ip, - defined($config{$h}{'ttl'}) ? (ttl => $config{$h}{'ttl'}) : (), + defined(opt('ttl', $h)) ? (ttl => opt('ttl', $h)) : (), name => $hostname, type => $rrset_type, }]), ); unless ($reply) { - failed("could not connect to $config{$h}{'server'}"); + failed("could not connect to " . opt('server', $h)); next; } (my $code) = ($reply =~ m%^s*HTTP/.*\s+(\d+)%i); @@ -5413,7 +5413,7 @@ sub nic_godaddy_update { if ($code eq "400") { $msg = 'GoDaddy API URL ($url) was malformed.'; } elsif ($code eq "401") { - if ($config{$h}{'login'}) { + if (opt('login', $h)) { $msg = 'login or password option incorrect.'; } else { $msg = 'login or password option missing.'; @@ -5488,9 +5488,9 @@ sub nic_henet_update { info("setting IPv$ipv address to $ip"); my $reply = geturl( proxy => opt('proxy'), - url => "https://$config{$h}{'server'}/nic/update?hostname=$h&myip=$ip", + url => "https://" . opt('server', $h) . "/nic/update?hostname=$h&myip=$ip", login => $h, - password => $config{$h}{'password'}, + password => opt('password', $h), ); next if !header_ok($reply); # dyn.dns.he.net can return 200 OK even if there is an error (e.g., bad authentication, @@ -5571,10 +5571,10 @@ sub nic_mythicdyn_update { info("Process configuration for IPV%s --------", $mythver); my $reply = geturl( proxy => opt('proxy'), - url => "https://ipv$mythver.$config{$h}{'server'}/dns/v2/dynamic/$h", + url => "https://ipv$mythver." . opt('server', $h) . "/dns/v2/dynamic/$h", method => 'POST', - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ipversion => $mythver, ); my $ok = header_ok($reply); @@ -5673,7 +5673,7 @@ EoINSTR1 my $type = ($ip eq ($ipv6 // '')) ? 'AAAA' : 'A'; $instructions .= <<"EoINSTR2"; update delete $_. $type -update add $_. $config{$_}{'ttl'} $type $ip +update add $_. ${\(opt('ttl', $_))} $type $ip EoINSTR2 } } @@ -5774,8 +5774,8 @@ sub nic_cloudflare_update { info('getting Cloudflare Zone ID'); # Get zone ID - my $url = "https://$config{$domain}{'server'}/zones/?"; - $url .= "name=" . $config{$domain}{'zone'}; + my $url = "https://" . opt('server', $domain) . "/zones/?"; + $url .= "name=" . opt('zone', $domain); my $reply = geturl(proxy => opt('proxy'), url => $url, @@ -5791,9 +5791,9 @@ sub nic_cloudflare_update { } # Pull the ID out of the json, messy - my ($zone_id) = map {$_->{name} eq $config{$domain}{'zone'} ? $_->{id} : ()} @{$response->{result}}; + my ($zone_id) = map {$_->{name} eq opt('zone', $domain) ? $_->{id} : ()} @{$response->{result}}; unless ($zone_id) { - failed("no zone ID found for zone $config{$domain}{'zone'}"); + failed("no zone ID found for zone " . opt('zone', $domain)); next; } info("Zone ID is %s", $zone_id); @@ -5809,7 +5809,7 @@ sub nic_cloudflare_update { $config{$domain}{"status-ipv$ipv"} = 'failed'; # Get DNS 'A' or 'AAAA' record ID - $url = "https://$config{$domain}{'server'}/zones/$zone_id/dns_records?"; + $url = "https://" . opt('server', $domain) . "/zones/$zone_id/dns_records?"; $url .= "type=$type&name=$domain"; $reply = geturl(proxy => opt('proxy'), url => $url, @@ -5831,7 +5831,7 @@ sub nic_cloudflare_update { } debug("DNS '$type' record ID: $dns_rec_id"); # Set domain - $url = "https://$config{$domain}{'server'}/zones/$zone_id/dns_records/$dns_rec_id"; + $url = "https://" . opt('server', $domain) . "/zones/$zone_id/dns_records/$dns_rec_id"; my $data = "{\"content\":\"$ip\"}"; $reply = geturl(proxy => opt('proxy'), url => $url, @@ -5888,17 +5888,18 @@ EoEXAMPLE sub nic_hetzner_update { for my $domain (@_) { local $_l = pushlogctx($domain); - my $headers = "Auth-API-Token: $config{$domain}{'password'}\n"; + my $headers = "Auth-API-Token: " . opt('password', $domain) . "\n"; $headers .= "Content-Type: application/json"; - (my $hostname = $domain) =~ s/\Q.$config{$domain}{zone}\E$//; + my $zone = opt('zone', $domain); + (my $hostname = $domain) =~ s/\Q.$zone\E$//; my $ipv4 = delete $config{$domain}{'wantipv4'}; my $ipv6 = delete $config{$domain}{'wantipv6'}; info("getting Hetzner Zone ID"); # Get zone ID - my $url = "https://$config{$domain}{'server'}/zones?name=" . $config{$domain}{'zone'}; + my $url = "https://" . opt('server', $domain) . "/zones?name=$zone"; my $reply = geturl(proxy => opt('proxy'), url => $url, @@ -5914,9 +5915,9 @@ sub nic_hetzner_update { } # Pull the ID out of the json, messy - my ($zone_id) = map {$_->{name} eq $config{$domain}{'zone'} ? $_->{id} : ()} @{$response->{zones}}; + my ($zone_id) = map {$_->{name} eq $zone ? $_->{id} : ()} @{$response->{zones}}; unless ($zone_id) { - failed("no zone ID found for zone $config{$domain}{'zone'}"); + failed("no zone ID found for zone " . opt('zone', $domain)); next; } info("Zone ID is %s", $zone_id); @@ -5931,7 +5932,7 @@ sub nic_hetzner_update { $config{$domain}{"status-ipv$ipv"} = 'failed'; # Get DNS 'A' or 'AAAA' record ID - $url = "https://$config{$domain}{'server'}/records?zone_id=$zone_id"; + $url = "https://" . opt('server', $domain) . "/records?zone_id=$zone_id"; $reply = geturl(proxy => opt('proxy'), url => $url, headers => $headers @@ -5952,14 +5953,14 @@ sub nic_hetzner_update { if ($dns_rec_id) { debug("DNS '$type' record ID: $dns_rec_id"); - $url = "https://$config{$domain}{'server'}/records/$dns_rec_id"; + $url = "https://" . opt('server', $domain) . "/records/$dns_rec_id"; $http_method = "PUT"; } else { debug("creating DNS '$type'"); - $url = "https://$config{$domain}{'server'}/records"; + $url = "https://" . opt('server', $domain) . "/records"; $http_method = "POST"; } - my $data = "{\"zone_id\":\"$zone_id\", \"name\": \"$hostname\", \"value\": \"$ip\", \"type\": \"$type\", \"ttl\": $config{$domain}{'ttl'}}"; + my $data = "{\"zone_id\":\"$zone_id\", \"name\": \"$hostname\", \"value\": \"$ip\", \"type\": \"$type\", \"ttl\": " . opt('ttl', $domain) . "}"; $reply = geturl(proxy => opt('proxy'), url => $url, @@ -6184,14 +6185,14 @@ sub nic_yandex_update { for my $host (@_) { local $_l = pushlogctx($host); my $ip = delete $config{$host}{'wantip'}; - my $headers = "PddToken: $config{$host}{'password'}\n"; + my $headers = "PddToken: " . opt('password', $host) . "\n"; info("setting IP address to $ip"); # Get record ID for host - my $url = "https://$config{$host}{'server'}/api2/admin/dns/list?"; + my $url = "https://" . opt('server', $host) . "/api2/admin/dns/list?"; $url .= "domain="; - $url .= $config{$host}{'login'}; + $url .= opt('login', $host); my $reply = geturl(proxy => opt('proxy'), url => $url, headers => $headers); next if !header_ok($reply); @@ -6211,9 +6212,9 @@ sub nic_yandex_update { } # Update the DNS record - $url = "https://$config{$host}{'server'}/api2/admin/dns/edit"; + $url = "https://" . opt('server', $host) . "/api2/admin/dns/edit"; my $data = "domain="; - $data .= $config{$host}{'login'}; + $data .= opt('login', $host); $data .= "&record_id="; $data .= $id; $data .= "&content="; @@ -6348,7 +6349,7 @@ sub nic_freemyip_update { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; info("setting IP address to $ip"); - my $url = "https://$config{$h}{'server'}/update?token=$config{$h}{'password'}&domain=$h"; + my $url = "https://" . opt('server', $h) . "/update?token=" . opt('password', $h) . "&domain=$h"; my $reply = geturl(proxy => opt('proxy'), url => $url); next if !header_ok($reply); (my $body = $reply) =~ s/^.*?\n\n//s; @@ -6403,7 +6404,7 @@ sub nic_ddnsfm_update { info("setting IPv$ipv address to $ip"); my $reply = geturl( proxy => opt('proxy'), - url => "$config{$h}{server}/update?key=$config{$h}{password}&domain=$h&myip=$ip", + url => opt('server', $h) . "/update?key=" . opt('password', $h) . "&domain=$h&myip=$ip", ); next if !header_ok($reply); $config{$h}{"ipv$ipv"} = $ip; @@ -6446,7 +6447,7 @@ sub nic_dondominio_update { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; info("setting IP address to $ip"); - my $url = "https://$config{$h}{'server'}/plain/?user=$config{$h}{'login'}&password=$config{$h}{'password'}&host=$h&ip=$ip"; + my $url = "https://" . opt('server', $h) . "/plain/?user=" . opt('login', $h) . "&password=" . opt('password', $h) . "&host=$h&ip=$ip"; my $reply = geturl(proxy => opt('proxy'), url => $url); next if !header_ok($reply); my @reply = split /\n/, $reply; @@ -6509,7 +6510,7 @@ sub nic_dnsmadeeasy_update { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; info("setting IP address to $ip"); - my $url = "$config{$h}{'server'}$config{$h}{'script'}?username=$config{$h}{'login'}&password=$config{$h}{'password'}&ip=$ip&id=$h"; + my $url = opt('server', $h) . opt('script', $h) . "?username=" . opt('login', $h) . "&password=" . opt('password', $h) . "&ip=$ip&id=$h"; my $reply = geturl(proxy => opt('proxy'), url => $url); next if !header_ok($reply); my @reply = split /\n/, $reply; @@ -6567,7 +6568,7 @@ sub nic_ovh_update { # Set the URL that we're going to update my $url; - $url .= "https://$config{$h}{'server'}$config{$h}{'script'}?system=dyndns"; + $url .= 'https://' . opt('server', $h) . opt('script', $h) . '?system=dyndns'; $url .= "&hostname=$h"; $url .= "&myip="; $url .= $ip if $ip; @@ -6575,12 +6576,12 @@ sub nic_ovh_update { my $reply = geturl( proxy => opt('proxy'), url => $url, - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ); if (!defined($reply) || !$reply) { - failed("could not connect to $config{$h}{'server'}"); + failed("could not connect to " . opt('server', $h)); next; } @@ -6682,16 +6683,16 @@ sub nic_porkbun_update { for my $h (@_) { local $_l = pushlogctx($h); my ($sub_domain, $domain); - if ($config{$h}{'root-domain'}) { + if (opt('root-domain', $h)) { warning("both 'root-domain' and 'on-root-domain' are set; ignoring the latter") - if $config{$h}{'on-root-domain'}; - $domain = $config{$h}{'root-domain'}; + if opt('on-root-domain', $h); + $domain = opt('root-domain', $h); $sub_domain = $h; if ($sub_domain !~ s/(?:^|\.)\Q$domain\E$//) { failed("hostname does not end with the 'root-domain' value: $domain"); next; } - } elsif ($config{$h}{'on-root-domain'}) { + } elsif (opt('on-root-domain', $h)) { $sub_domain = ''; $domain = $h; } else { @@ -6708,8 +6709,8 @@ sub nic_porkbun_update { headers => ['Content-Type: application/json'], method => 'POST', data => encode_json({ - secretapikey => $config{$h}{'secretapikey'}, - apikey => $config{$h}{'apikey'}, + secretapikey => opt('secretapikey', $h), + apikey => opt('apikey', $h), }), ); next if !header_ok($reply); @@ -6746,8 +6747,8 @@ sub nic_porkbun_update { headers => ['Content-Type: application/json'], method => 'POST', data => encode_json({ - secretapikey => $config{$h}{'secretapikey'}, - apikey => $config{$h}{'apikey'}, + secretapikey => opt('secretapikey', $h), + apikey => opt('apikey', $h), content => $ip, ttl => $ttl, notes => $notes, @@ -6854,15 +6855,15 @@ sub nic_dinahosting_update { my $ip = delete $config{$h}{'wantip'}; info("setting IP address to $ip"); my ($hostname, $domain) = split(/\./, $h, 2); - my $url = "https://$config{$h}{'server'}$config{$h}{'script'}"; + my $url = 'https://' . opt('server', $h) . opt('script', $h); $url .= "?hostname=$hostname"; $url .= "&domain=$domain"; $url .= "&command=Domain_Zone_UpdateType" . is_ipv6($ip) ? 'AAAA' : 'A'; $url .= "&ip=$ip"; my $reply = geturl( proxy => opt('proxy'), - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), url => $url, ); $config{$h}{'status'} = 'failed'; # assume failure until otherwise determined @@ -6917,7 +6918,7 @@ sub nic_directnic_update { my $ip = delete $config{$h}{"wantipv$ipv"} or next; info("setting IPv$ipv address to $ip"); - my $url = $config{$h}{"urlv$ipv"}; + my $url = opt("urlv$ipv", $h); if (!defined($url)) { failed("missing urlv$ipv option"); next; @@ -7005,16 +7006,17 @@ sub nic_gandi_update { local $_l = pushlogctx($h); for my $ipv ('ipv4', 'ipv6') { my $ip = delete $config{$h}{"want$ipv"} or next; - (my $hostname = $h) =~ s/\.\Q$config{$h}{zone}\E$//; + my $zone = opt('zone', $h); + (my $hostname = $h) =~ s/\.\Q$zone\E$//; info("setting IP address to $ip"); my @headers = ('Content-Type: application/json'); - if ($config{$h}{'use-personal-access-token'} == 1) { - push(@headers, "Authorization: Bearer $config{$h}{'password'}"); + if (opt('use-personal-access-token', $h) == 1) { + push(@headers, "Authorization: Bearer " . opt('password', $h)); } else { - push(@headers, "Authorization: Apikey $config{$h}{'password'}"); + push(@headers, "Authorization: Apikey " . opt('password', $h)); } my $rrset_type = $ipv eq 'ipv6' ? 'AAAA' : 'A'; - my $url = "https://$config{$h}{'server'}$config{$h}{'script'}/livedns/domains/$config{$h}{'zone'}/records/$hostname/$rrset_type"; + my $url = "https://" . opt('server', $h) . opt('script', $h) . "/livedns/domains/$zone/records/$hostname/$rrset_type"; my $reply = geturl( proxy => opt('proxy'), url => $url, @@ -7029,8 +7031,8 @@ sub nic_gandi_update { failed("response is not a JSON object: $reply"); next; } - if ($response->{'rrset_values'}->[0] eq $ip && (!defined($config{$h}{'ttl'}) || - $response->{'rrset_ttl'} eq $config{$h}{'ttl'})) { + if ($response->{'rrset_values'}->[0] eq $ip && (!defined(opt('ttl', $h)) || + $response->{'rrset_ttl'} eq opt('ttl', $h))) { $config{$h}{'ip'} = $ip; $config{$h}{'mtime'} = $now; $config{$h}{"status-$ipv"} = "good"; @@ -7043,7 +7045,7 @@ sub nic_gandi_update { headers => \@headers, method => 'PUT', data => encode_json({ - defined($config{$h}{'ttl'}) ? (rrset_ttl => $config{$h}{'ttl'}) : (), + defined(opt('ttl', $h)) ? (rrset_ttl => opt('ttl', $h)) : (), rrset_values => [$ip], }), ); @@ -7103,7 +7105,7 @@ sub nic_keysystems_update { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; info("setting IP address to $ip"); - my $url = "$config{$h}{'server'}/update.php?hostname=$h&password=$config{$h}{'password'}&ip=$ip"; + my $url = opt('server', $h) . "/update.php?hostname=$h&password=" . opt('password', $h) . "&ip=$ip"; my $reply = geturl(proxy => opt('proxy'), url => $url); last if !header_ok($reply); @@ -7151,7 +7153,7 @@ sub nic_regfishde_update { my $ipv6 = delete $config{$h}{'wantipv6'}; info("setting IPv4 address to $ipv4") if $ipv4; info("setting IPv6 address to $ipv6") if $ipv6; - my $url = "https://$config{$h}{'server'}/?fqdn=$h&forcehost=1&token=$config{$h}{'password'}"; + my $url = 'https://' . opt('server', $h) . "/?fqdn=$h&forcehost=1&token=" . opt('password', $h); $url .= "&ipv4=$ipv4" if $ipv4; $url .= "&ipv6=$ipv6" if $ipv6; @@ -7220,10 +7222,10 @@ sub nic_enom_update { info("setting IP address to $ip"); my $url; - $url = "https://$config{$h}{'server'}/interface.asp?Command=SetDNSHost"; + $url = 'https://' . opt('server', $h) . '/interface.asp?Command=SetDNSHost'; $url .= "&HostName=$h"; - $url .= "&Zone=$config{$h}{'login'}"; - $url .= "&DomainPassword=$config{$h}{'password'}"; + $url .= '&Zone=' . opt('login', $h); + $url .= '&DomainPassword=' . opt('password', $h); $url .= "&Address="; $url .= $ip if $ip; @@ -7283,15 +7285,15 @@ sub nic_digitalocean_update_one { info("setting $ipv address to $ip"); - my $server = $config{$h}{'server'}; + my $server = opt('server', $h); my $type = $ipv eq 'ipv6' ? 'AAAA' : 'A'; my $headers; $headers = "Content-Type: application/json\n"; - $headers .= "Authorization: Bearer $config{$h}{'password'}\n"; + $headers .= 'Authorization: Bearer ' . opt('password', $h) . "\n"; my $list_url; - $list_url = "https://$server/v2/domains/$config{$h}{'zone'}/records"; + $list_url = "https://$server/v2/domains/" . opt('zone', $h) . '/records'; $list_url .= "?name=$h"; $list_url .= "&type=$type"; @@ -7328,7 +7330,7 @@ sub nic_digitalocean_update_one { my $update_data = encode_json({'type' => $type, 'data' => $ip}); my $update_resp = geturl( proxy => opt('proxy'), - url => "https://$server/v2/domains/$config{$h}{'zone'}/records/$record_id", + url => "https://$server/v2/domains/" . opt('zone', $h) . "/records/$record_id", method => 'PATCH', headers => $headers, data => $update_data, @@ -7433,8 +7435,8 @@ sub nic_infomaniak_update { my $reply = geturl( proxy => opt('proxy'), url => "https://infomaniak.com/nic/update?hostname=$h&myip=$ip", - login => $config{$h}{'login'}, - password => $config{$h}{'password'}, + login => opt('login', $h), + password => opt('password', $h), ); next if !header_ok($reply); (my $body = $reply) =~ s/^.*?\n\n//s; @@ -7465,8 +7467,8 @@ sub nic_infomaniak_update { ## host must be specified; the host names are mentioned in the email. ###################################################################### sub nic_emailonly_update { - # Note: This is logged after $config{$_}{'max-interval'] even if the IP address hasn't changed, - # so it is best to avoid phrasing like, "IP address has changed." + # Note: This is logged after opt('max-interval', $_) even if the IP address hasn't changed, so + # it is best to avoid phrasing like, "IP address has changed." logmsg(email => 1, raw => 1, join("\n", 'Host IP addresses:', map({ my $ipv4 = delete($config{$_}{'wantipv4'}); my $ipv6 = delete($config{$_}{'wantipv6'}); From e8f0358bbb88b9c68398a69e115c8601320ad11b Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 16:18:01 -0400 Subject: [PATCH 25/32] Rely on `opt()` fallback if a value is invalid --- ddclient.in | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/ddclient.in b/ddclient.in index 7ba3c47..9e23ebb 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1964,13 +1964,7 @@ sub init_config { # TODO: Move this check to where the command-line options are actually processed. if (!eval { $globals{$k} = check_value($globals{$k}, $def); 1; }) { warning("ignoring invalid variable value '$k=$globals{$k}': $@"); - if ($def->{'required'}) { - # TODO: What's the point of this? The opt() function will fall back to the default - # value if $globals{$k} is undefined. - $globals{$k} = default($k); - } else { - $globals{$k} = undef; - } + delete($globals{$k}); } } @@ -1994,26 +1988,19 @@ sub init_config { # TODO: This silently ignores unknown options passed via --options. for my $k (keys %$svars) { + next if !defined($config{$h}{$k}); my $def = $svars->{$k}; - # TODO: Why doesn't this try %opt and %globals before falling back to the variable - # default? By ignoring %opt and %globals, `--options` will not have any effect on any - # hosts that are not specified on the command line (via `--host=a,b` and/or - # `--options=host=c,d`). - # TODO: Why not just leave $conf->{$k} undef? Then opt() would automatically fall back - # to %opt, %globals, or the variable default. - my $ovalue = $config{$h}{$k} // $def->{'default'}; # _read_config already checked any value from the config file, so the purpose of this # check is to validate command-line options from --options which were merged into # $config{$h} above. # TODO: Move this check to where --options is actually processed. - if (!eval { $conf->{$k} = check_value($ovalue, $def); 1; }) { - $ovalue //= '(not set)'; + if (!eval { $conf->{$k} = check_value($config{$h}{$k}, $def); 1; }) { if ($def->{'required'}) { - warning("skipping host $h: invalid variable value '$k=$ovalue': $@"); + warning("skipping host $h: invalid variable value '$k=$config{$h}{$k}': $@"); delete $config{$h}; next HOST; } else { - warning("host $h: ignoring invalid variable value '$k=$ovalue': $@"); + warning("host $h: ignoring invalid variable value '$k=$config{$h}{$k}': $@"); $conf->{$k} = undef; } } From 18bd312216d991663f40ec721fe6fc7d3b6eb14e Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 16:24:41 -0400 Subject: [PATCH 26/32] Don't initialize `$config{$h}` entries to `undef` There's no need -- if the key doesn't exist the value returned is already `undef`. This prevents debug output from being littered with `undef` lines. --- ddclient.in | 1 - 1 file changed, 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index 9e23ebb..0ea9f08 100755 --- a/ddclient.in +++ b/ddclient.in @@ -2001,7 +2001,6 @@ sub init_config { next HOST; } else { warning("host $h: ignoring invalid variable value '$k=$config{$h}{$k}': $@"); - $conf->{$k} = undef; } } } From 564b315bfaf2c287d7c49fc139d09c74efc213a6 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 22:07:11 -0400 Subject: [PATCH 27/32] Convert command-line argument warnings into fatal errors --- ChangeLog.md | 3 +++ ddclient.in | 28 ++++++---------------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 9e8ed3a..9d5ba48 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -15,6 +15,9 @@ repository history](https://github.com/ddclient/ddclient/commits/master). * The default web service for `--webv4` and `--webv6` has changed from Google Domains (which has shut down) to ipify. [5b104ad1](https://github.com/ddclient/ddclient/commit/5b104ad116c023c3760129cab6e141f04f72b406) + * Invalid command-line options or values are now fatal errors (instead of + discarded with a warning). + [#TODO](https://github.com/ddclient/ddclient/pull/TODO) * All log messages are now written to STDERR, not a mix of STDOUT and STDERR. [#676](https://github.com/ddclient/ddclient/pull/676) * For `--protocol=freedns` and `--protocol=nfsn`, the core module diff --git a/ddclient.in b/ddclient.in index 0ea9f08..31a30da 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1953,19 +1953,12 @@ sub init_config { for my $k (keys %globals) { # TODO: This might grab an arbitrary protocol-specific variable, which could cause # surprising behavior. - my $def = $variables{'merged'}{$k}; - if (!$def) { - warning("ignoring unknown setting '$k=$globals{$k}'"); - delete($globals{$k}); - next; - } + my $def = $variables{'merged'}{$k} or fatal("unknown option '$k=$globals{$k}'"); # _read_config already checked any value from the config file, so the purpose of this check # is to validate command-line options which were merged into %globals above. # TODO: Move this check to where the command-line options are actually processed. - if (!eval { $globals{$k} = check_value($globals{$k}, $def); 1; }) { - warning("ignoring invalid variable value '$k=$globals{$k}': $@"); - delete($globals{$k}); - } + eval { $globals{$k} = check_value($globals{$k}, $def); 1; } + or fatal("invalid option value '$k=$globals{$k}': $@"); } ## now the host definitions... @@ -1978,9 +1971,7 @@ sub init_config { load_json_support($proto) if (grep($_ eq $proto, ("1984", "cloudflare", "digitalocean", "directnic", "gandi", "godaddy", "hetzner", "yandex", "nfsn", "njalla", "porkbun", "dnsexit2"))); if (!exists($protocols{$proto})) { - warning("skipping host: %s: unrecognized protocol '%s'", $h, $proto); - delete $config{$h}; - next; + fatal("host %s: unrecognized protocol: '%s'", $h, $proto); } my $svars = $protocols{$proto}{'variables'}; @@ -1994,15 +1985,8 @@ sub init_config { # check is to validate command-line options from --options which were merged into # $config{$h} above. # TODO: Move this check to where --options is actually processed. - if (!eval { $conf->{$k} = check_value($config{$h}{$k}, $def); 1; }) { - if ($def->{'required'}) { - warning("skipping host $h: invalid variable value '$k=$config{$h}{$k}': $@"); - delete $config{$h}; - next HOST; - } else { - warning("host $h: ignoring invalid variable value '$k=$config{$h}{$k}': $@"); - } - } + eval { $conf->{$k} = check_value($config{$h}{$k}, $def); 1; } + or fatal("host $h: invalid option value '$k=$config{$h}{$k}': $@"); } $config{$h} = $conf; } From 967bf2f6e83c1c429e8f1599ca32f02e78a4fb0f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 22:38:52 -0400 Subject: [PATCH 28/32] Error out if given an unknown per-host option --- ddclient.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ddclient.in b/ddclient.in index 31a30da..41a4114 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1976,11 +1976,8 @@ sub init_config { my $svars = $protocols{$proto}{'variables'}; my $conf = {'host' => $h, 'protocol' => $proto}; - - # TODO: This silently ignores unknown options passed via --options. - for my $k (keys %$svars) { - next if !defined($config{$h}{$k}); - my $def = $svars->{$k}; + for my $k (keys(%{$config{$h}})) { + my $def = $svars->{$k} or fatal("host $h: unknown option: $k"); # _read_config already checked any value from the config file, so the purpose of this # check is to validate command-line options from --options which were merged into # $config{$h} above. From 42f720df86f386c7b85bd886dc5ee7b4f5dff392 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 23:45:26 -0400 Subject: [PATCH 29/32] Move `--options` validation to where it is processed --- ddclient.in | 58 +++++++++++++++++------------------------------------ 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/ddclient.in b/ddclient.in index 41a4114..2e9348e 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1892,15 +1892,29 @@ sub init_config { delete $options{'host'}; } ## merge options into host definitions or globals - # TODO: Keys and values should be validated before mutating %config or %globals. if (@hosts) { for my $h (@hosts) { - $config{$h} = {%{$config{$h} // {}}, %options, 'host' => $h}; + $config{$h} //= {'host' => $h}; + my $proto = $options{'protocol'} // opt('protocol', $h); + my $protodef = $protocols{$proto} or fatal("host $h: invalid protocol: $proto"); + for my $var (keys(%options)) { + my $def = $protodef->{variables}{$var} + or fatal("host $h: unknown option '--options=$var=$options{$var}'"); + eval { $config{$h}{$var} = check_value($options{$var}, $def); 1; } + or fatal("host $h: invalid option value '--options=$var=$options{$var}': $@"); + } } $opt{'host'} = join(',', @hosts); } else { - # TODO: Why not merge the values into %opt? - %globals = (%globals, %options); + for my $var (keys(%options)) { + # TODO: This might grab an arbitrary protocol-specific variable definition, which + # could cause surprising behavior. + my $def = $variables{'merged'}{$var} + or fatal("unknown option '--options=$var=$options{$var}'"); + # TODO: Why not merge the values into %opt? + eval { $globals{$var} = check_value($options{$var}, $def); 1; } + or fatal("invalid option value '--options=$var=$options{$var}': $@"); + } } } @@ -1944,48 +1958,12 @@ sub init_config { # TODO: Why aren't the hosts specified by --host added to %config except when --options is also # given? - ## sanity check.. - # TODO: The code below doesn't look like a mere "sanity check", so I'm guessing the above - # comment is out of date. Figure out what this code is actually doing and refactor to improve - # the readability so that a comment isn't necessary. - ## make sure config entries have all defaults and they meet minimums - ## first the globals... - for my $k (keys %globals) { - # TODO: This might grab an arbitrary protocol-specific variable, which could cause - # surprising behavior. - my $def = $variables{'merged'}{$k} or fatal("unknown option '$k=$globals{$k}'"); - # _read_config already checked any value from the config file, so the purpose of this check - # is to validate command-line options which were merged into %globals above. - # TODO: Move this check to where the command-line options are actually processed. - eval { $globals{$k} = check_value($globals{$k}, $def); 1; } - or fatal("invalid option value '$k=$globals{$k}': $@"); - } - - ## now the host definitions... - HOST: for my $h (keys %config) { $config{$h}{use} = 'disabled' if opt('usev4', $h) ne 'disabled' || opt('usev6', $h) ne 'disabled'; my $proto = opt('protocol', $h); load_sha1_support($proto) if (grep($_ eq $proto, ("freedns", "nfsn"))); load_json_support($proto) if (grep($_ eq $proto, ("1984", "cloudflare", "digitalocean", "directnic", "gandi", "godaddy", "hetzner", "yandex", "nfsn", "njalla", "porkbun", "dnsexit2"))); - - if (!exists($protocols{$proto})) { - fatal("host %s: unrecognized protocol: '%s'", $h, $proto); - } - - my $svars = $protocols{$proto}{'variables'}; - my $conf = {'host' => $h, 'protocol' => $proto}; - for my $k (keys(%{$config{$h}})) { - my $def = $svars->{$k} or fatal("host $h: unknown option: $k"); - # _read_config already checked any value from the config file, so the purpose of this - # check is to validate command-line options from --options which were merged into - # $config{$h} above. - # TODO: Move this check to where --options is actually processed. - eval { $conf->{$k} = check_value($config{$h}{$k}, $def); 1; } - or fatal("host $h: invalid option value '$k=$config{$h}{$k}': $@"); - } - $config{$h} = $conf; } } From a06c5323946b088c7c5b6954adcfa00eb488ff33 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 2 Jul 2024 02:23:27 -0400 Subject: [PATCH 30/32] Call `load_sha1_support`, `load_json_support` once --- ddclient.in | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ddclient.in b/ddclient.in index 2e9348e..5cf0cf7 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1961,10 +1961,14 @@ sub init_config { for my $h (keys %config) { $config{$h}{use} = 'disabled' if opt('usev4', $h) ne 'disabled' || opt('usev6', $h) ne 'disabled'; - my $proto = opt('protocol', $h); - load_sha1_support($proto) if (grep($_ eq $proto, ("freedns", "nfsn"))); - load_json_support($proto) if (grep($_ eq $proto, ("1984", "cloudflare", "digitalocean", "directnic", "gandi", "godaddy", "hetzner", "yandex", "nfsn", "njalla", "porkbun", "dnsexit2"))); } + my @protos = map(opt('protocol', $_), keys(%config)); + my @needs_sha1 = grep({ my $p = $_; grep($_ eq $p, @protos); } qw(freedns nfsn)); + load_sha1_support(join(', ', @needs_sha1)) if @needs_sha1; + my @needs_json = grep({ my $p = $_; grep($_ eq $p, @protos); } + qw(1984 cloudflare digitalocean directnic dnsexit2 gandi godaddy hetzner + nfsn njalla porkbun yandex)); + load_json_support(join(', ', @needs_json)) if @needs_json; } sub usage { From 3be5b91601eb7dbc31c2ab6653c20038f7486a4e Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 1 Jul 2024 02:18:56 -0400 Subject: [PATCH 31/32] Improve documentation for `--host` and `--options` command-line args --- ddclient.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 5cf0cf7..64ed848 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1212,9 +1212,9 @@ my @opt = ( "", ["login", "=s", "--login= : log in to the dynamic DNS service as "], ["password", "=s", "--password= : log in to the dynamic DNS service with password "], - ["host", "=s", "--host= : update DNS information for "], + ["host", "=s", "--host=[,,...]\n : only update the given hosts. The hosts must already be defined in the config file (see '--file') unless '--options' is also specified"], "", - ["options", "=s", "--options==[,=,...]\n : optional per-service arguments (see below)"], + ["options", "=s", "--options==[,=,...]\n : override settings from the config file (see '--file') with the given values. Applies to all hosts"], "", ["ssl", "!", '--{no}ssl : use encryption (TLS) when the scheme (either "http://" or "https://") is missing from a URL'], ["ssl_ca_dir", "=s", "--ssl_ca_dir= : look in for certificates of trusted certificate authorities (default: auto-detect)"], From 442eac96c737e1b1058d2d86582650937d64aefa Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 18 Aug 2024 00:58:50 -0400 Subject: [PATCH 32/32] Update changelog for recent config fixes --- ChangeLog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index 9d5ba48..3d603c7 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -98,6 +98,8 @@ repository history](https://github.com/ddclient/ddclient/commits/master). ### Bug fixes + * Fixed numerous bugs in command-line option and configuration file + processing. [#TODO](https://github.com/ddclient/ddclient/pull/TODO) * `noip`: Fixed failure to honor IP discovery settings in some circumstances. [#591](https://github.com/ddclient/ddclient/pull/591) * Fixed `--usev6` with providers that have not yet been updated to use the new