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 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 350BF1F51B for ; Tue, 17 Oct 2023 23:38:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1697585897; bh=j/ETVaCQOSamntRxipQNsu6/u1hkCQaMPeJjWO0g4kI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Ssp9rk33q1aGA9h0rTTp5esOMKp+po1QUPrXA7aHHAoj3ghRKglDBkvugKWNZ9Coo UOUw0tGD9RkJk6YcPmoz83K1tI5UFbzcsBET6QZ20HOHec0ghU9MKDzTyvOjWkGueT 85o80xw9ZbQVja5YPg9TEBVNaAcwAHH6oYysz4dU= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 07/30] use read_all in more places to improve safety Date: Tue, 17 Oct 2023 23:37:52 +0000 Message-ID: <20231017233815.1637932-8-e@80x24.org> In-Reply-To: <20231017233815.1637932-1-e@80x24.org> References: <20231017233815.1637932-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: `readline' ops may not detect errors on partial reads. This saves us some code to reduce cognitive overhead for readers. We'll also support reusing a destination buffers so it can work more nicely with existing code. --- lib/PublicInbox/Gcf2.pm | 7 +++---- lib/PublicInbox/Git.pm | 9 +++++---- lib/PublicInbox/InboxWritable.pm | 6 +++--- lib/PublicInbox/LeiALE.pm | 11 +++-------- lib/PublicInbox/LeiBlob.pm | 6 +++--- lib/PublicInbox/LeiConfig.pm | 2 +- lib/PublicInbox/LeiMailSync.pm | 5 ++--- lib/PublicInbox/LeiSucks.pm | 5 +++-- lib/PublicInbox/MultiGit.pm | 3 ++- lib/PublicInbox/ViewVCS.pm | 12 +++++------- lib/PublicInbox/WWW.pm | 4 ++-- lib/PublicInbox/XapHelper.pm | 3 ++- lib/PublicInbox/XapHelperCxx.pm | 6 +++--- script/public-inbox-edit | 4 ++-- script/public-inbox-init | 3 ++- 15 files changed, 41 insertions(+), 45 deletions(-) diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm index 37262e28..4f163cde 100644 --- a/lib/PublicInbox/Gcf2.pm +++ b/lib/PublicInbox/Gcf2.pm @@ -9,7 +9,7 @@ use PublicInbox::Spawn qw(which popen_rd); # may set PERL_INLINE_DIRECTORY use Fcntl qw(SEEK_SET); use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC); use IO::Handle; # autoflush -use PublicInbox::Git; +use PublicInbox::Git qw(read_all); use PublicInbox::Lock; BEGIN { @@ -43,12 +43,11 @@ BEGIN { # build them. my $f = "$dir/gcf2_libgit2.h"; open my $src, '<', $f; - local $/; - $c_src = <$src> // die "read $f: $!"; + $c_src = read_all($src); } unless ($c_src) { seek($err, 0, SEEK_SET); - $err = do { local $/; <$err> }; + $err = read_all($err); die "E: libgit2 not installed: $err\n"; } # append pkg-config results to the source to ensure Inline::C diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index 35bd10ef..a460d155 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -550,11 +550,12 @@ sub modified ($;$) { # read_all/try_cat can probably be moved somewhere else... -sub read_all ($;$) { - my ($fh, $len) = @_; - my $r = read($fh, my $buf, $len //= -s $fh); +sub read_all ($;$$) { + my ($fh, $len, $bref) = @_; + $bref //= \(my $buf); + my $r = read($fh, $$bref, $len //= -s $fh); croak("$fh read ($r != $len)") if $len != $r; - $buf; + $$bref; } sub try_cat { diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm index 65952aa2..6af72e71 100644 --- a/lib/PublicInbox/InboxWritable.pm +++ b/lib/PublicInbox/InboxWritable.pm @@ -7,6 +7,7 @@ use strict; use v5.10.1; use parent qw(PublicInbox::Inbox PublicInbox::Umask Exporter); use PublicInbox::Import; +use PublicInbox::Git qw(read_all); use PublicInbox::Filter::Base qw(REJECT); use Errno qw(ENOENT); our @EXPORT_OK = qw(eml_from_path); @@ -114,9 +115,8 @@ sub filter { sub eml_from_path ($) { my ($path) = @_; if (sysopen(my $fh, $path, O_RDONLY|O_NONBLOCK)) { - return unless -f $fh; # no FIFOs or directories - my $str = do { local $/; <$fh> } or return; - PublicInbox::Eml->new(\$str); + return unless -f $fh && -s _; # no FIFOs or directories + PublicInbox::Eml->new(\(my $str = read_all($fh, -s _))); } else { # ENOENT is common with Maildir warn "failed to open $path: $!\n" if $! != ENOENT; undef; diff --git a/lib/PublicInbox/LeiALE.pm b/lib/PublicInbox/LeiALE.pm index cc9a2095..b198af1c 100644 --- a/lib/PublicInbox/LeiALE.pm +++ b/lib/PublicInbox/LeiALE.pm @@ -9,7 +9,7 @@ package PublicInbox::LeiALE; use strict; use v5.10.1; use parent qw(PublicInbox::LeiSearch PublicInbox::Lock); -use PublicInbox::Git; +use PublicInbox::Git qw(read_all); use PublicInbox::Import; use PublicInbox::LeiXSearch; use Fcntl qw(SEEK_SET); @@ -54,11 +54,7 @@ sub refresh_externals { $self->git->cleanup; my $lk = $self->lock_for_scope; my $cur_lxs = ref($lxs)->new; - my $orig = do { - local $/; - readline($self->{lockfh}) // - die "readline($self->{lock_path}): $!"; - }; + my $orig = read_all($self->{lockfh}); my $new = ''; my $old = ''; my $gone = 0; @@ -91,8 +87,7 @@ sub refresh_externals { $new = $old = ''; my $f = $self->git->{git_dir}.'/objects/info/alternates'; if (open my $fh, '<', $f) { - local $/; - $old = <$fh> // die "readline($f): $!"; + read_all($fh, -s $fh, \$old); } for my $x (@ibxish) { $new .= $lei->canonpath_harder($x->git->{git_dir})."/objects\n"; diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm index d069d4a8..40f64bd9 100644 --- a/lib/PublicInbox/LeiBlob.pm +++ b/lib/PublicInbox/LeiBlob.pm @@ -10,6 +10,7 @@ use parent qw(PublicInbox::IPC); use PublicInbox::Spawn qw(run_wait popen_rd which); use PublicInbox::DS; use PublicInbox::Eml; +use PublicInbox::Git qw(read_all); sub get_git_dir ($$) { my ($lei, $d) = @_; @@ -137,9 +138,8 @@ sub lei_blob { extract_attach($lei, $blob, $bref) : $lei->out($$bref); if ($opt->{mail}) { - my $eh = $rdr->{2}; - seek($eh, 0, 0); - return $lei->child_error($cerr, do { local $/; <$eh> }); + seek($rdr->{2}, 0, 0); + return $lei->child_error($cerr, read_all($rdr->{2})); } # else: fall through to solver below } diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm index b3495487..c47708d8 100644 --- a/lib/PublicInbox/LeiConfig.pm +++ b/lib/PublicInbox/LeiConfig.pm @@ -28,7 +28,7 @@ sub cfg_edit_done { # PktOp lei->do_env cb $lei->cfg_dump($self->{-f}); } or do { seek($fh, 0, SEEK_SET); - return cfg_do_edit($self, do { local $/; <$fh> }); + return cfg_do_edit($self, read_all($fh)); }; $self->cfg_verify($cfg) if $self->can('cfg_verify'); } diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm index 415459d5..74ef1362 100644 --- a/lib/PublicInbox/LeiMailSync.pm +++ b/lib/PublicInbox/LeiMailSync.pm @@ -10,7 +10,7 @@ use PublicInbox::Compat qw(uniqstr); use DBI qw(:sql_types); # SQL_BLOB use PublicInbox::ContentHash qw(git_sha); use Carp (); -use PublicInbox::Git qw(%HEXLEN2SHA); +use PublicInbox::Git qw(%HEXLEN2SHA read_all); sub dbh_new { my ($self) = @_; @@ -456,8 +456,7 @@ WHERE b.oidbin = ? open my $fh, '<', $f or next; # some (buggy) Maildir writers are non-atomic: next unless -s $fh; - local $/; - my $raw = <$fh>; + my $raw = read_all($fh, -s _); if ($vrfy) { my $sha = $HEXLEN2SHA{length($oidhex)}; my $got = git_sha($sha, \$raw)->hexdigest; diff --git a/lib/PublicInbox/LeiSucks.pm b/lib/PublicInbox/LeiSucks.pm index 35d0a8de..82aea8d4 100644 --- a/lib/PublicInbox/LeiSucks.pm +++ b/lib/PublicInbox/LeiSucks.pm @@ -12,6 +12,7 @@ use Config; use POSIX (); use PublicInbox::Config; use PublicInbox::IPC; +use PublicInbox::Git qw(read_all); sub lei_sucks { my ($lei, @argv) = @_; @@ -58,8 +59,8 @@ sub lei_sucks { for my $m (grep(m{^PublicInbox/}, sort keys %INC)) { my $f = $INC{$m} // next; # lazy require failed (missing dep) open my $fh, '<', $f or do { warn "open($f): $!"; next }; - my $hex = sha1_hex('blob '.(-s $fh)."\0". - (do { local $/; <$fh> } // die("read: $!"))); + my $size = -s $fh; + my $hex = sha1_hex("blob $size\0".read_all($fh, $size)); push @out, ' '.$hex.' '.$m."\n"; } push @out, <<'EOM'; diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm index 74a9e1df..35bd0251 100644 --- a/lib/PublicInbox/MultiGit.pm +++ b/lib/PublicInbox/MultiGit.pm @@ -9,6 +9,7 @@ use PublicInbox::Spawn qw(run_die popen_rd); use PublicInbox::Import; use File::Temp 0.19; use List::Util qw(max); +use PublicInbox::Git qw(read_all); sub new { my ($cls, $topdir, $all, $epfx) = @_; @@ -31,7 +32,7 @@ sub read_alternates { qr!\A\Q../../$self->{epfx}\E/([0-9]+)\.git/objects\z! : undef; $$moderef = (stat($fh))[2] & 07777; - for my $rel (split(/^/m, do { local $/; <$fh> })) { + for my $rel (split(/^/m, read_all($fh, -s _))) { chomp(my $dir = $rel); my $score; if (defined($is_edir) && $dir =~ $is_edir) { diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm index 8a90c2d6..86c46e69 100644 --- a/lib/PublicInbox/ViewVCS.pm +++ b/lib/PublicInbox/ViewVCS.pm @@ -17,6 +17,7 @@ use strict; use v5.10.1; use File::Temp 0.19 (); # newdir use PublicInbox::SolverGit; +use PublicInbox::Git qw(read_all); use PublicInbox::GitAsyncCat; use PublicInbox::WwwStream qw(html_oneshot); use PublicInbox::Linkify; @@ -61,12 +62,9 @@ sub dbg_log ($) { warn "seek(log): $!"; return '
debug log seek error
'; } - $log = do { local $/; <$log> } // do { - if (!eof($log)) { - warn "readline(log): $!"; - return '
debug log read error
'; - } - ''; + $log = eval { read_all($log) } // do { + warn "read(log): $@"; + return '
debug log read error
'; }; return '' if $log eq ''; $ctx->{-linkify} //= PublicInbox::Linkify->new; @@ -251,7 +249,7 @@ EOM if (-s $fh > $MAX_SIZE) { print $zfh "---\n patch is too large to show\n"; } else { # prepare flush_diff: - read($fh, $x, -s _); + read_all($fh, -s _, \$x); utf8_maybe($x); $ctx->{-apfx} = $ctx->{-spfx} = $upfx; $x =~ s/\r?\n/\n/gs; diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm index 9919f975..86fd7e22 100644 --- a/lib/PublicInbox/WWW.pm +++ b/lib/PublicInbox/WWW.pm @@ -587,9 +587,9 @@ sub stylesheets_prepare ($$) { next; }; my $ctime = 0; - my $local = do { local $/; <$fh> }; + my $local = read_all($fh, -s $fh); if ($local =~ /\S/) { - $ctime = sprintf('%x',(stat($fh))[10]); + $ctime = sprintf('%x',(stat(_))[10]); $local = $mini->($local); } diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm index ae907766..ca993ca8 100644 --- a/lib/PublicInbox/XapHelper.pm +++ b/lib/PublicInbox/XapHelper.pm @@ -10,6 +10,7 @@ $GLP->configure(qw(require_order bundling no_ignore_case no_auto_abbrev)); use PublicInbox::Search qw(xap_terms); use PublicInbox::CodeSearch; use PublicInbox::IPC; +use PublicInbox::Git qw(read_all); use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX); use PublicInbox::DS qw(awaitpid); use POSIX qw(:signal_h); @@ -123,7 +124,7 @@ sub cmd_dump_roots { $req->{A} or return warn('dump_roots requires -A PREFIX'); open my $fh, '<', $root2id_file or die "open($root2id_file): $!"; my $root2id; # record format: $OIDHEX "\0" uint32_t - my @x = split(/\0/, do { local $/; <$fh> } // die "readline: $!"); + my @x = split(/\0/, read_all($fh)); while (@x) { my $oidhex = shift @x; $root2id->{$oidhex} = shift @x; diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm index dbb0a915..83015379 100644 --- a/lib/PublicInbox/XapHelperCxx.pm +++ b/lib/PublicInbox/XapHelperCxx.pm @@ -8,6 +8,7 @@ package PublicInbox::XapHelperCxx; use v5.12; use PublicInbox::Spawn qw(popen_rd which); +use PublicInbox::Git qw(read_all); use PublicInbox::Search; use Fcntl qw(SEEK_SET); use Config; @@ -34,7 +35,7 @@ sub xap_cfg (@) { chomp(my $ret = do { local $/; <$rd> }); return $ret if close($rd); seek($err, 0, SEEK_SET) or die "seek: $!"; - $err = do { local $/; <$err> }; + $err = read_all($err); die < }) or die - "read $edit_fn: $!\n"; + my $new_raw = read_all($new_fh); if (!$opt->{raw}) { PublicInbox::Eml::strip_from($new_raw); diff --git a/script/public-inbox-init b/script/public-inbox-init index b3a16cfb..33bee310 100755 --- a/script/public-inbox-init +++ b/script/public-inbox-init @@ -125,13 +125,14 @@ my $auto_unlink = PublicInbox::OnDestroy->new($$, sub { unlink $lockfile }); my $perm = 0644 & ~umask; my %seen; if (-e $pi_config) { + require PublicInbox::Git; open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n"; my @st = stat($oh); $perm = $st[2]; defined $perm or die "(f)stat failed on $pi_config: $!\n"; chmod($perm & 07777, $fh) or die "(f)chmod failed on future $pi_config: $!\n"; - defined(my $old = do { local $/; <$oh> }) or die "read $pi_config: $!\n"; + my $old = PublicInbox::Git::read_all($oh); print $fh $old or die "failed to write: $!\n"; close $oh or die "failed to close $pi_config: $!\n";