user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
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	[thread overview]
Message-ID: <20210319123557.19068-2-e@80x24.org> (raw)
In-Reply-To: <20210319123557.19068-1-e@80x24.org>

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

  reply	other threads:[~2021-03-19 12:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-19 12:35 [PATCH 0/2] newline rejection for new stuff Eric Wong
2021-03-19 12:35 ` Eric Wong [this message]
2021-03-19 12:35 ` [PATCH 2/2] config: ignore extindex entries with newlines in paths Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210319123557.19068-2-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).