Bug 1837680: Add Message-ID header when missing at send

[kanru]

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.

Details

Add a default Message-ID builder and add it to message header if there isn't already one. This change is based on #146

Additional info

• bug#1837680

Test Plan

not verified yet

1. Register a new account with an email address
2. Verify that the sign-up email has correct Message-ID header

[mrenvoize]

Is this not just a direct copy of #146

[justdave]

This appears to be an additional change on top of #146, and it's really hard to see because the branch contains a merge commit instead of containing the commit from the other PR and then the new one on top of it.

This PR applies the following change on top of #146:

diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm
index e0e0f49ad..3943dc899 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->localconfig->urlbase;
+  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) {

[justdave]

@kanru : I would like to merge this into the other PR (#146) so it can all go in together. I can't do it with the commit giving you credit without your help though, since you didn't check the box to let the maintainers commit to your branch. If you want credit for this, please do the following:

1. check out the PR branch from Bug 1837680: Add Message-ID header when missing at send #146 as follows:
2. git fetch upstream pull/146/head:pr146_bug_1837680 - replace upstream with whatever remote is set to pull from this repo.
3. git checkout pr146_bug_1837680 <- creates the local branch to hold it
4. git checkout bug-1837680 <- switches back to your branch
5. git rebase -i pr146_bug_1837680 <- this should give you an editor with instructions in it, follow the instructions as appropriate.
6. Because of the way your merge commit was done, you may have to do some manual conflict resolution. Your end goal is to get it to look like the diff I made here
7. once your rebase completes, force-push it back to this PR:
8. git push --force

Once that's all done, then we can cherry-pick that patch into the other PR and keep your attribution intact on it.

If that isn't done by the time the other PR is ready to land, we'll probably incorporate it manually, and hopefully I remember to mention you in the commit message. 😄

[kanru]

@justdave Thanks for the instructions! It looks like the diff was reversed. I changed it to match my original diff. Since you will merge #146, I only kept my change and updated the description.

[justdave]

Wait, so the original patch on #146 already had it in params and you're changing it to localconfig? Yep, does seem that way. My diff was backwards when I made it apparently. In that case, I don't think this change is needed at all. urlbase is in params and not in localconfig, so the original patch in #146 was correct.

[kanru]

grep Bugzilla->params->{'urlbase'} returned 0 matches but Bugzilla->localconfig->urlbase returned many matches though?

[justdave]

so it does... I didn't realize they moved that. Bugzilla/Config.pm has a shim to make sure the params reference still works though:

  $params{urlbase} = Bugzilla->localconfig->urlbase;

Probably to let old extensions still work.

That said, we should do this anyway.

[justdave]

OK, patch has been cherry-picked into the other PR, so GitHub will automatically include you in the Co-Author line on the final commit. Closing this one out!

Thanks for your help!