diff --git a/Makefile.am b/Makefile.am index 738f301..54c1831 100644 --- a/Makefile.am +++ b/Makefile.am @@ -68,6 +68,7 @@ handwritten_tests = \ t/geturl_connectivity.pl \ t/geturl_response.pl \ t/group_hosts_by.pl \ + t/header_ok.pl \ t/interval_expired.pl \ t/is-and-extract-ipv4.pl \ t/is-and-extract-ipv6.pl \ diff --git a/ddclient.in b/ddclient.in index 9af3867..eaf40e8 100755 --- a/ddclient.in +++ b/ddclient.in @@ -3774,24 +3774,20 @@ sub nic_updateable { ###################################################################### sub header_ok { my ($host, $line) = @_; - my $ok = 0; - - if ($line =~ m%^s*HTTP/.*\s+(\d+)%i) { - my $result = $1; - - if ($result =~ m/^2\d\d$/) { - $ok = 1; - - } elsif ($result eq '401') { - failed("updating %s: authentication failed (%s)", $host, $line); - } elsif ($result eq '403') { - failed("updating %s: not authorized (%s)", $host, $line); - } - - } else { - failed("updating %s: unexpected line (%s)", $host, $line); + $line =~ s/\r?\n.*//s; + my ($code, $msg) = ($line =~ qr%^\s*HTTP/.*\s+(\d+)\s*(?:\s+([^\s].*))?$%i); + if (!defined($code)) { + failed('updating %s: unexpected HTTP response: %s', $host, $line); + return 0; + } elsif ($code !~ qr/^2\d\d$/) { + my %msgs = ( + '401' => 'authentication failed', + '403' => 'not authorized', + ); + failed('updating %s: %s %s', $host, $code, $msg // $msgs{$code} // ''); + return 0; } - return $ok; + return 1; } ###################################################################### diff --git a/t/header_ok.pl b/t/header_ok.pl new file mode 100644 index 0000000..197cc8d --- /dev/null +++ b/t/header_ok.pl @@ -0,0 +1,74 @@ +use Test::More; +SKIP: { eval { require Test::Warnings; } or skip($@, 1); } +eval { require 'ddclient'; } or BAIL_OUT($@); +my $have_mock = eval { require Test::MockModule; }; + +my $failmsg; +my $module; +if ($have_mock) { + $module = Test::MockModule->new('ddclient'); + # Note: 'mock' is used instead of 'redefine' because 'redefine' is not available in the versions + # of Test::MockModule distributed with old Debian and Ubuntu releases. + $module->mock('failed', sub { $failmsg //= ''; $failmsg .= sprintf(shift, @_) . "\n"; }); +} + +my @test_cases = ( + { + desc => 'malformed not OK', + input => 'malformed', + want => 0, + wantmsg => qr/unexpected/, + }, + { + desc => 'HTTP/1.1 200 OK', + input => 'HTTP/1.1 200 OK', + want => 1, + }, + { + desc => 'HTTP/2 200 OK', + input => 'HTTP/2 200 OK', + want => 1, + }, + { + desc => 'HTTP/3 200 OK', + input => 'HTTP/3 200 OK', + want => 1, + }, + { + desc => '401 not OK, fallback message', + input => 'HTTP/1.1 401 ', + want => 0, + wantmsg => qr/authentication failed/, + }, + { + desc => '403 not OK, fallback message', + input => 'HTTP/1.1 403 ', + want => 0, + wantmsg => qr/not authorized/, + }, + { + desc => 'other 4xx not OK', + input => 'HTTP/1.1 456 bad', + want => 0, + wantmsg => qr/bad/, + }, + { + desc => 'only first line is logged on error', + input => "HTTP/1.1 404 not found\n\nbody", + want => 0, + wantmsg => qr/(?!body)/, + }, +); + +for my $tc (@test_cases) { + subtest $tc->{desc} => sub { + $failmsg = ''; + is(ddclient::header_ok('host', $tc->{input}), $tc->{want}, 'return value matches'); + SKIP: { + skip('Test::MockModule not available') if !$have_mock; + like($failmsg, $tc->{wantmsg} // qr/^$/, 'fail message matches'); + } + }; +} + +done_testing();