]> git.ipfire.org Git - thirdparty/public-inbox.git/commitdiff
use read_all in more places to improve safety
authorEric Wong <e@80x24.org>
Tue, 17 Oct 2023 23:37:52 +0000 (23:37 +0000)
committerEric Wong <e@80x24.org>
Wed, 18 Oct 2023 20:50:24 +0000 (20:50 +0000)
`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.

15 files changed:
lib/PublicInbox/Gcf2.pm
lib/PublicInbox/Git.pm
lib/PublicInbox/InboxWritable.pm
lib/PublicInbox/LeiALE.pm
lib/PublicInbox/LeiBlob.pm
lib/PublicInbox/LeiConfig.pm
lib/PublicInbox/LeiMailSync.pm
lib/PublicInbox/LeiSucks.pm
lib/PublicInbox/MultiGit.pm
lib/PublicInbox/ViewVCS.pm
lib/PublicInbox/WWW.pm
lib/PublicInbox/XapHelper.pm
lib/PublicInbox/XapHelperCxx.pm
script/public-inbox-edit
script/public-inbox-init

index 37262e28fdd6ce5dfd5d047581e119ff249b1b10..4f163cdea615e533f0405785793756d50d142b11 100644 (file)
@@ -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
index 35bd10efaa7cdf6a68ed2833cda56e9be1558c3d..a460d15551fc31cdee2a43e8e12a41f233cc9f3c 100644 (file)
@@ -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 {
index 65952aa2dcb0f2a844b51283cdede55d1326ae49..6af72e71a3d70b0dab3f190cdd1e10ecdc7b732b 100644 (file)
@@ -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;
index cc9a2095af08e6b8f55145735ceb1e65c00cc44d..b198af1c3389902f0890ba7ab37a8063a08ce374 100644 (file)
@@ -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";
index d069d4a837e183ce17a689417715604339d1fcc9..40f64bd97113f1884f05ad8984190e15c336c6a1 100644 (file)
@@ -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
        }
 
index b34954879ccdd7b7a9b2e58c0c81802f0666f259..c47708d82ab4fba0b2c1b6e6508e464643b31533 100644 (file)
@@ -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');
 }
index 415459d5c127f3beb2b83175ba207ce27b20da59..74ef136261078970d0480473e3d0d26819a059a8 100644 (file)
@@ -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;
index 35d0a8de5dc51c0710d67ddc8ca5613745140756..82aea8d4c4037c3f2ab90e008e5c90299706f7b2 100644 (file)
@@ -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';
index 74a9e1df1d410f5de4587e59eaa3a46a641afc8e..35bd0251ee1cb2eacec7c076f4ea2c35d2499863 100644 (file)
@@ -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) {
index 8a90c2d6a2560d6eac2266796532fa59f8432f1c..86c46e69e2ed305ebdd80e6ae717a7d5d2564010 100644 (file)
@@ -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 '<pre>debug log seek error</pre>';
        }
-       $log = do { local $/; <$log> } // do {
-               if (!eof($log)) {
-                       warn "readline(log): $!";
-                       return '<pre>debug log read error</pre>';
-               }
-               '';
+       $log = eval { read_all($log) } // do {
+               warn "read(log): $@";
+               return '<pre>debug log read error</pre>';
        };
        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;
index 9919f975c7bb02d4a9178d85f59453b9858f32eb..86fd7e227ea087e91fc1646ced19bd687c621de7 100644 (file)
@@ -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);
                        }
 
index ae907766c31b4dd215ce9a4315f661b4d96eb0ad..ca993ca86756f8b6d38b51a3c26a0dc7b8ae5ae2 100644 (file)
@@ -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;
index dbb0a9154a36a6f58ac4db788eb47df5bac444d9..83015379daa0aef947b43b2713d8e6dba77697a4 100644 (file)
@@ -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 <<EOM;
 @$cmd failed: Xapian development files missing? (\$?=$?)
 $err
@@ -70,8 +71,7 @@ sub build () {
        for (@srcs) {
                say $fh qq(# line 1 "$_");
                open my $rfh, '<', $_;
-               local $/;
-               print $fh readline($rfh);
+               print $fh read_all($rfh);
        }
        print $fh PublicInbox::Search::generate_cxx();
        print $fh PublicInbox::CodeSearch::generate_cxx();
index 1fb6f32b45eec240b81fa2463fc9eea939974ac9..7702881769d572cfa133ea671b6905711b801022 100755 (executable)
@@ -15,6 +15,7 @@ PublicInbox::Admin::check_require('-index');
 use PublicInbox::Eml;
 use PublicInbox::InboxWritable qw(eml_from_path);
 use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
 
 my $help = <<'EOF';
 usage: public-inbox-edit -m MESSAGE-ID [--all] [INBOX_DIRS]
@@ -184,8 +185,7 @@ retry_edit:
        # rename/relink $edit_fn
        open my $new_fh, '<', $edit_fn or
                die "can't read edited file ($edit_fn): $!\n";
-       defined(my $new_raw = do { local $/; <$new_fh> }) or die
-               "read $edit_fn: $!\n";
+       my $new_raw = read_all($new_fh);
 
        if (!$opt->{raw}) {
                PublicInbox::Eml::strip_from($new_raw);
index b3a16cfbf69d6f6bc21d64597dcef7a552e546cb..33bee31018cbe2697b8435ab8a6bd0665f2eccdc 100755 (executable)
@@ -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";