ds: don't try ->close after ->accept_SSL failure

Eric Wong <e@80x24.org> wrote:
> --- a/lib/PublicInbox/DS.pm
> +++ b/lib/PublicInbox/DS.pm
> @@ -341,8 +341,8 @@ sub greet {
>  	my $ev = EPOLLIN;
>  	my $wbuf;
>  	if ($sock->can('accept_SSL') && !$sock->accept_SSL) {
> -		return CORE::close($sock) if $! != EAGAIN;
> -		$ev = PublicInbox::TLS::epollbit() or return CORE::close($sock);
> +		return $sock->close if $! != EAGAIN;
> +		$ev = PublicInbox::TLS::epollbit() or return $sock->close;
>  		$wbuf = [ \&accept_tls_step, $self->can('do_greet')];
>  	}
>  	new($self, $sock, $ev | EPOLLONESHOT);

Noticed this on deploy:

-----8<-----
Subject: [PATCH] ds: don't try ->close after ->accept_SSL failure

->accept_SSL failures leaves the socket ref as a GLOB (not
IO::Handle) and unable to respond to the ->close method.
Calling close in any form isn't actually necessary at all,
so just let refcounting destroy the socket.

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 33f80087792f0769de43e6ce13c5f05f3617d44b..da26efc4368309a34278cb596320ca29249b4a1b 100644 (file)
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -341,8 +341,7 @@ sub greet {
        my $ev = EPOLLIN;
        my $wbuf;
        if ($sock->can('accept_SSL') && !$sock->accept_SSL) {
-               return $sock->close if $! != EAGAIN;
-               $ev = PublicInbox::TLS::epollbit() or return $sock->close;
+               return if $! != EAGAIN || !($ev = PublicInbox::TLS::epollbit());
                $wbuf = [ \&accept_tls_step, $self->can('do_greet')];
        }
        new($self, $sock, $ev | EPOLLONESHOT);

diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index cf3c95c96127aafc37c7048eb444d339fb0a3e5d..a11a0dd9b8abecf5934d011bd918c81d258a1bcb 100644 (file)
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -185,6 +185,10 @@ for my $args (
                is($x, undef, 'no BSD accept filter for plain NNTP');
        };
 
+       my $s = tcp_connect($nntps);
+       syswrite($s, '->accept_SSL_ will fail on this!');
+       ok(!sysread($s, my $rbuf, 128), 'EOF or ECONNRESET on ->accept_SSL fail');
+
        $c = undef;
        $td->kill;
        $td->join;
@@ -195,6 +199,7 @@ for my $args (
                <$fh>;
        };
        unlike($eout, qr/wide/i, 'no Wide character warnings');
+       unlike($eout, qr/^E:/, 'no other errors');
 }
 done_testing();