Bug 1837680: Add Message-ID header when missing at send (#205)

Detect whether a Message-ID header has been included in the message
passed to MessageToMTA and generate one if it's missing.

r=justdave

diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm
index 313cd574e6b96d369bfe41e52a93a667c19b4a59..90c08996f7b854143629baa8d785bc94b69a80f1 100644 (file)
--- a/Bugzilla/Mailer.pm
+++ b/Bugzilla/Mailer.pm
@@ -148,6 +148,10 @@ sub MessageToMTA {
 
   my $email = ref($msg) ? $msg : Bugzilla::MIME->new($msg);
 
+  # Ensure that the message contains a Message-ID header
+  my $message_id = $email->header('Message-ID');
+  $email->header_set('Message-ID', build_message_id()) if (!$message_id);
+
   # If we're called from within a transaction, we don't want to send the
   # email immediately, in case the transaction is rolled back. Instead we
   # insert it into the mail_staging table, and bz_commit_transaction calls
@@ -260,9 +264,18 @@ sub build_thread_marker {
 
   my $sitespec = '@' . Bugzilla->params->{'urlbase'};
   $sitespec =~ s/:\/\//\./;    # Make the protocol look like part of the domain
-  $sitespec =~ s/^([^:\/]+):(\d+)/$1/;    # Remove a port number, to relocate
-  if ($2) {
-    $sitespec = "-$2$sitespec";    # Put the port number back in, before the '@'
+  $sitespec =~ s/\/$//;        # Drop the lone trailing slash every urlbase has
+
+  # Multiple Bugzillas can be hosted in different subdirectories on the same
+  # domain, so relocate any path component in front of the domain (like the
+  # port below) rather than discarding it — '/' is illegal in a domain.
+  if ($sitespec =~ s/^(\@[^\/]+)\/(.+)$/$1/) {
+    (my $path = $2) =~ s/\//-/g;    # sanitize any remaining internal slashes
+    $sitespec = "-$path$sitespec";
+  }
+
+  if ($sitespec =~ s/^([^:\/]+):(\d+)/$1/) {    # Remove port number, to relocate
+    $sitespec = "-$2$sitespec";                  # Put the port number back in, before the '@'
   }
 
   my $threadingmarker;
@@ -280,6 +293,35 @@ sub build_thread_marker {
   return $threadingmarker;
 }
 
+# Builds Message-ID header
+sub build_message_id {
+  my ($user_id) = @_;
+
+  # Don't fall back to current user: this is called from contexts with no logged-in
+  # user (job queue, email_in.pl). The random bits below ensure uniqueness anyway.
+  $user_id //= '';
+
+  my $sitespec = '@' . Bugzilla->params->{'urlbase'};
+  $sitespec =~ s/:\/\//\./;    # Make the protocol look like part of the domain
+  $sitespec =~ s/\/$//;        # Drop the lone trailing slash every urlbase has
+
+  # Multiple Bugzillas can be hosted in different subdirectories on the same
+  # domain, so relocate any path component in front of the domain (like the
+  # port below) rather than discarding it — '/' is illegal in a domain.
+  if ($sitespec =~ s/^(\@[^\/]+)\/(.+)$/$1/) {
+    (my $path = $2) =~ s/\//-/g;    # sanitize any remaining internal slashes
+    $sitespec = "-$path$sitespec";
+  }
+
+  if ($sitespec =~ s/^([^:\/]+):(\d+)/$1/) {    # Remove port number, to relocate
+    $sitespec = "-$2$sitespec";                  # Put the port number back in, before the '@'
+  }
+
+  my $rand_bits  = generate_random_password(10);
+  my $message_id = '<bugzilla-' . ($user_id ne '' ? "$user_id-" : '') . "$rand_bits$sitespec>";
+  return $message_id;
+}
+
 sub send_staged_mail {
   my $dbh = Bugzilla->dbh;
 
@@ -324,6 +366,10 @@ the message is sent immediately.
 
 Builds header suitable for use as a threading marker in email notifications.
 
+=item C<build_message_id>
+
+Builds a unique message_id string suitable for use as in the Message-ID mail header.
+
 =item C<send_staged_mail>
 
 Sends all staged messages -- called after a database transaction is committed.

diff --git a/t/014mailer.t b/t/014mailer.t
new file mode 100644 (file)
index 0000000..68593b8
--- /dev/null
+++ b/t/014mailer.t
@@ -0,0 +1,110 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+##################
+#Bugzilla Test 14#
+####Mailer.pm#####
+
+use 5.14.0;
+use strict;
+use warnings;
+
+use lib qw(. lib t);
+use Support::Files;
+use Test::More tests => 20;
+
+BEGIN {
+  use_ok('Bugzilla');
+  use_ok('Bugzilla::Mailer', 'build_thread_marker');
+}
+
+# Inject urlbase into the params cache without needing a real installation.
+my $params = Bugzilla->params;
+
+# ---- build_message_id: sitespec transformation ----
+#
+# The central invariant: whatever urlbase looks like, the resulting
+# Message-ID must be a valid <local-part@domain> with no '/' characters
+# after the '@'.
+
+my $mid;
+
+# Plain http, trailing slash only — baseline case.
+$params->{urlbase} = 'http://bugs.example.org/';
+$mid = Bugzilla::Mailer::build_message_id();
+like($mid, qr/^<bugzilla-[A-Za-z0-9]+\@http\.bugs\.example\.org>$/,
+  'simple http urlbase: correct format and sitespec');
+unlike($mid, qr/\@[^>]*\//, 'simple http urlbase: no slash after @');
+
+# Path component after the hostname — must be relocated in front of the
+# domain (not discarded), so that different Bugzillas hosted in
+# subdirectories of the same domain don't collide on the same sitespec.
+$params->{urlbase} = 'https://bugs.example.org/subdir/';
+$mid = Bugzilla::Mailer::build_message_id();
+like($mid, qr/^<bugzilla-[A-Za-z0-9]+-subdir\@https\.bugs\.example\.org>$/,
+  'https with path: path component relocated in front of the domain');
+unlike($mid, qr/\@[^>]*\//, 'https with path: no slash after @');
+
+# Non-standard port — port must move before the '@'.
+$params->{urlbase} = 'https://bugs.example.org:8080/';
+$mid = Bugzilla::Mailer::build_message_id();
+like($mid, qr/^<bugzilla-[A-Za-z0-9]+-8080\@https\.bugs\.example\.org>$/,
+  'https with port: port relocated before @');
+unlike($mid, qr/\@[^>]*\//, 'https with port: no slash after @');
+
+# Port and path together — port then path, domain always last.
+$params->{urlbase} = 'https://bugs.example.org:8080/subdir/';
+$mid = Bugzilla::Mailer::build_message_id();
+like($mid,
+  qr/^<bugzilla-[A-Za-z0-9]+-8080-subdir\@https\.bugs\.example\.org>$/,
+  'https with port and path: both relocated, domain last');
+unlike($mid, qr/\@[^>]*\//, 'https with port and path: no slash after @');
+
+# Nested subdirectory — internal slashes must be sanitized too.
+$params->{urlbase} = 'https://bugs.example.org/foo/bar/';
+$mid = Bugzilla::Mailer::build_message_id();
+like($mid, qr/^<bugzilla-[A-Za-z0-9]+-foo-bar\@https\.bugs\.example\.org>$/,
+  'nested path: internal slashes sanitized');
+unlike($mid, qr/\//, 'nested path: no slash anywhere in Message-ID');
+
+# ---- build_message_id: user_id handling ----
+
+$params->{urlbase} = 'https://bugs.example.org/';
+
+my $mid_with_user = Bugzilla::Mailer::build_message_id(42);
+like($mid_with_user, qr/^<bugzilla-42-[A-Za-z0-9]+\@/,
+  'with user_id: user_id present in local-part');
+unlike($mid_with_user, qr/bugzilla--/,
+  'with user_id: no double-dash');
+
+my $mid_no_user = Bugzilla::Mailer::build_message_id();
+like($mid_no_user, qr/^<bugzilla-[A-Za-z0-9]+\@/,
+  'without user_id: clean local-part (no double-dash)');
+unlike($mid_no_user, qr/bugzilla--/,
+  'without user_id: no double-dash');
+
+# ---- build_thread_marker: same sitespec logic ----
+
+my $marker;
+
+# Path component must be relocated here too, not stripped.
+$params->{urlbase} = 'https://bugs.example.org/subdir/';
+$marker = build_thread_marker(99, 7, 1);    # new bug
+like($marker, qr/Message-ID: <bug-99-7-subdir\@https\.bugs\.example\.org>/,
+  'new thread with path: path relocated in front of the domain');
+unlike($marker, qr/\@[^>]*\//, 'new thread with path: no slash after @');
+
+# Port relocation for non-standard port.
+$params->{urlbase} = 'https://bugs.example.org:8080/';
+$marker = build_thread_marker(99, 7, 1);    # new bug
+like($marker, qr/Message-ID: <bug-99-7-8080\@https\.bugs\.example\.org>/,
+  'new thread with port: port relocated before @');
+
+# Reply includes In-Reply-To pointing at the root message.
+$marker = build_thread_marker(99, 7, 0);    # reply
+like($marker, qr/In-Reply-To: <bug-99-7-8080\@https\.bugs\.example\.org>/,
+  'reply thread: In-Reply-To uses correct sitespec');