From: Eric Wong <e@80x24.org>
Date: Thu, 2 Nov 2023 21:35:50 +0000 (+0000)
Subject: ds: don't try ->close after ->accept_SSL failure
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e85d3280129ce60a9587895a44df765a39447634;p=thirdparty%2Fpublic-inbox.git

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 33f800877..da26efc43 100644
--- 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 cf3c95c96..a11a0dd9b 100644
--- 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();