From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-2.9 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, URIBL_SBL,URIBL_SBL_A shortcircuit=no autolearn=no 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 F41281FA18 for ; Mon, 21 Dec 2020 07:51:22 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/5] use rel2abs_collapsed when loading Inbox objects Date: Mon, 21 Dec 2020 07:51:20 +0000 Message-Id: <20201221075122.1587-4-e@80x24.org> In-Reply-To: <20201221075122.1587-1-e@80x24.org> References: <20201221075122.1587-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: We need to canonicalize paths for inboxes which do not have a newsgroup defined, otherwise ->eidx_key matches can fail in unexpected ways. --- lib/PublicInbox/Admin.pm | 11 +---------- lib/PublicInbox/Config.pm | 28 +++++++++++++++++++++++----- lib/PublicInbox/ExtSearchIdx.pm | 5 ----- script/public-inbox-convert | 2 +- script/public-inbox-init | 2 +- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm index ea82133a..c972fb68 100644 --- a/lib/PublicInbox/Admin.pm +++ b/lib/PublicInbox/Admin.pm @@ -10,7 +10,7 @@ our @EXPORT_OK = qw(setup_signals); use PublicInbox::Config; use PublicInbox::Inbox; use PublicInbox::Spawn qw(popen_rd); -use File::Spec (); +*rel2abs_collapsed = \&PublicInbox::Config::rel2abs_collapsed; sub setup_signals { my ($cb, $arg) = @_; # optional @@ -27,15 +27,6 @@ sub setup_signals { }; } -# abs_path resolves symlinks, so we want to avoid it if rel2abs -# is sufficient and doesn't leave "/.." or "/../" -sub rel2abs_collapsed ($) { - my $p = File::Spec->rel2abs($_[0]); - return $p if substr($p, -3, 3) ne '/..' && index($p, '/../') < 0; # likely - require Cwd; - Cwd::abs_path($p); -} - sub resolve_inboxdir { my ($cd, $ver) = @_; my $try = $cd // '.'; diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm index 2f5c83cd..577337dc 100644 --- a/lib/PublicInbox/Config.pm +++ b/lib/PublicInbox/Config.pm @@ -368,6 +368,16 @@ sub git_bool { } } +# abs_path resolves symlinks, so we want to avoid it if rel2abs +# is sufficient and doesn't leave "/.." or "/../" +sub rel2abs_collapsed { + require File::Spec; + my $p = File::Spec->rel2abs($_[-1]); + return $p if substr($p, -3, 3) ne '/..' && index($p, '/../') < 0; + require Cwd; + Cwd::abs_path($p); +} + sub _fill { my ($self, $pfx) = @_; my $ibx = {}; @@ -391,9 +401,9 @@ EOF } # "mainrepo" is backwards compatibility: - $ibx->{inboxdir} //= $self->{"$pfx.mainrepo"} // return; - if ($ibx->{inboxdir} =~ /\n/s) { - warn "E: `$ibx->{inboxdir}' must not contain `\\n'\n"; + my $dir = $ibx->{inboxdir} //= $self->{"$pfx.mainrepo"} // return; + if (index($dir, "\n") >= 0) { + warn "E: `$dir' must not contain `\\n'\n"; return; } foreach my $k (qw(obfuscate)) { @@ -436,7 +446,7 @@ EOF $self->{-by_list_id}->{lc($list_id)} = $ibx; } } - if (my $ngname = $ibx->{newsgroup}) { + if (defined(my $ngname = $ibx->{newsgroup})) { if (ref($ngname)) { delete $ibx->{newsgroup}; warn 'multiple newsgroups not supported: '. @@ -445,7 +455,8 @@ EOF # wildmat-exact and RFC 3501 (IMAP) ATOM-CHAR. # Leave out a few chars likely to cause problems or conflicts: # '|', '<', '>', ';', '#', '$', '&', - } elsif ($ngname =~ m![^A-Za-z0-9/_\.\-\~\@\+\=:]!) { + } elsif ($ngname =~ m![^A-Za-z0-9/_\.\-\~\@\+\=:]! || + $ngname eq '') { delete $ibx->{newsgroup}; warn "newsgroup name invalid: `$ngname'\n"; } else { @@ -454,6 +465,13 @@ EOF $self->{-by_newsgroup}->{$ngname} = $ibx; } } + unless (defined $ibx->{newsgroup}) { # for ->eidx_key + my $abs = rel2abs_collapsed($dir); + if ($abs ne $dir) { + warn "W: `$dir' canonicalized to `$abs'\n"; + $ibx->{inboxdir} = $abs; + } + } $self->{-by_name}->{$name} = $ibx; if ($ibx->{obfuscate}) { $ibx->{-no_obfuscate} = $self->{-no_obfuscate}; diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index b82d0546..c4b429df 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -72,11 +72,6 @@ sub attach_inbox { warn "W: skipping $key (no UIDVALIDITY)\n"; return; } - my $ibxdir = File::Spec->canonpath($ibx->{inboxdir}); - if ($ibxdir ne $ibx->{inboxdir}) { - warn "W: `$ibx->{inboxdir}' canonicalized to `$ibxdir'\n"; - $ibx->{inboxdir} = $ibxdir; - } $self->{ibx_map}->{$key} //= do { push @{$self->{ibx_list}}, $ibx; $ibx; diff --git a/script/public-inbox-convert b/script/public-inbox-convert index fbd527a6..800c364c 100755 --- a/script/public-inbox-convert +++ b/script/public-inbox-convert @@ -75,7 +75,7 @@ if ($opt->{'index'}) { } local %ENV = (%$env, %ENV) if $env; my $new = { %$old }; -$new->{inboxdir} = PublicInbox::Admin::rel2abs_collapsed($new_dir); +$new->{inboxdir} = $cfg->rel2abs_collapsed($new_dir); $new->{version} = 2; $new = PublicInbox::InboxWritable->new($new, { nproc => $opt->{jobs} }); $new->{-no_fsync} = 1 if !$opt->{fsync}; diff --git a/script/public-inbox-init b/script/public-inbox-init index eb605a51..afaa4c12 100755 --- a/script/public-inbox-init +++ b/script/public-inbox-init @@ -138,7 +138,7 @@ close($fh) or die "failed to close $pi_config_tmp: $!\n"; my $pfx = "publicinbox.$name"; my @x = (qw/git config/, "--file=$pi_config_tmp"); -PublicInbox::Admin::rel2abs_collapsed($inboxdir); +PublicInbox::Config::rel2abs_collapsed($inboxdir); die "`\\n' not allowed in `$inboxdir'\n" if index($inboxdir, "\n") >= 0; if (-f "$inboxdir/inbox.lock") {