Merge pull request #686 from rhansen/header_ok

Minor improvements to `header_ok` utility function

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 \

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;
 }
 
 ######################################################################

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();