watch: don't propagate header changes to other inboxes

Message-IDs may be appended by v2 inboxes for messages with with
conflicting Message-IDs or SpamAssassin (or another spam filter)
can change headers of both v1 and v2 messages.  So we rely on
`local' to reach into Eml internals to localize ->header_set
modifications and rely on modern Perl having copy-on-write
scalars to minimize memory traffic in the common case.

diff --git a/MANIFEST b/MANIFEST
index b0b4f71c2f4cc4902e53dcdd8808ecb64d5a8a21..30adab809e7bbf67a27fa8082b263303dafa02b7 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -641,6 +641,7 @@ t/watch_maildir.t
 t/watch_maildir_v2.t
 t/watch_mh.t
 t/watch_multiple_headers.t
+t/watch_v1_v2_mix_no_modify.t
 t/www_altid.t
 t/www_listing.t
 t/www_static.t

diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index a37038b598c995dbb978d4d32966a1b507a70fe3..eb6eb85f11137f91d64695d4e0ad97a178dec886 100644 (file)
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -210,6 +210,10 @@ sub _remove_spam {
 sub import_eml ($$$) {
        my ($self, $ibx, $eml) = @_;
 
+       # v2 may add a new Message-ID header on conflicts w/ different content
+       # and SpamAssassin may alter headers.  $copy is CoW in newer Perls.
+       local $eml->{hdr} = \(my $copy = ${$eml->{hdr}});
+
        # any header match means it's eligible for the inbox:
        if (my $watch_hdrs = $ibx->{-watchheaders}) {
                my $ok;

diff --git a/t/eml.t b/t/eml.t
index 690ada57937b24de6f919abf6cd76a41fc2dca03..d63f7a3beb9fb6e21b017f91121b425d3db0d27b 100644 (file)
--- a/t/eml.t
+++ b/t/eml.t
@@ -28,6 +28,15 @@ sub mime_load ($) {
        is($eml->as_string, "a: b\n\nhi\n", '->as_string');
        my $empty = PublicInbox::Eml->new("\n\n");
        is($empty->as_string, "\n\n", 'empty message');
+
+       # we use this pattern in -watch for writing an eml to multiple inboxes
+       my $in = do { # n.b. `my' must be used to assign a local var
+               local $eml->{hdr} = \(my $copy = ${$eml->{hdr}});
+               $eml->header_set(qw(Message-ID <a@example> <b@example>));
+               ${$eml->{hdr}};
+       };
+       like $in, qr/<a\@example>.*?<b\@example>/s, 'Message-ID set in copy';
+       is $eml->header_raw('Message-ID'), undef, 'local $eml->{hdr} works';
 }
 
 for my $cls (@classes) {

diff --git a/t/watch_v1_v2_mix_no_modify.t b/t/watch_v1_v2_mix_no_modify.t
new file mode 100644 (file)
index 0000000..759fd0b
--- /dev/null
+++ b/t/watch_v1_v2_mix_no_modify.t
@@ -0,0 +1,74 @@
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use v5.12;
+use autodie;
+use PublicInbox::TestCommon;
+require_mods 'v2';
+use PublicInbox::InboxIdle;
+use PublicInbox::DS qw(now);
+use PublicInbox::IO qw(write_file);
+use PublicInbox::Eml;
+my $tmpdir = tmpdir;
+local $ENV{HOME} = "$tmpdir";
+local $ENV{PI_CONFIG} = "$tmpdir/.public-inbox/config";
+my $msg = <<'EOM';
+From: a@example.com
+To: v1@example.com, v2@example.com
+Subject: test
+Message-ID: <a@example>
+
+hi
+EOM
+
+for my $v (2, 1) {
+       run_script(['-init', "-V$v", "v$v", '-Lbasic', "$tmpdir/v$v",
+               "https://example.com/v$v", "v$v\@example.com" ]) or
+                       xbail "init -V$v";
+       write_file '>>', $ENV{PI_CONFIG}, "watch = maildir:$tmpdir/md$v";
+       mkdir "$tmpdir/md$v/$_" for ('', qw(cur tmp new));
+}
+my $orig = "$tmpdir/md2/cur/initial:2,S";
+write_file '>', $orig, $msg;
+
+my $delivered = 0;
+my $cb = sub {
+       my ($ibx) = @_;
+       diag "message delivered to `$ibx->{name}'";
+       $delivered++;
+};
+my $ii = PublicInbox::InboxIdle->new(my $cfg = PublicInbox::Config->new);
+my $obj = bless \$cb, 'PublicInbox::TestCommon::InboxWakeup';
+$cfg->each_inbox(sub { $_[0]->subscribe_unlock('ident', $obj) });
+my $exp = now + 10;
+local @PublicInbox::DS::post_loop_do = (sub { $delivered < 1 || now > $exp});
+
+my $rdr;
+open $rdr->{2}, '>>', undef;
+my $w = start_script(['-watch'], undef, $rdr);
+diag 'waiting for -watch to import 2 existing messages..';
+PublicInbox::DS::add_timer 10, \&PublicInbox::Config::noop;
+PublicInbox::DS::event_loop;
+is $delivered, 1, 'delivered initial message';
+
+$exp = now + 10;
+PublicInbox::DS::add_timer 10, \&PublicInbox::Config::noop;
+local @PublicInbox::DS::post_loop_do = (sub { $delivered < 3 || now > $exp });
+my $tmpmsg = "$tmpdir/md2/tmp/conflict:2,S";
+write_file '>', $tmpmsg, $msg, "hi\n";
+link $tmpmsg, "$tmpdir/md1/cur/conflict:2,S";
+rename $tmpmsg, "$tmpdir/md2/cur/conflict:2,S";
+PublicInbox::DS::event_loop;
+is $delivered, 3, 'delivered new conflicting message to v2 and new to v1';
+
+my $v1ibx = $cfg->lookup('v1@example.com');
+my $v1msgs = $v1ibx->over->recent;
+is scalar(@$v1msgs), 1, 'only one message in v1';
+my $v1eml = PublicInbox::Eml->new($v1ibx->git->cat_file($v1msgs->[0]->{blob}));
+my @mid = $v1eml->header_raw('Message-ID');
+is_deeply \@mid, [ qw[<a@example>] ], 'only one Message-ID in v1 message';
+
+my $v2msgs = $cfg->lookup('v2@example.com')->over->recent;
+is scalar(@$v2msgs), 2, 'both messages in v2';
+
+done_testing;