From 831e1a6f15727f42c3ce4cf791d0efccee7535bb Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Sat, 25 Jan 2025 17:52:47 +0000 Subject: [PATCH 1/5] 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 924f88a9b9166808ea153a0e3414870e62f6e61e Mon Sep 17 00:00:00 2001 From: Kan-Ru Chen Date: Wed, 18 Mar 2026 21:19:15 +0900 Subject: [PATCH 2/5] 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 6380e217ad62cb65ff80c704bd632a49494a0148 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 22 Jun 2026 22:17:51 +0100 Subject: [PATCH 3/5] 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 0d384910df4f7a97eb94f26c8f22754a160b4d00 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 22 Jun 2026 22:26:20 +0100 Subject: [PATCH 4/5] 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 29db59188d1267d3562f7dd6d95c30d6cf286ab6 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 22 Jun 2026 22:38:13 +0100 Subject: [PATCH 5/5] 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');