From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 6F58A1F461 for ; Sun, 7 Jul 2019 07:08:31 +0000 (UTC) Date: Sun, 7 Jul 2019 07:08:31 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 5/4] nntp: improve error reporting for COMPRESS Message-ID: <20190707070831.fafviqilsywuc5bw@dcvr> References: <20190705225339.5698-1-e@80x24.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20190705225339.5698-1-e@80x24.org> List-Id: Add some checks for errors at initialization, though there's not much that can be done with ENOMEM-type errors aside from dropping clients. We can also get rid of the scary FIXME for MemLevel=8. It was a stupid error on my part in the original per-client deflate stream implementation calling C::R::Z::{Inflate,Deflate} in array context and getting the extra dualvar error code as a string result, causing the {zin}/{zout} array refs to have extra array elements. --- lib/PublicInbox/NNTP.pm | 3 +-- lib/PublicInbox/NNTPdeflate.pm | 33 +++++++++++++++++++++------------ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 895858b7..6fee29f4 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -922,9 +922,8 @@ sub cmd_starttls ($) { # RFC 8054 sub cmd_compress ($$) { my ($self, $alg) = @_; - return '503 Only the DEFLATE is supported' if uc($alg) ne 'DEFLATE'; + return '503 Only DEFLATE is supported' if uc($alg) ne 'DEFLATE'; return r502 if $self->compressed || !$have_deflate; - res($self, '206 Compression active'); PublicInbox::NNTPdeflate->enable($self); $self->requeue; undef diff --git a/lib/PublicInbox/NNTPdeflate.pm b/lib/PublicInbox/NNTPdeflate.pm index 78da2a58..10e2337c 100644 --- a/lib/PublicInbox/NNTPdeflate.pm +++ b/lib/PublicInbox/NNTPdeflate.pm @@ -30,24 +30,33 @@ my %IN_OPT = ( # global deflate context and buffer my $zbuf = \(my $buf = ''); -my $zout = Compress::Raw::Zlib::Deflate->new( - # nnrpd (INN) and Compress::Raw::Zlib favor MemLevel=9, - # but the zlib C library and git use MemLevel=8 as the default. - # FIXME: sometimes clients fail with 8, so we use 9 - # -MemLevel => 9, - - # needs more testing, nothing obviously different in terms of memory - -Bufsize => 65536, +my $zout; +{ + my $err; + ($zout, $err) = Compress::Raw::Zlib::Deflate->new( + # nnrpd (INN) and Compress::Raw::Zlib favor MemLevel=9, + # the zlib C library and git use MemLevel=8 as the default + # -MemLevel => 9, + -Bufsize => 65536, # same as nnrpd + -WindowBits => -15, # RFC 1951 + -AppendOutput => 1, + ); + $err == Z_OK or die "Failed to initialize zlib deflate stream: $err"; +} - -WindowBits => -15, # RFC 1951 - -AppendOutput => 1, -); sub enable { my ($class, $self) = @_; + my ($in, $err) = Compress::Raw::Zlib::Inflate->new(%IN_OPT); + if ($err != Z_OK) { + $self->err("Inflate->new failed: $err"); + $self->res('403 Unable to activate compression'); + return; + } unlock_hash(%$self); + $self->res('206 Compression active'); bless $self, $class; - $self->{zin} = [ Compress::Raw::Zlib::Inflate->new(%IN_OPT), '' ]; + $self->{zin} = [ $in, '' ]; } # overrides PublicInbox::NNTP::compressed -- EW