From 9d094c641a9c26e1b74094cc30ecad6c3a9d027b Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Sat, 25 Jan 2025 17:52:47 +0000 Subject: [PATCH 1/7] Bug 1837680: Add Message-ID header when missing at send Detect whether a Message-ID header has been included in the message passed to MessageToMTA and generate one if it's missing. GMail and other modern mail services now expect a Message-ID as part of the message else they mark them as spam or drop them entirely. --- Bugzilla/Mailer.pm | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index b778e431f..3943dc899 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -98,6 +98,11 @@ sub MessageToMTA { $email->header_set('MIME-Version', '1.0') if !$email->header('MIME-Version'); + # We ensure there's a Message-ID header set otherwise some mailsystems + # treat us as spam. + $email->header_set('Message-ID', build_message_id()) + if !$email->header('Message-ID'); + # Encode the headers correctly in quoted-printable foreach my $header ($email->header_names) { $header = lc $header; @@ -270,4 +275,23 @@ sub build_thread_marker { return $threadingmarker; } +# Builds Message-ID header +sub build_message_id { + my ($user_id) = @_; + + if (!defined $user_id) { + $user_id = Bugzilla->user->id; + } + + 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 '@' + } + + my $rand_bits = generate_random_password(10); + my $message_id = ""; + return $message_id; +} 1; From 616ec924ec076dda3226ea8ad839bb65d8fa4232 Mon Sep 17 00:00:00 2001 From: Kan-Ru Chen Date: Wed, 18 Mar 2026 21:19:15 +0900 Subject: [PATCH 2/7] Bug 1837680: use new urlbase reference location Bugzilla->params->{'urlbase'} has been moved to Bugzilla->localconfig->urlbase Signed-off-by: Kan-Ru Chen --- Bugzilla/Mailer.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index 3943dc899..e0e0f49ad 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -283,7 +283,7 @@ sub build_message_id { $user_id = Bugzilla->user->id; } - my $sitespec = '@' . Bugzilla->params->{'urlbase'}; + my $sitespec = '@' . Bugzilla->localconfig->urlbase; $sitespec =~ s/:\/\//\./; # Make the protocol look like part of the domain $sitespec =~ s/^([^:\/]+):(\d+)/$1/; # Remove a port number, to relocate if ($2) { From 7884003b6eb51a54482176a2d98dbe32517e1931 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 22 Jun 2026 22:17:51 +0100 Subject: [PATCH 3/7] Bug 1837680: Fix Message-ID sitespec sanitization and user fallback - Strip path components from urlbase when building sitespec, as '/' is illegal after '@' in a Message-ID (affects both build_thread_marker and build_message_id) - Fix stale $2 capture variable risk by conditioning port relocation on the substitution succeeding, in both functions - Avoid falling back to Bugzilla->user->id in build_message_id, which is called from contexts with no logged-in user; random bits ensure uniqueness without a user identifier --- Bugzilla/Mailer.pm | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index e0e0f49ad..7d2e4bbb3 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -257,9 +257,9 @@ sub build_thread_marker { my $sitespec = '@' . Bugzilla->localconfig->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/\/.*$//; # Strip path component — / is illegal after @ in Message-ID + if ($sitespec =~ s/^([^:\/]+):(\d+)/$1/) { # Remove port number, to relocate + $sitespec = "-$2$sitespec"; # Put the port number back in, before the '@' } my $threadingmarker = "References: "; @@ -279,15 +279,15 @@ sub build_thread_marker { sub build_message_id { my ($user_id) = @_; - if (!defined $user_id) { - $user_id = Bugzilla->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->localconfig->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/:\/\//\./; # Make the protocol look like part of the domain + $sitespec =~ s/\/.*$//; # Strip path component — / is illegal after @ in Message-ID + 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); From 95c9ce223256d48af307b2e23f707d31c5c10314 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 22 Jun 2026 22:26:20 +0100 Subject: [PATCH 4/7] Bug 1837680: Fix trailing whitespace and Message-ID double-dash - Remove trailing whitespace from the spam-guard comment and blank line - Omit the user_id segment entirely when absent rather than producing a double-dash (bugzilla--rand@domain), giving bugzilla-rand@domain --- Bugzilla/Mailer.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index 7d2e4bbb3..41773fb3e 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -99,10 +99,10 @@ sub MessageToMTA { if !$email->header('MIME-Version'); # We ensure there's a Message-ID header set otherwise some mailsystems - # treat us as spam. + # treat us as spam. $email->header_set('Message-ID', build_message_id()) if !$email->header('Message-ID'); - + # Encode the headers correctly in quoted-printable foreach my $header ($email->header_names) { $header = lc $header; @@ -291,7 +291,7 @@ sub build_message_id { } my $rand_bits = generate_random_password(10); - my $message_id = ""; + my $message_id = '"; return $message_id; } 1; From 8189c2985e26e8b2dd45e30d5d0500efcef619c3 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 22 Jun 2026 22:38:13 +0100 Subject: [PATCH 5/7] Bug 1837680: Add unit tests for build_message_id and build_thread_marker Test the sitespec transformation for four urlbase forms (plain, with path, with port, with port+path) to guard against regressions in the '/' stripping and port relocation logic. Also covers user_id handling in build_message_id (present, absent, no double-dash). --- t/014mailer.t | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 t/014mailer.t diff --git a/t/014mailer.t b/t/014mailer.t new file mode 100644 index 000000000..a3f9dd1d1 --- /dev/null +++ b/t/014mailer.t @@ -0,0 +1,113 @@ +# 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.10.1; +use strict; +use warnings; + +use lib qw(. lib local/lib/perl5 t); +use Support::Files; +use Test::More tests => 18; + +BEGIN { + use_ok('Bugzilla'); + use_ok('Bugzilla::Mailer', 'build_thread_marker'); +} + +# Inject a stub localconfig into the process cache so the sitespec +# transformation functions can be tested without a real installation. +{ + + package Bugzilla::Test::FakeLocalconfig; + + sub new { my ($class, $urlbase) = @_; bless {urlbase => $urlbase}, $class } + sub urlbase { $_[0]->{urlbase} } +} + +sub set_urlbase { + my ($urlbase) = @_; + Bugzilla->process_cache->{localconfig} + = Bugzilla::Test::FakeLocalconfig->new($urlbase); +} + +# ---- build_message_id: sitespec transformation ---- +# +# The central invariant: whatever urlbase looks like, the resulting +# Message-ID must be a valid with no '/' characters +# after the '@'. + +my $mid; + +# Plain http, trailing slash only — baseline case. +set_urlbase('http://bugs.example.org/'); +$mid = Bugzilla::Mailer::build_message_id(); +like($mid, qr/^$/, + 'simple http urlbase: correct format and sitespec'); +unlike($mid, qr/\@[^>]*\//, 'simple http urlbase: no slash after @'); + +# Path component after the hostname — the key regression this patch fixes. +set_urlbase('https://bugs.example.org/bugzilla/'); +$mid = Bugzilla::Mailer::build_message_id(); +like($mid, qr/^$/, + 'https with path: path component stripped from sitespec'); +unlike($mid, qr/\@[^>]*\//, 'https with path: no slash after @'); + +# Non-standard port — port must move before the '@'. +set_urlbase('https://bugs.example.org:8080/'); +$mid = Bugzilla::Mailer::build_message_id(); +like($mid, qr/^$/, + 'https with port: port relocated before @'); +unlike($mid, qr/\@[^>]*\//, 'https with port: no slash after @'); + +# Port and path together. +set_urlbase('https://bugs.example.org:8080/bugzilla/'); +$mid = Bugzilla::Mailer::build_message_id(); +like($mid, qr/^$/, + 'https with port and path: path stripped, port relocated'); +unlike($mid, qr/\@[^>]*\//, 'https with port and path: no slash after @'); + +# ---- build_message_id: user_id handling ---- + +set_urlbase('https://bugs.example.org/'); + +my $mid_with_user = Bugzilla::Mailer::build_message_id(42); +like($mid_with_user, qr/^/, + 'new thread with path: path stripped from sitespec'); +unlike($marker, qr/\@[^>]*\//, 'new thread with path: no slash after @'); + +# Port relocation for non-standard port. +set_urlbase('https://bugs.example.org:8080/'); +$marker = build_thread_marker(99, 7, 1); # new bug +like($marker, qr/Message-ID: /, + '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: /, + 'reply thread: In-Reply-To uses correct sitespec'); From ba6bb1ce8169b5ee140f7ba758976b7df94cb4ef Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 10 Jul 2026 16:07:02 +0100 Subject: [PATCH 6/7] Bug 1837680: Relocate urlbase path component instead of discarding it Stripping the path from urlbase loses information: two independent Bugzilla installs hosted in different subdirectories of the same domain (e.g. /team-a/ and /team-b/) would produce identical sitespecs, letting mail clients that group by References/In-Reply-To/Message-ID cross-thread unrelated bugs from separate installations. Relocate the path in front of the domain instead, the same way the port number is already relocated, sanitizing any internal '/' since it's illegal in a domain. Updates build_thread_marker and build_message_id identically, plus the corresponding tests. --- Bugzilla/Mailer.pm | 22 ++++++++++++++++++++-- t/014mailer.t | 36 +++++++++++++++++++++++------------- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index 41773fb3e..c0669ebe8 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -257,7 +257,16 @@ sub build_thread_marker { my $sitespec = '@' . Bugzilla->localconfig->urlbase; $sitespec =~ s/:\/\//\./; # Make the protocol look like part of the domain - $sitespec =~ s/\/.*$//; # Strip path component — / is illegal after @ in Message-ID + $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 '@' } @@ -285,7 +294,16 @@ sub build_message_id { my $sitespec = '@' . Bugzilla->localconfig->urlbase; $sitespec =~ s/:\/\//\./; # Make the protocol look like part of the domain - $sitespec =~ s/\/.*$//; # Strip path component — / is illegal after @ in Message-ID + $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 '@' } diff --git a/t/014mailer.t b/t/014mailer.t index a3f9dd1d1..63677600d 100644 --- a/t/014mailer.t +++ b/t/014mailer.t @@ -15,7 +15,7 @@ use warnings; use lib qw(. lib local/lib/perl5 t); use Support::Files; -use Test::More tests => 18; +use Test::More tests => 20; BEGIN { use_ok('Bugzilla'); @@ -53,11 +53,13 @@ like($mid, qr/^$/, 'simple http urlbase: correct format and sitespec'); unlike($mid, qr/\@[^>]*\//, 'simple http urlbase: no slash after @'); -# Path component after the hostname — the key regression this patch fixes. -set_urlbase('https://bugs.example.org/bugzilla/'); +# 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. +set_urlbase('https://bugs.example.org/subdir/'); $mid = Bugzilla::Mailer::build_message_id(); -like($mid, qr/^$/, - 'https with path: path component stripped from sitespec'); +like($mid, qr/^$/, + '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 '@'. @@ -67,13 +69,21 @@ like($mid, qr/^$/, 'https with port: port relocated before @'); unlike($mid, qr/\@[^>]*\//, 'https with port: no slash after @'); -# Port and path together. -set_urlbase('https://bugs.example.org:8080/bugzilla/'); +# Port and path together — port then path, domain always last. +set_urlbase('https://bugs.example.org:8080/subdir/'); $mid = Bugzilla::Mailer::build_message_id(); -like($mid, qr/^$/, - 'https with port and path: path stripped, port relocated'); +like($mid, + qr/^$/, + '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. +set_urlbase('https://bugs.example.org/foo/bar/'); +$mid = Bugzilla::Mailer::build_message_id(); +like($mid, qr/^$/, + 'nested path: internal slashes sanitized'); +unlike($mid, qr/\//, 'nested path: no slash anywhere in Message-ID'); + # ---- build_message_id: user_id handling ---- set_urlbase('https://bugs.example.org/'); @@ -94,11 +104,11 @@ unlike($mid_no_user, qr/bugzilla--/, my $marker; -# Path component must be stripped here too. -set_urlbase('https://bugs.example.org/bugzilla/'); +# Path component must be relocated here too, not stripped. +set_urlbase('https://bugs.example.org/subdir/'); $marker = build_thread_marker(99, 7, 1); # new bug -like($marker, qr/Message-ID: /, - 'new thread with path: path stripped from sitespec'); +like($marker, qr/Message-ID: /, + '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. From c7e26a6fa8ba71f0af788938ca557629fc64af14 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sat, 11 Jul 2026 01:44:30 -0400 Subject: [PATCH 7/7] use perl 5.14.0 --- t/014mailer.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/014mailer.t b/t/014mailer.t index 63677600d..1d67458dc 100644 --- a/t/014mailer.t +++ b/t/014mailer.t @@ -9,7 +9,7 @@ #Bugzilla Test 14# ####Mailer.pm##### -use 5.10.1; +use 5.14.0; use strict; use warnings;