* [PATCH 1/3] treewide: use *nix-specific dirname regexps
2021-08-11 11:26 [PATCH 0/3] lei pathname canonicalization fixes Eric Wong
@ 2021-08-11 11:26 ` Eric Wong
2021-08-11 11:26 ` [PATCH 2/3] lei_saved_search: canonicalized relative save paths Eric Wong
2021-08-11 11:26 ` [PATCH 3/3] lei: attempt to canonicalize away "/../" pathnames Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-08-11 11:26 UTC (permalink / raw)
To: meta
None of our code elsewhere accounts for non-*nix pathnames and
it's not worth our time to start. So stop wasting CPU cycles
giving the illusion that we'd care about non-*nix pathnames.
---
lib/PublicInbox/IMAPTracker.pm | 4 ++--
lib/PublicInbox/LEI.pm | 2 +-
lib/PublicInbox/OverIdx.pm | 4 ++--
lib/PublicInbox/Xapcmd.pm | 5 ++---
script/public-inbox-init | 3 +--
t/init.t | 1 -
6 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/lib/PublicInbox/IMAPTracker.pm b/lib/PublicInbox/IMAPTracker.pm
index 5eb33cf7..2fd66440 100644
--- a/lib/PublicInbox/IMAPTracker.pm
+++ b/lib/PublicInbox/IMAPTracker.pm
@@ -75,9 +75,9 @@ sub new {
}
if (!-f $dbname) {
require File::Path;
- require File::Basename;
require PublicInbox::Spawn;
- File::Path::mkpath(File::Basename::dirname($dbname));
+ my ($dir) = ($dbname =~ m!(.*?/)[^/]+\z!);
+ File::Path::mkpath($dir);
open my $fh, '+>>', $dbname or die "failed to open $dbname: $!";
PublicInbox::Spawn::nodatacow_fd(fileno($fh));
}
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e6f763e1..be4754df 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -798,7 +798,7 @@ sub _lei_cfg ($;$) {
delete $self->{cfg};
return bless {}, 'PublicInbox::Config';
}
- my (undef, $cfg_dir, undef) = File::Spec->splitpath($f);
+ my ($cfg_dir) = ($f =~ m!(.*?/)[^/]+\z!);
-d $cfg_dir or mkpath($cfg_dir) or die "mkpath($cfg_dir): $!\n";
open my $fh, '>>', $f or die "open($f): $!\n";
@st = stat($fh) or die "fstat($f): $!\n";
diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 8f7cf2bb..e0893337 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -463,8 +463,8 @@ sub create {
};
unless (-r $fn) {
require File::Path;
- require File::Basename;
- File::Path::mkpath(File::Basename::dirname($fn));
+ my ($dir) = ($fn =~ m!(.*?/)[^/]+\z!);
+ File::Path::mkpath($dir);
}
# create the DB:
PublicInbox::Over::dbh($self);
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 8b8958c7..588e7b94 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -8,7 +8,6 @@ use PublicInbox::Over;
use PublicInbox::SearchIdx;
use File::Temp 0.19 (); # ->newdir
use File::Path qw(remove_tree);
-use File::Basename qw(dirname);
use POSIX qw(WNOHANG _exit);
# support testing with dev versions of Xapian which installs
@@ -199,7 +198,7 @@ sub prepare_run {
warn
"--reshard=$reshard ignored for v1 $ibx->{inboxdir}\n";
}
- my $dir = dirname($old);
+ my ($dir) = ($old =~ m!(.*?/)[^/]+/*\z!);
same_fs_or_die($dir, $old);
my $v = PublicInbox::Search::SCHEMA_VERSION();
my $wip = File::Temp->newdir("xapian$v-XXXX", DIR => $dir);
@@ -431,7 +430,7 @@ sub cpdb ($$) { # cb_spawn callback
my ($tmp, $ft);
local %SIG = %SIG;
if ($opt->{compact}) {
- my $dir = dirname($new);
+ my ($dir) = ($new =~ m!(.*?/)[^/]+/*\z!);
same_fs_or_die($dir, $new);
$ft = File::Temp->newdir("$new.compact-XXXX", DIR => $dir);
setup_signals();
diff --git a/script/public-inbox-init b/script/public-inbox-init
index 6fac4d18..ced88235 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -93,8 +93,7 @@ $ng =~ m![^A-Za-z0-9/_\.\-\~\@\+\=:]! and
require PublicInbox::Config;
my $pi_config = PublicInbox::Config->default_file;
-require File::Basename;
-my $dir = File::Basename::dirname($pi_config);
+my ($dir) = ($pi_config =~ m!(.*?/)[^/]+\z!);
require File::Path;
File::Path::mkpath($dir); # will croak on fatal errors
diff --git a/t/init.t b/t/init.t
index 7382e05b..efa3314d 100644
--- a/t/init.t
+++ b/t/init.t
@@ -6,7 +6,6 @@ use Test::More;
use PublicInbox::Config;
use PublicInbox::TestCommon;
use PublicInbox::Admin;
-use File::Basename;
my ($tmpdir, $for_destroy) = tmpdir();
sub quiet_fail {
my ($cmd, $msg) = @_;
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/3] lei_saved_search: canonicalized relative save paths
2021-08-11 11:26 [PATCH 0/3] lei pathname canonicalization fixes Eric Wong
2021-08-11 11:26 ` [PATCH 1/3] treewide: use *nix-specific dirname regexps Eric Wong
@ 2021-08-11 11:26 ` Eric Wong
2021-08-11 11:26 ` [PATCH 3/3] lei: attempt to canonicalize away "/../" pathnames Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-08-11 11:26 UTC (permalink / raw)
To: meta
Storing relative paths with '..' in them can be expensive to
resolve when running 'lei up', so prefer storing canonicalized
absolute paths. We only do this for paths with '..' in them,
though, since this can lose symlink info.
---
lib/PublicInbox/LeiSavedSearch.pm | 9 ++++++++-
t/lei-q-save.t | 9 +++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm
index cfbf68c3..2a0e9321 100644
--- a/lib/PublicInbox/LeiSavedSearch.pm
+++ b/lib/PublicInbox/LeiSavedSearch.pm
@@ -115,9 +115,16 @@ sub up { # updating existing saved search via "lei up"
sub new { # new saved search "lei q --save"
my ($cls, $lei) = @_;
my $self = bless { ale => $lei->ale }, $cls;
+ require File::Path;
my $dst = $lei->{ovv}->{dst};
+
+ # canonicalize away relative paths into the config
+ if ($lei->{ovv}->{fmt} eq 'maildir' &&
+ $dst =~ m!(?:/*|\A)\.\.(?:/*|\z)! && !-d $dst) {
+ File::Path::make_path($dst);
+ $lei->{ovv}->{dst} = $dst = $lei->abs_path($dst);
+ }
my $dir = lss_dir_for($lei, \$dst);
- require File::Path;
File::Path::make_path($dir); # raises on error
$self->{-cfg} = {};
my $f = $self->{'-f'} = "$dir/lei.saved-search";
diff --git a/t/lei-q-save.t b/t/lei-q-save.t
index b1ca4e92..eada2dd4 100644
--- a/t/lei-q-save.t
+++ b/t/lei-q-save.t
@@ -202,5 +202,14 @@ test_lei(sub {
lei_ok([qw(edit-search), $v2s], { VISUAL => 'cat', EDITOR => 'cat' });
like($lei_out, qr/^\[lei/sm, 'edit-search can cat');
+
+ lei_ok('-C', "$home/v2s",
+ qw(q -q --save -o ../s m:testmessage@example.com));
+ lei_ok qw(ls-search);
+ unlike $lei_out, qr{/\.\./s$}sm, 'relative path not in ls-search';
+ like $lei_out, qr{^\Q$home\E/s$}sm,
+ 'absolute path appears in ls-search';
+ lei_ok qw(up ../s -C), "$home/v2s", \'relative lei up';
+ lei_ok qw(up), "$home/s", \'absolute lei up';
});
done_testing;
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/3] lei: attempt to canonicalize away "/../" pathnames
2021-08-11 11:26 [PATCH 0/3] lei pathname canonicalization fixes Eric Wong
2021-08-11 11:26 ` [PATCH 1/3] treewide: use *nix-specific dirname regexps Eric Wong
2021-08-11 11:26 ` [PATCH 2/3] lei_saved_search: canonicalized relative save paths Eric Wong
@ 2021-08-11 11:26 ` Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-08-11 11:26 UTC (permalink / raw)
To: meta
As documented, File::Spec->canonpath does not canonicalize
"/../". While we want to do our best to preserve symlinks in
pathnames, leaving "/../" can mislead our inotify|kqueue usage.
---
lib/PublicInbox/LEI.pm | 14 ++++++++++----
lib/PublicInbox/LeiALE.pm | 8 ++++----
lib/PublicInbox/LeiBlob.pm | 4 ++--
lib/PublicInbox/LeiInit.pm | 3 +--
lib/PublicInbox/LeiLcat.pm | 2 +-
lib/PublicInbox/LeiOverview.pm | 2 +-
lib/PublicInbox/LeiQuery.pm | 2 +-
lib/PublicInbox/LeiRediff.pm | 2 +-
lib/PublicInbox/LeiUp.pm | 2 +-
t/lei_xsearch.t | 8 +++++---
10 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index be4754df..54fac7b4 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -94,6 +94,12 @@ sub rel2abs {
# abs_path resolves symlinks in parent iff all parents exist
sub abs_path { Cwd::abs_path($_[1]) // rel2abs(@_) }
+sub canonpath_harder {
+ my $p = $_[-1]; # $_[0] may be self
+ $p = File::Spec->canonpath($p);
+ $p =~ m!(?:/*|\A)\.\.(?:/*|\z)! && -e $p ? Cwd::abs_path($p) : $p;
+}
+
sub share_path ($) { # $HOME/.local/share/lei/$FOO
my ($self) = @_;
rel2abs($self, ($self->{env}->{XDG_DATA_HOME} //
@@ -808,8 +814,8 @@ sub _lei_cfg ($;$) {
my $cfg = PublicInbox::Config->git_config_dump($f);
$cfg->{-st} = $cur_st;
$cfg->{'-f'} = $f;
- if ($sto && File::Spec->canonpath($sto_dir // store_path($self))
- eq File::Spec->canonpath($cfg->{'leistore.dir'} //
+ if ($sto && canonpath_harder($sto_dir // store_path($self))
+ eq canonpath_harder($cfg->{'leistore.dir'} //
store_path($self))) {
$cfg->{-lei_store} = $sto;
$cfg->{-lei_note_event} = $lne;
@@ -1382,7 +1388,7 @@ sub refresh_watches {
next;
}
if ($url =~ /\Amaildir:(.+)/i) {
- my $d = File::Spec->canonpath($1);
+ my $d = canonpath_harder($1);
if ($state eq 'pause') {
cancel_maildir_watch($d, $cfg_f);
} elsif (!exists($MDIR2CFGPATH->{$d}->{$cfg_f})) {
@@ -1400,7 +1406,7 @@ sub refresh_watches {
next if exists $seen{$url};
delete $old->{$url};
if ($url =~ /\Amaildir:(.+)/i) {
- my $d = File::Spec->canonpath($1);
+ my $d = canonpath_harder($1);
cancel_maildir_watch($d, $cfg_f);
} else { # TODO: imap/nntp/jmap
$lei->child_error(1, "E: watch $url TODO");
diff --git a/lib/PublicInbox/LeiALE.pm b/lib/PublicInbox/LeiALE.pm
index cb570ab4..cc9a2095 100644
--- a/lib/PublicInbox/LeiALE.pm
+++ b/lib/PublicInbox/LeiALE.pm
@@ -33,7 +33,7 @@ sub new {
for my $loc ($lei->externals_each) { # locals only
$lxs->prepare_external($loc) if -d $loc;
}
- $self->refresh_externals($lxs);
+ $self->refresh_externals($lxs, $lei);
$self;
}
@@ -50,7 +50,7 @@ sub overs_all { # for xoids_for (called only in lei workers?)
}
sub refresh_externals {
- my ($self, $lxs) = @_;
+ my ($self, $lxs, $lei) = @_;
$self->git->cleanup;
my $lk = $self->lock_for_scope;
my $cur_lxs = ref($lxs)->new;
@@ -72,7 +72,7 @@ sub refresh_externals {
}
my @ibxish = $cur_lxs->locals;
for my $x ($lxs->locals) {
- my $d = File::Spec->canonpath($x->{inboxdir} // $x->{topdir});
+ my $d = $lei->canonpath_harder($x->{inboxdir} // $x->{topdir});
$seen_ibxish{$d} //= do {
$new .= "$d\n";
push @ibxish, $x;
@@ -95,7 +95,7 @@ sub refresh_externals {
$old = <$fh> // die "readline($f): $!";
}
for my $x (@ibxish) {
- $new .= File::Spec->canonpath($x->git->{git_dir})."/objects\n";
+ $new .= $lei->canonpath_harder($x->git->{git_dir})."/objects\n";
}
$self->{ibxish} = \@ibxish;
return if $old eq $new;
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 09217964..3158ca3b 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -112,7 +112,7 @@ sub lei_blob {
if ($opt->{mail} // ($has_hints ? 0 : 1)) {
if (grep(defined, @$opt{qw(include only)})) {
$lxs = $lei->lxs_prepare;
- $lei->ale->refresh_externals($lxs);
+ $lei->ale->refresh_externals($lxs, $lei);
}
my $rdr = {};
if ($opt->{mail}) {
@@ -155,7 +155,7 @@ sub lei_blob {
return $lei->fail('no --git-dir to try') unless @$git_dirs;
unless ($lxs) {
$lxs = $lei->lxs_prepare or return;
- $lei->ale->refresh_externals($lxs);
+ $lei->ale->refresh_externals($lxs, $lei);
}
if ($lxs->remotes) {
require PublicInbox::LeiRemote;
diff --git a/lib/PublicInbox/LeiInit.pm b/lib/PublicInbox/LeiInit.pm
index c6c0c01b..6558ac0a 100644
--- a/lib/PublicInbox/LeiInit.pm
+++ b/lib/PublicInbox/LeiInit.pm
@@ -4,7 +4,6 @@
# for the "lei init" command, not sure if it's even needed...
package PublicInbox::LeiInit;
use v5.10.1;
-use File::Spec;
sub lei_init {
my ($self, $dir) = @_;
@@ -13,7 +12,7 @@ sub lei_init {
$dir //= $self->store_path;
$dir = $self->rel2abs($dir);
my @cur = stat($cur) if defined($cur);
- $cur = File::Spec->canonpath($cur // $dir);
+ $cur = $self->canonpath_harder($cur // $dir);
my @dir = stat($dir);
my $exists = "# leistore.dir=$cur already initialized" if @dir;
if (@cur) {
diff --git a/lib/PublicInbox/LeiLcat.pm b/lib/PublicInbox/LeiLcat.pm
index cb5eb5b4..4a0c24a9 100644
--- a/lib/PublicInbox/LeiLcat.pm
+++ b/lib/PublicInbox/LeiLcat.pm
@@ -128,7 +128,7 @@ sub _stdin { # PublicInbox::InputPipe::consume callback for --stdin
sub lei_lcat {
my ($lei, @argv) = @_;
my $lxs = $lei->lxs_prepare or return;
- $lei->ale->refresh_externals($lxs);
+ $lei->ale->refresh_externals($lxs, $lei);
my $sto = $lei->_lei_store(1);
$lei->{lse} = $sto->search;
my $opt = $lei->{opt};
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index e4242d9b..223db222 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -76,7 +76,7 @@ sub new {
$fmt //= $devfd >= 0 ? 'json' : (detect_fmt($lei, $dst) or return);
if (index($dst, '://') < 0) { # not a URL, so assume path
- $dst = File::Spec->canonpath($dst);
+ $dst = $lei->canonpath_harder($dst);
} # else URL
my $self = bless { fmt => $fmt, dst => $dst }, $class;
diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm
index eb7b98d4..37b660f9 100644
--- a/lib/PublicInbox/LeiQuery.pm
+++ b/lib/PublicInbox/LeiQuery.pm
@@ -116,7 +116,7 @@ sub lei_q {
my ($self, @argv) = @_;
PublicInbox::Config->json; # preload before forking
my $lxs = lxs_prepare($self) or return;
- $self->ale->refresh_externals($lxs);
+ $self->ale->refresh_externals($lxs, $self);
my $opt = $self->{opt};
my %mset_opt = map { $_ => $opt->{$_} } qw(threads limit offset);
$mset_opt{asc} = $opt->{'reverse'} ? 1 : 0;
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index 7607b44f..0ba5897c 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -215,7 +215,7 @@ sub lei_rediff {
$lei->{curl} //= which('curl') or return
$lei->fail('curl needed for', $lxs->remotes);
}
- $lei->ale->refresh_externals($lxs);
+ $lei->ale->refresh_externals($lxs, $lei);
my $self = bless {
-force_eml => 1, # for LeiInput->input_fh
lxs => $lxs,
diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm
index 9069232b..3356d11e 100644
--- a/lib/PublicInbox/LeiUp.pm
+++ b/lib/PublicInbox/LeiUp.pm
@@ -42,7 +42,7 @@ sub up1 ($$) {
}
$lei->{lss} = $lss; # for LeiOverview->new
my $lxs = $lei->lxs_prepare or return;
- $lei->ale->refresh_externals($lxs);
+ $lei->ale->refresh_externals($lxs, $lei);
$lei->_start_query;
}
diff --git a/t/lei_xsearch.t b/t/lei_xsearch.t
index 3eb44233..d9ddb297 100644
--- a/t/lei_xsearch.t
+++ b/t/lei_xsearch.t
@@ -11,6 +11,7 @@ require PublicInbox::ExtSearchIdx;
require_git 2.6;
require_ok 'PublicInbox::LeiXSearch';
require_ok 'PublicInbox::LeiALE';
+require_ok 'PublicInbox::LEI';
my ($home, $for_destroy) = tmpdir();
my @ibx;
for my $V (1..2) {
@@ -88,18 +89,19 @@ is($lxs->over, undef, '->over fails');
my $smsg = $lxs->smsg_for($mitem) or BAIL_OUT 'smsg_for broken';
my $ale = PublicInbox::LeiALE::_new("$home/ale");
- $ale->refresh_externals($lxs);
+ my $lei = bless {}, 'PublicInbox::LEI';
+ $ale->refresh_externals($lxs, $lei);
my $exp = [ $smsg->{blob}, 'blob', -s 't/utf8.eml' ];
is_deeply([ $ale->git->check($smsg->{blob}) ], $exp, 'ale->git->check');
$lxs = PublicInbox::LeiXSearch->new;
$lxs->prepare_external($v2ibx);
- $ale->refresh_externals($lxs);
+ $ale->refresh_externals($lxs, $lei);
is_deeply([ $ale->git->check($smsg->{blob}) ], $exp,
'ale->git->check remembered inactive external');
rename("$home/v1tmp", "$home/v1moved") or BAIL_OUT "rename: $!";
- $ale->refresh_externals($lxs);
+ $ale->refresh_externals($lxs, $lei);
is($ale->git->check($smsg->{blob}), undef,
'missing after directory gone');
}
^ permalink raw reply related [flat|nested] 4+ messages in thread