[PATCH 0/4] watch improvements for mirroring

[PATCH 0/4] watch improvements for mirroring

[Eric Wong]

public-inbox-watch is more liberal than public-inbox-mda,
so we need to make some adjustments about how it handles
messages with missing Message-IDs, Subjects, and such.

Eric Wong (4):
      emergency: avoid needless mkpath dependency
      watch_maildir: add scan test
      watch_maildir: spam removal support
      import: allow messages without subject

 lib/PublicInbox/Emergency.pm    |   4 +-
 lib/PublicInbox/Import.pm       |   7 +-
 lib/PublicInbox/WatchMaildir.pm | 141 ++++++++++++++++++++++++++++++----------
 t/watch_maildir.t               |  83 +++++++++++++++++++++++
 4 files changed, 197 insertions(+), 38 deletions(-)

[PATCH 1/4] emergency: avoid needless mkpath dependency

[Eric Wong]

Be more explicit and slightly speed up tests.
---
 lib/PublicInbox/Emergency.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Emergency.pm b/lib/PublicInbox/Emergency.pm
index e402d30..4ee8621 100644
--- a/lib/PublicInbox/Emergency.pm
+++ b/lib/PublicInbox/Emergency.pm
@@ -12,11 +12,11 @@ use IO::Handle;
 sub new {
 	my ($class, $dir) = @_;
 
+	-d $dir or mkdir($dir) or die "failed to mkdir($dir): $!\n";
 	foreach (qw(new tmp cur)) {
 		my $d = "$dir/$_";
 		next if -d $d;
-		require File::Path;
-		File::Path::mkpath($d); # croaks on fatal errors
+		-d $d or mkdir($d) or die "failed to mkdir($d): $!\n";
 	}
 	bless { dir => $dir, files => {}, t => 0, cnt => 0 }, $class;
 }

[PATCH 2/4] watch_maildir: add scan test

[Eric Wong]

This should be portable despite the intended use of this
directory being non-portable.
---
 lib/PublicInbox/WatchMaildir.pm |  7 ++++++-
 t/watch_maildir.t               | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 t/watch_maildir.t

diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 3536375..f1a21b9 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -45,10 +45,14 @@ sub new {
 	}, $class;
 }
 
+sub _done_for_now {
+	$_->done foreach values %{$_[0]->{importers}};
+}
+
 sub _try_fsn_paths {
 	my ($self, $paths) = @_;
 	_try_path($self, $_->{path}) foreach @$paths;
-	$_->done foreach values %{$self->{importers}};
+	_done_for_now($self);
 }
 
 sub _try_path {
@@ -133,6 +137,7 @@ sub scan {
 		}
 		closedir $dh;
 	}
+	_done_for_now($self);
 }
 
 1;
diff --git a/t/watch_maildir.t b/t/watch_maildir.t
new file mode 100644
index 0000000..6564a86
--- /dev/null
+++ b/t/watch_maildir.t
@@ -0,0 +1,39 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use Test::More;
+use File::Temp qw/tempdir/;
+use Email::MIME;
+use PublicInbox::Config;
+
+my $tmpdir = tempdir('watch_maildir-XXXXXX', TMPDIR => 1, CLEANUP => 1);
+my $git_dir = "$tmpdir/test.git";
+my $maildir = "$tmpdir/md";
+use_ok 'PublicInbox::WatchMaildir';
+use_ok 'PublicInbox::Emergency';
+my $cfgpfx = "publicinbox.test";
+my $addr = 'test-public@example.com';
+is(system(qw(git init -q --bare), $git_dir), 0, 'initialized git dir');
+
+my $msg = <<EOF;
+From: user\@example.com
+To: $addr
+Subject: spam
+Message-Id: <a\@b.com>
+Date: Sat, 18 Jun 2016 00:00:00 +0000
+
+msg
+EOF
+PublicInbox::Emergency->new($maildir)->prepare(\$msg);
+
+my $config = PublicInbox::Config->new({
+	"$cfgpfx.address" => $addr,
+	"$cfgpfx.mainrepo" => $git_dir,
+	"$cfgpfx.watch" => "maildir:$maildir",
+});
+
+PublicInbox::WatchMaildir->new($config)->scan;
+my $git = PublicInbox::Git->new($git_dir);
+my @list = $git->qx(qw(rev-list refs/heads/master));
+is(scalar @list, 1, 'one revision in rev-list');
+
+done_testing;

[PATCH 3/4] watch_maildir: spam removal support

[Eric Wong]

We can support spam removal by watching a special "spam"
Maildir, too.  We can run public-inbox-learn as a separate
step, and that command will be improved to support
auto-learning, too.
---
 lib/PublicInbox/WatchMaildir.pm | 134 ++++++++++++++++++++++++++++++----------
 t/watch_maildir.t               |  46 +++++++++++++-
 2 files changed, 146 insertions(+), 34 deletions(-)

diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index f1a21b9..cb64f89 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -9,11 +9,24 @@ $Email::MIME::ContentType::STRICT_PARAMS = 0; # user input is imperfect
 use PublicInbox::Git;
 use PublicInbox::Import;
 use PublicInbox::MDA;
+use PublicInbox::Spawn qw(spawn);
 
 sub new {
 	my ($class, $config) = @_;
 	my (%mdmap, @mdir);
-	foreach my $k (keys %$config) {
+	my $k = 'publicinboxlearn.watchspam';
+	if (my $spamdir = $config->{$k}) {
+		if ($spamdir =~ s/\Amaildir://) {
+			$spamdir =~ s!/+\z!!;
+			# skip "new", no MUA has seen it, yet.
+			my $cur = "$spamdir/cur";
+			push @mdir, $cur;
+			$mdmap{$cur} = 'watchspam';
+		} else {
+			warn "unsupported $k=$spamdir\n";
+		}
+	}
+	foreach $k (keys %$config) {
 		$k =~ /\Apublicinbox\.([^\.]+)\.watch\z/ or next;
 		my $name = $1;
 		my $watch = $config->{$k};
@@ -27,8 +40,9 @@ sub new {
 			my $new = "$watch/new";
 			my $cur = "$watch/cur";
 			push @mdir, $new, $cur;
-			$mdmap{$new} = $inbox;
-			$mdmap{$cur} = $inbox;
+			die "$new already in use\n" if $mdmap{$new};
+			die "$cur already in use\n" if $mdmap{$cur};
+			$mdmap{$new} = $mdmap{$cur} = $inbox;
 		} else {
 			warn "watch unsupported: $k=$watch\n";
 		}
@@ -55,6 +69,42 @@ sub _try_fsn_paths {
 	_done_for_now($self);
 }
 
+sub _check_spam {
+	my ($self, $path) = @_;
+	my $mime = _path_to_mime($path) or return;
+	_force_mid($mime);
+	foreach my $inbox (values %{$self->{mdmap}}) {
+		next unless ref $inbox;
+		my $im = _importer_for($self, $inbox);
+		$im->remove($mime);
+		if (my $scrub = _scrubber_for($inbox)) {
+			my $scrubbed = $scrub->scrub($mime) or next;
+			$im->remove($scrubbed);
+		}
+	}
+}
+
+# used to hash the relevant portions of a message when there are conflicts
+sub _hash_mime2 {
+	my ($mime) = @_;
+	require Digest::SHA;
+	my $dig = Digest::SHA->new('SHA-1');
+	$dig->add($mime->header_obj->header_raw('Subject'));
+	$dig->add($mime->body_raw);
+	$dig->hexdigest;
+}
+
+sub _force_mid {
+	my ($mime) = @_;
+	# probably a bad idea, but we inject a Message-Id if
+	# one is missing, here..
+	my $mid = $mime->header_obj->header_raw('Message-Id');
+	if (!defined $mid || $mid =~ /\A\s*\z/) {
+		$mid = '<' . _hash_mime2($mime) . '@generated>';
+		$mime->header_set('Message-Id', $mid);
+	}
+}
+
 sub _try_path {
 	my ($self, $path) = @_;
 	if ($path !~ $self->{mdre}) {
@@ -66,44 +116,22 @@ sub _try_path {
 		warn "unmappable dir: $1\n";
 		return;
 	}
-	my $im = $inbox->{-import} ||= eval {
-		my $git = $inbox->git;
-		my $name = $inbox->{name};
-		my $addr = $inbox->{-primary_address};
-		PublicInbox::Import->new($git, $name, $addr);
-	};
-	$self->{importers}->{"$im"} = $im;
-	my $mime;
-	if (open my $fh, '<', $path) {
-		local $/;
-		my $str = <$fh>;
-		$str or return;
-		$mime = Email::MIME->new(\$str);
-	} elsif ($!{ENOENT}) {
-		return;
-	} else {
-		warn "failed to open $path: $!\n";
-		return;
+	if (!ref($inbox) && $inbox eq 'watchspam') {
+		return _check_spam($self, $path);
 	}
-
+	my $im = _importer_for($self, $inbox);
+	my $mime = _path_to_mime($path) or return;
 	$mime->header_set($_) foreach @PublicInbox::MDA::BAD_HEADERS;
 	my $wm = $inbox->{-watchheader};
 	if ($wm) {
 		my $v = $mime->header_obj->header_raw($wm->[0]);
 		return unless ($v && $v =~ $wm->[1]);
 	}
-	my $f = $inbox->{filter};
-	if ($f && $f =~ /::/) {
-		eval "require $f";
-		if ($@) {
-			warn $@;
-		} else {
-			$f = $f->new;
-			$mime = $f->scrub($mime);
-		}
+	if (my $scrub = _scrubber_for($inbox)) {
+		$mime = $scrub->scrub($mime) or return;
 	}
-	$mime or return;
-	my $mid = $mime->header_obj->header_raw('Message-Id');
+
+	_force_mid($mime);
 	$im->add($mime);
 }
 
@@ -140,4 +168,44 @@ sub scan {
 	_done_for_now($self);
 }
 
+sub _path_to_mime {
+	my ($path) = @_;
+	if (open my $fh, '<', $path) {
+		local $/;
+		my $str = <$fh>;
+		$str or return;
+		return Email::MIME->new(\$str);
+	} elsif ($!{ENOENT}) {
+		return;
+	} else {
+		warn "failed to open $path: $!\n";
+		return;
+	}
+}
+
+sub _importer_for {
+	my ($self, $inbox) = @_;
+	my $im = $inbox->{-import} ||= eval {
+		my $git = $inbox->git;
+		my $name = $inbox->{name};
+		my $addr = $inbox->{-primary_address};
+		PublicInbox::Import->new($git, $name, $addr);
+	};
+	$self->{importers}->{"$im"} = $im;
+}
+
+sub _scrubber_for {
+	my ($inbox) = @_;
+	my $f = $inbox->{filter};
+	if ($f && $f =~ /::/) {
+		eval "require $f";
+		if ($@) {
+			warn $@;
+		} else {
+			return $f->new;
+		}
+	}
+	undef;
+}
+
 1;
diff --git a/t/watch_maildir.t b/t/watch_maildir.t
index 6564a86..8a2c934 100644
--- a/t/watch_maildir.t
+++ b/t/watch_maildir.t
@@ -8,6 +8,7 @@ use PublicInbox::Config;
 my $tmpdir = tempdir('watch_maildir-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $git_dir = "$tmpdir/test.git";
 my $maildir = "$tmpdir/md";
+my $spamdir = "$tmpdir/spam";
 use_ok 'PublicInbox::WatchMaildir';
 use_ok 'PublicInbox::Emergency';
 my $cfgpfx = "publicinbox.test";
@@ -21,14 +22,17 @@ Subject: spam
 Message-Id: <a\@b.com>
 Date: Sat, 18 Jun 2016 00:00:00 +0000
 
-msg
+something
 EOF
 PublicInbox::Emergency->new($maildir)->prepare(\$msg);
+my $sem = PublicInbox::Emergency->new($spamdir); # create dirs
 
 my $config = PublicInbox::Config->new({
 	"$cfgpfx.address" => $addr,
 	"$cfgpfx.mainrepo" => $git_dir,
 	"$cfgpfx.watch" => "maildir:$maildir",
+	"$cfgpfx.filter" => 'PublicInbox::Filter::Vger',
+	"publicinboxlearn.watchspam" => "maildir:$spamdir",
 });
 
 PublicInbox::WatchMaildir->new($config)->scan;
@@ -36,4 +40,44 @@ my $git = PublicInbox::Git->new($git_dir);
 my @list = $git->qx(qw(rev-list refs/heads/master));
 is(scalar @list, 1, 'one revision in rev-list');
 
+my $write_spam = sub {
+	is(scalar glob("$spamdir/new/*"), undef, 'no spam existing');
+	$sem->prepare(\$msg);
+	$sem->commit;
+	my @new = glob("$spamdir/new/*");
+	is(scalar @new, 1);
+	my @p = split(m!/+!, $new[0]);
+	ok(link($new[0], "$spamdir/cur/".$p[-1]));
+	is(unlink($new[0]), 1);
+};
+$write_spam->();
+is(unlink(glob("$maildir/new/*")), 1, 'unlinked old spam');
+PublicInbox::WatchMaildir->new($config)->scan;
+@list = $git->qx(qw(rev-list refs/heads/master));
+is(scalar @list, 2, 'two revisions in rev-list');
+@list = $git->qx(qw(ls-tree -r --name-only refs/heads/master));
+is(scalar @list, 0, 'tree is empty');
+
+# check with scrubbing
+{
+	$msg .= qq(--
+To unsubscribe from this list: send the line "unsubscribe git" in
+the body of a message to majordomo\@vger.kernel.org
+More majordomo info at  http://vger.kernel.org/majordomo-info.html\n);
+	PublicInbox::Emergency->new($maildir)->prepare(\$msg);
+	PublicInbox::WatchMaildir->new($config)->scan;
+	@list = $git->qx(qw(ls-tree -r --name-only refs/heads/master));
+	is(scalar @list, 1, 'tree has one file');
+	my $mref = $git->cat_file('HEAD:'.$list[0]);
+	like($$mref, qr/something\n\z/s, 'message scrubbed on import');
+
+	is(unlink(glob("$maildir/new/*")), 1, 'unlinked spam');
+	$write_spam->();
+	PublicInbox::WatchMaildir->new($config)->scan;
+	@list = $git->qx(qw(ls-tree -r --name-only refs/heads/master));
+	is(scalar @list, 0, 'tree is empty');
+	@list = $git->qx(qw(rev-list refs/heads/master));
+	is(scalar @list, 4, 'four revisions in rev-list');
+}
+
 done_testing;

[PATCH 4/4] import: allow messages without subject

[Eric Wong]

Because our WatchMaildir module is liberal about what
it accepts, we can potentially have messages without a
subject.
---
 lib/PublicInbox/Import.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index c13d483..5ffc26e 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -112,8 +112,11 @@ sub remove {
 	defined($n) or die "read final byte of cat-blob failed: $!";
 	die "bad read on final byte: <$lf>" if $lf ne "\n";
 	my $cur = Email::MIME->new($buf);
-	if ($cur->header('Subject') ne $mime->header('Subject') ||
-			norm_body($cur) ne norm_body($mime)) {
+	my $cur_s = $cur->header('Subject');
+	$cur_s = '' unless defined $cur_s;
+	my $cur_m = $mime->header('Subject');
+	$cur_m = '' unless defined $cur_m;
+	if ($cur_s ne $cur_m || norm_body($cur) ne norm_body($mime)) {
 		return ('MISMATCH', $cur);
 	}
 

[PATCH 5/4] watch_maildir: tighten up path checks

[Eric Wong]

Only mark seen messages as spam, otherwise it could be
too aggressive and cause problems or over training.
We wouldn't want a wayward FIFO ruining our day, either :)
---
 lib/PublicInbox/WatchMaildir.pm | 12 +++++-------
 t/watch_maildir.t               |  3 ++-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index cb64f89..4468a44 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -71,6 +71,7 @@ sub _try_fsn_paths {
 
 sub _check_spam {
 	my ($self, $path) = @_;
+	$path =~ /:2,[A-R]*S[T-Z]*\z/ or return;
 	my $mime = _path_to_mime($path) or return;
 	_force_mid($mime);
 	foreach my $inbox (values %{$self->{mdmap}}) {
@@ -107,6 +108,9 @@ sub _force_mid {
 
 sub _try_path {
 	my ($self, $path) = @_;
+	my @p = split(m!/+!, $path);
+	return unless $p[-1] =~ /\A[a-zA-Z0-9][\w:,=\.]+\z/;
+	return unless -f $path;
 	if ($path !~ $self->{mdre}) {
 		warn "unrecognized path: $path\n";
 		return;
@@ -155,13 +159,7 @@ sub scan {
 			next;
 		}
 		while (my $fn = readdir($dh)) {
-			next unless $fn =~ /\A[a-zA-Z0-9][\w:,=\.]+\z/;
-			$fn = "$dir/$fn";
-			if (-f $fn) {
-				_try_path($self, $fn);
-			} else {
-				warn "not a file: $fn\n";
-			}
+			_try_path($self, "$dir/$fn");
 		}
 		closedir $dh;
 	}
diff --git a/t/watch_maildir.t b/t/watch_maildir.t
index 8a2c934..e8c9740 100644
--- a/t/watch_maildir.t
+++ b/t/watch_maildir.t
@@ -25,6 +25,7 @@ Date: Sat, 18 Jun 2016 00:00:00 +0000
 something
 EOF
 PublicInbox::Emergency->new($maildir)->prepare(\$msg);
+ok(POSIX::mkfifo("$maildir/cur/fifo", 0777));
 my $sem = PublicInbox::Emergency->new($spamdir); # create dirs
 
 my $config = PublicInbox::Config->new({
@@ -47,7 +48,7 @@ my $write_spam = sub {
 	my @new = glob("$spamdir/new/*");
 	is(scalar @new, 1);
 	my @p = split(m!/+!, $new[0]);
-	ok(link($new[0], "$spamdir/cur/".$p[-1]));
+	ok(link($new[0], "$spamdir/cur/".$p[-1].":2,S"));
 	is(unlink($new[0]), 1);
 };
 $write_spam->();