From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
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 46AF71F9F3
	for <meta@public-inbox.org>; Wed, 11 Aug 2021 11:26:19 +0000 (UTC)
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 3/3] lei: attempt to canonicalize away "/../" pathnames
Date: Wed, 11 Aug 2021 11:26:18 +0000
Message-Id: <20210811112618.24084-4-e@80x24.org>
In-Reply-To: <20210811112618.24084-1-e@80x24.org>
References: <20210811112618.24084-1-e@80x24.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
List-Id: <meta.public-inbox.org>

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');
 }