From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id AFA001FA3D for ; Thu, 9 Nov 2023 10:09:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1699524588; bh=cSVeGac/hWrnITCThumNqEZ6Fq7gTr64Y9hawg7r6lc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=1cnrKboC9aaa/5/W50ytzarl0DlVQAB173XROdudwCT8QKyvyf8TG4xuijp3OFATN O3B+lq67XPpH1XUSDbXoKXcFsJNOrV8YNTcMR+abAVc62WKxj2IQ8TUde5XahqD7Uc phwf/cNGOY1mzWvqSFli5hMZvlDVbwdiLcZhlh3o= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 10/13] lei_input: always close single `eml' inputs Date: Thu, 9 Nov 2023 10:09:43 +0000 Message-ID: <20231109100946.1440611-11-e@80x24.org> In-Reply-To: <20231109100946.1440611-1-e@80x24.org> References: <20231109100946.1440611-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: This matches the behavior we have for multi-message mbox files since we rely on ->close to detect errors on bad mboxes. This ensures we'll notice errors reading single messages from stdin. We'll also start relying more on strace error injection to test error handling. --- lib/PublicInbox/LeiInput.pm | 13 +++++++------ lib/PublicInbox/TestCommon.pm | 23 ++++++++++++++++++++++- t/io.t | 8 +------- t/lei-import.t | 27 +++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 14 deletions(-) diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm index 4cd18c09..adb356c9 100644 --- a/lib/PublicInbox/LeiInput.pm +++ b/lib/PublicInbox/LeiInput.pm @@ -81,9 +81,11 @@ sub input_fh { my ($self, $ifmt, $fh, $name, @args) = @_; if ($ifmt eq 'eml') { my $buf = do { local $/; <$fh> }; - (defined($buf) && eof($fh) && close($fh)) or - return $self->{lei}->child_error(0, <<""); -error reading $name: $! + my $ok = defined($buf) ? 1 : 0; + ++$ok if eof($fh); + ++$ok if $fh->close; + $ok == 3 or return $self->{lei}->child_error($?, <<""); +error reading $name: $! (\$?=$?) PublicInbox::Eml::strip_from($buf); @@ -246,9 +248,8 @@ sub input_path_url { my $rdr = { 2 => $lei->{2} }; my $fh = popen_rd($fp, undef, $rdr); eval { $self->input_fh('eml', $fh, $input, @args) }; - my @err = ($@ ? $@ : ()); - $fh->close or push @err, "\$?=$?"; - $lei->child_error($?, "@$fp failed: @err") if @err; + my $err = $@ ? ": $@" : ''; + $lei->child_error($?, "@$fp failed$err") if $err || $?; } else { $self->folder_missing("$ifmt:$input"); } diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index 83e99b42..46e6a538 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -28,7 +28,8 @@ BEGIN { quit_waiter_pipe wait_for_eof tcp_host_port test_lei lei lei_ok $lei_out $lei_err $lei_opt test_httpd xbail require_cmd is_xdeeply tail_f - ignore_inline_c_missing no_pollerfd no_coredump cfg_new); + ignore_inline_c_missing no_pollerfd no_coredump cfg_new + strace strace_inject); require Test::More; my @methods = grep(!/\W/, @Test::More::EXPORT); eval(join('', map { "*$_=\\&Test::More::$_;" } @methods)); @@ -933,6 +934,26 @@ sub cfg_new ($;@) { PublicInbox::Config->new($f); } +our $strace_cmd; +sub strace () { + skip 'linux only test' if $^O ne 'linux'; + require_cmd('strace', 1); +} + +sub strace_inject () { + my $cmd = strace; + state $ver = do { + require PublicInbox::Spawn; + my $v = PublicInbox::Spawn::run_qx([$cmd, '--version']); + $v =~ m!version\s+([1-9]+\.[0-9]+)! or + xbail "no strace --version: $v"; + eval("v$1"); + }; + $ver ge v4.16 or skip "$cmd too old for syscall injection (". + sprintf('v%vd', $ver). ' < v4.16)'; + $cmd +} + package PublicInbox::TestCommon::InboxWakeup; use strict; sub on_inbox_unlock { ${$_[0]}->($_[1]) } diff --git a/t/io.t b/t/io.t index 4c7a97a3..3ea00ed8 100644 --- a/t/io.t +++ b/t/io.t @@ -9,13 +9,7 @@ use PublicInbox::Spawn qw(which run_qx); # only test failures SKIP: { -skip 'linux only test' if $^O ne 'linux'; -my $strace = which('strace') or skip 'strace missing for test'; -my $v = run_qx([$strace, '--version']); -$v =~ m!version\s+([1-9]+\.[0-9]+)! or xbail "no strace --version: $v"; -$v = eval("v$1"); -$v ge v4.16 or skip "$strace too old for syscall injection (". - sprintf('v%vd', $v). ' < v4.16)'; +my $strace = strace_inject; my $env = { PERL5LIB => join(':', @INC) }; my $opt = { 1 => \my $out, 2 => \my $err }; my $dst = "$tmpdir/dst"; diff --git a/t/lei-import.t b/t/lei-import.t index b2c1de9b..6ad4c97b 100644 --- a/t/lei-import.t +++ b/t/lei-import.t @@ -154,6 +154,33 @@ do { } until (!lei('ls-label') || $lei_out =~ /\bbin\b/ || now > $end); like($lei_out, qr/\bbin\b/, 'commit-delay eventually commits'); +SKIP: { + my $strace = strace_inject; # skips if strace is old or non-Linux + my $tmpdir = tmpdir; + my $tr = "$tmpdir/tr"; + my $cmd = [ $strace, "-o$tr", '-f', + "-P", File::Spec->rel2abs('t/plack-qp.eml'), + '-e', 'inject=readv,read:error=EIO']; + lei_ok qw(daemon-pid); + chomp(my $daemon_pid = $lei_out); + push @$cmd, '-p', $daemon_pid; + my $strace_opt = { 1 => \my $out, 2 => \my $err }; + require PublicInbox::Spawn; + require PublicInbox::AutoReap; + my $pid = PublicInbox::Spawn::spawn($cmd, \%ENV, $strace_opt); + my $ar = PublicInbox::AutoReap->new($pid); + tick; # wait for strace to attach + ok(!lei(qw(import -F eml t/plack-qp.eml)), + '-F eml import fails on pathname error injection'); + like($lei_err, qr!error reading t/plack-qp\.eml: Input/output error!, + 'EIO noted in stderr'); + open $fh, '<', 't/plack-qp.eml'; + ok(!lei(qw(import -F eml -), undef, { %$lei_opt, 0 => $fh }), + '-F eml import fails on stdin error injection'); + like($lei_err, qr!error reading .*?: Input/output error!, + 'EIO noted in stderr'); +} + # see t/lei_to_mail.t for "import -F mbox*" }); done_testing;