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=-3.4 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, NUMERIC_HTTP_ADDR,WEIRD_PORT 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 14CEB1F9FD for ; Fri, 19 Mar 2021 12:35:58 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 1/2] lei: disallow "\n" in local externals paths Date: Fri, 19 Mar 2021 10:35:56 -0200 Message-Id: <20210319123557.19068-2-e@80x24.org> In-Reply-To: <20210319123557.19068-1-e@80x24.org> References: <20210319123557.19068-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: git 2.11 and earlier could not handle git directories with newlines in them, nor does libgit2 support them. Followup-to: d87dd0e679587043 ("config: reject `\n' in `inboxdir'") --- lib/PublicInbox/LeiExternal.pm | 5 +++++ lib/PublicInbox/LeiXSearch.pm | 2 ++ t/lei-externals.t | 8 ++++++-- t/lei-mirror.t | 5 +++++ t/lei.t | 12 ++++++++++++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/PublicInbox/LeiExternal.pm b/lib/PublicInbox/LeiExternal.pm index 47791d4e..b5dd85e1 100644 --- a/lib/PublicInbox/LeiExternal.pm +++ b/lib/PublicInbox/LeiExternal.pm @@ -170,9 +170,14 @@ sub lei_add_external { $self->fail(<<""); # TODO: did you mean "update-external?" --mirror destination `$location' already exists + } elsif (-d $location) { + index($location, "\n") >= 0 and + return $self->fail("`\\n' not allowed in `$location'"); } if ($location !~ m!\Ahttps?://! && !-d $location) { $mirror // return $self->fail("$location not a directory"); + index($location, "\n") >= 0 and + return $self->fail("`\\n' not allowed in `$location'"); $mirror = ext_canonicalize($mirror); require PublicInbox::LeiMirror; PublicInbox::LeiMirror->start($self, $mirror => $location); diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm index 22c8026c..d95a218e 100644 --- a/lib/PublicInbox/LeiXSearch.pm +++ b/lib/PublicInbox/LeiXSearch.pm @@ -502,8 +502,10 @@ sub prepare_external { return add_uri($self, URI->new($loc)); } elsif (-f "$loc/ei.lock") { require PublicInbox::ExtSearch; + die "`\\n' not allowed in `$loc'\n" if index($loc, "\n") >= 0; $loc = PublicInbox::ExtSearch->new($loc); } elsif (-f "$loc/inbox.lock" || -d "$loc/public-inbox") { + die "`\\n' not allowed in `$loc'\n" if index($loc, "\n") >= 0; require PublicInbox::Inbox; # v2, v1 $loc = bless { inboxdir => $loc }, 'PublicInbox::Inbox'; } else { diff --git a/t/lei-externals.t b/t/lei-externals.t index 2a92d101..1695ff0b 100644 --- a/t/lei-externals.t +++ b/t/lei-externals.t @@ -78,8 +78,12 @@ test_lei(sub { ok(!-e $config_file && !-e $store_dir, 'nothing created by ls-external'); - ok(!lei('add-external', "$home/nonexistent", - "fails on non-existent dir")); + ok(!lei('add-external', "$home/nonexistent"), + "fails on non-existent dir"); + like($lei_err, qr/not a directory/, 'noted non-existence'); + mkdir "$home/new\nline" or BAIL_OUT "mkdir: $!"; + ok(!lei('add-external', "$home/new\nline"), "fails on newline"); + like($lei_err, qr/`\\n' not allowed/, 'newline noted in error'); lei_ok('ls-external', \'ls-external works after add failure'); is($lei_out.$lei_err, '', 'ls-external still has no output'); my $cfg = PublicInbox::Config->new($cfg_path); diff --git a/t/lei-mirror.t b/t/lei-mirror.t index 1d113e3e..9769f31b 100644 --- a/t/lei-mirror.t +++ b/t/lei-mirror.t @@ -29,8 +29,13 @@ test_lei({ tmpdir => $tmpdir }, sub { ok(!lei('add-external', $t2, '--mirror', "$http/t2/"), '--mirror fails if reused') or diag "$lei_err.$lei_out = $?"; + ok(!lei('add-external', "$home/t2\nnewline", '--mirror', "$http/t2/"), + '--mirror fails on newline'); + like($lei_err, qr/`\\n' not allowed/, 'newline noted in error'); + lei_ok('ls-external'); like($lei_out, qr!\Q$t2\E!, 'still in ls-externals'); + unlike($lei_out, qr!\Qnewline\E!, 'newline entry not added'); ok(!lei('add-external', "$t2-fail", '-Lmedium'), '--mirror v2'); ok(!-d "$t2-fail", 'destination not created on failure'); diff --git a/t/lei.t b/t/lei.t index 74a775ca..2bf4b862 100644 --- a/t/lei.t +++ b/t/lei.t @@ -133,6 +133,18 @@ my $test_fail = sub { is($? >> 8, 1, 'chdir at end fails to /dev/null'); lei('-C', '/dev/null', 'q', 'whatever'); is($? >> 8, 1, 'chdir at beginning fails to /dev/null'); + + for my $lk (qw(ei inbox)) { + my $d = "$home/newline\n$lk"; + mkdir $d; + open my $fh, '>', "$d/$lk.lock" or BAIL_OUT "open $d/$lk.lock"; + for my $fl (qw(-I --only)) { + ok(!lei('q', $fl, $d, 'whatever'), + "newline $lk.lock fails with q $fl"); + like($lei_err, qr/`\\n' not allowed/, + "error noted with q $fl"); + } + } SKIP: { skip 'no curl', 3 unless which('curl'); lei(qw(q --only http://127.0.0.1:99999/bogus/ t:m));