From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) 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.1 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 84B8B1F597; Sun, 29 Jul 2018 10:05:13 +0000 (UTC) Date: Sun, 29 Jul 2018 10:05:13 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH v2] mda: allow configuring globally without spamc support Message-ID: <20180729100513.GA9157@dcvr> References: <20180729003315.27069-1-e@80x24.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180729003315.27069-1-e@80x24.org> List-Id: This reuses some of the configuration from -watch, but remains independent since some configurations will use -watch for some inboxes and -mda for others. The default remains "spamc" for -mda users so nothing changes without explicit configuration. Per-inbox configurations may also be supported in the future. --- Documentation/public-inbox-config.pod | 9 ++++ Documentation/public-inbox-mda.pod | 4 ++ MANIFEST | 1 + lib/PublicInbox/Spamcheck.pm | 25 +++++++++++ lib/PublicInbox/WatchMaildir.pm | 17 ++----- script/public-inbox-mda | 27 +++++++---- t/v2mda.t | 65 +++++++++++++++++++++------ 7 files changed, 114 insertions(+), 34 deletions(-) create mode 100644 lib/PublicInbox/Spamcheck.pm diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod index 22ee909..f7353dc 100644 --- a/Documentation/public-inbox-config.pod +++ b/Documentation/public-inbox-config.pod @@ -91,6 +91,15 @@ C Default: none +=item publicinboxmda.spamcheck + +This may be set to C to disable the use of SpamAssassin +L for filtering spam before it is imported into git +history. Other spam filtering backends may be supported in +the future. + +Default: spamc + =item publicinboxwatch.spamcheck This may be set to C to enable the use of SpamAssassin diff --git a/Documentation/public-inbox-mda.pod b/Documentation/public-inbox-mda.pod index a6c704e..1a5ade8 100644 --- a/Documentation/public-inbox-mda.pod +++ b/Documentation/public-inbox-mda.pod @@ -12,6 +12,10 @@ Mail Delivery Agent (MDA) for public-inbox installations. Each system user may have their own public-inbox instances. This may be invoked via L or similar tools. +By default, it relies on L for filtering mail, +but may be disabled via +L + =head1 ENVIRONMENT =over 8 diff --git a/MANIFEST b/MANIFEST index 003c3c5..a455dba 100644 --- a/MANIFEST +++ b/MANIFEST @@ -98,6 +98,7 @@ lib/PublicInbox/SearchIdxPart.pm lib/PublicInbox/SearchMsg.pm lib/PublicInbox/SearchThread.pm lib/PublicInbox/SearchView.pm +lib/PublicInbox/Spamcheck.pm lib/PublicInbox/Spamcheck/Spamc.pm lib/PublicInbox/Spawn.pm lib/PublicInbox/SpawnPP.pm diff --git a/lib/PublicInbox/Spamcheck.pm b/lib/PublicInbox/Spamcheck.pm new file mode 100644 index 0000000..062479d --- /dev/null +++ b/lib/PublicInbox/Spamcheck.pm @@ -0,0 +1,25 @@ +# Copyright (C) 2018 all contributors +# License: AGPL-3.0+ +package PublicInbox::Spamcheck; +use strict; +use warnings; + +sub get { + my ($config, $key, $default) = @_; + my $spamcheck = $config->{$key}; + $spamcheck = $default unless $spamcheck; + + return if !$spamcheck || $spamcheck eq 'none'; + + if ($spamcheck eq 'spamc') { + $spamcheck = 'PublicInbox::Spamcheck::Spamc'; + } + if ($spamcheck =~ /::/) { + eval "require $spamcheck"; + return $spamcheck->new; + } + warn "unsupported $key=$spamcheck\n"; + undef; +} + +1; diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm index 10dc618..13dea16 100644 --- a/lib/PublicInbox/WatchMaildir.pm +++ b/lib/PublicInbox/WatchMaildir.pm @@ -14,6 +14,7 @@ use PublicInbox::Spawn qw(spawn); use PublicInbox::InboxWritable; use File::Temp qw//; use PublicInbox::Filter::Base; +use PublicInbox::Spamcheck; *REJECT = *PublicInbox::Filter::Base::REJECT; sub new { @@ -40,19 +41,9 @@ sub new { } my $k = 'publicinboxwatch.spamcheck'; - my $spamcheck = $config->{$k}; - if ($spamcheck) { - if ($spamcheck eq 'spamc') { - $spamcheck = 'PublicInbox::Spamcheck::Spamc'; - } - if ($spamcheck =~ /::/) { - eval "require $spamcheck"; - $spamcheck = _spamcheck_cb($spamcheck->new); - } else { - warn "unsupported $k=$spamcheck\n"; - $spamcheck = undef; - } - } + my $default = undef; + my $spamcheck = PublicInbox::Spamcheck::get($config, $k, $default); + $spamcheck = _spamcheck_cb($spamcheck) if $spamcheck; # need to make all inboxes writable for spam removal: $config->each_inbox(sub { PublicInbox::InboxWritable->new($_[0]) }); diff --git a/script/public-inbox-mda b/script/public-inbox-mda index 2b7f298..183b915 100755 --- a/script/public-inbox-mda +++ b/script/public-inbox-mda @@ -20,8 +20,8 @@ use PublicInbox::MDA; use PublicInbox::Config; use PublicInbox::Emergency; use PublicInbox::Filter::Base; -use PublicInbox::Spamcheck::Spamc; use PublicInbox::InboxWritable; +use PublicInbox::Spamcheck; # n.b: hopefully we can setup the emergency path without bailing due to # user error, we really want to setup the emergency destination ASAP @@ -33,7 +33,9 @@ $str =~ s/\A[\r\n]*From [^\r\n]*\r?\n//s; $ems->prepare(\$str); my $simple = Email::Simple->new(\$str); my $config = PublicInbox::Config->new; - +my $key = 'publicinboxmda.spamcheck'; +my $default = 'PublicInbox::Spamcheck::Spamc'; +my $spamc = PublicInbox::Spamcheck::get($config, $key, $default); my $recipient = $ENV{ORIGINAL_RECIPIENT}; defined $recipient or die "ORIGINAL_RECIPIENT not defined in ENV\n"; my $dst = $config->lookup($recipient); # first check @@ -43,13 +45,22 @@ $dst = PublicInbox::InboxWritable->new($dst); # pre-check, MDA has stricter rules than an importer might; do_exit(0) unless PublicInbox::MDA->precheck($simple, $dst->{address}); -my $spamc = PublicInbox::Spamcheck::Spamc->new; -$str = ''; -my $spam_ok = $spamc->spamcheck($ems->fh, \$str); $simple = undef; -$emm = PublicInbox::Emergency->new($emergency); -$emm->prepare(\$str); -$ems = $ems->abort; +my $spam_ok; +if ($spamc) { + $str = ''; + $spam_ok = $spamc->spamcheck($ems->fh, \$str); + # update the emergency dump with the new message: + $emm = PublicInbox::Emergency->new($emergency); + $emm->prepare(\$str); + $ems = $ems->abort; +} else { # no spam checking configured: + $spam_ok = 1; + $emm = $ems; + my $fh = $emm->fh; + read($fh, $str, -s $fh); +} + my $mime = PublicInbox::MIME->new(\$str); do_exit(0) unless $spam_ok; diff --git a/t/v2mda.t b/t/v2mda.t index 6145720..d041ffd 100644 --- a/t/v2mda.t +++ b/t/v2mda.t @@ -35,11 +35,13 @@ my $mime = PublicInbox::MIME->create( my $mda = "blib/script/public-inbox-mda"; ok(-f "blib/script/public-inbox-mda", '-mda exists'); my $main_bin = getcwd()."/t/main-bin"; +my $fail_bin = getcwd()."/t/fail-bin"; local $ENV{PI_DIR} = "$tmpdir/foo"; +my $fail_path = "$fail_bin:blib/script:$ENV{PATH}"; local $ENV{PATH} = "$main_bin:blib/script:$ENV{PATH}"; -local $ENV{PI_EMERGENCY} = "$tmpdir/fail"; -ok(mkdir "$tmpdir/fail"); - +my $faildir = "$tmpdir/fail"; +local $ENV{PI_EMERGENCY} = $faildir; +ok(mkdir $faildir); my @cmd = (qw(public-inbox-init), "-V$V", $ibx->{name}, $ibx->{mainrepo}, 'http://localhost/test', $ibx->{address}->[0]); @@ -62,17 +64,54 @@ if ($V == 1) { ok(PublicInbox::Import::run_die($cmd, undef, $rdr), 'v1 indexed'); } my $msgs = $ibx->search->query(''); +is(scalar(@$msgs), 1, 'only got one message'); my $saved = $ibx->smsg_mime($msgs->[0]); is($saved->{mime}->as_string, $mime->as_string, 'injected message'); -my $patch = 't/data/0001.patch'; -open my $fh, '<', $patch or die "failed to open $patch: $!\n"; -$rdr = { 0 => fileno($fh) }; -ok(PublicInbox::Import::run_die(['public-inbox-mda'], undef, $rdr), - 'mda delivered a patch'); -my $post = $ibx->search->reopen->query('dfpost:6e006fd7'); -is(scalar(@$post), 1, 'got one result for dfpost'); -my $pre = $ibx->search->query('dfpre:090d998'); -is(scalar(@$pre), 1, 'got one result for dfpre'); -is($post->[0]->{blob}, $pre->[0]->{blob}, 'same message in both cases'); +{ + my @new = glob("$faildir/new/*"); + is_deeply(\@new, [], 'nothing in faildir'); + local $ENV{PATH} = $fail_path; + $mime->header_set('Message-ID', ''); + ok($tmp->sysseek(0, SEEK_SET) && + $tmp->truncate(0) && + $tmp->print($mime->as_string) && + $tmp->flush && + $tmp->sysseek(0, SEEK_SET), + 'rewound and rewrite temporary file'); + my $cmd = ['public-inbox-mda']; + ok(PublicInbox::Import::run_die($cmd, undef, $rdr), + 'mda did not die on "spam"'); + @new = glob("$faildir/new/*"); + is(scalar(@new), 1, 'got a message in faildir'); + $msgs = $ibx->search->reopen->query(''); + is(scalar(@$msgs), 1, 'no new message'); + + my $config = "$ENV{PI_DIR}/config"; + ok(-f $config, 'config exists'); + my $k = 'publicinboxmda.spamcheck'; + is(system('git', 'config', "--file=$config", $k, 'none'), 0, + 'disabled spamcheck for mda'); + ok($tmp->sysseek(0, SEEK_SET), 'rewound input file'); + + ok(PublicInbox::Import::run_die($cmd, undef, $rdr), 'mda did not die'); + my @again = glob("$faildir/new/*"); + is_deeply(\@again, \@new, 'no new message in faildir'); + $msgs = $ibx->search->reopen->query(''); + is(scalar(@$msgs), 2, 'new message added OK'); +} + +{ + my $patch = 't/data/0001.patch'; + open my $fh, '<', $patch or die "failed to open $patch: $!\n"; + $rdr = { 0 => fileno($fh) }; + ok(PublicInbox::Import::run_die(['public-inbox-mda'], undef, $rdr), + 'mda delivered a patch'); + my $post = $ibx->search->reopen->query('dfpost:6e006fd7'); + is(scalar(@$post), 1, 'got one result for dfpost'); + my $pre = $ibx->search->query('dfpre:090d998'); + is(scalar(@$pre), 1, 'got one result for dfpre'); + is($post->[0]->{blob}, $pre->[0]->{blob}, 'same message in both cases'); +} + done_testing(); -- EW