600230 - All emails should have a unique Message-ID

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, enhancement)

Description

[Frédéric Buclin]

Per RFC 2822 section 3.6.4:

"Though optional, every message SHOULD have a "Message-ID:" field. [...] The "Message-ID:" field contains a single unique message identifier."

This is currently a problem for GCC mailing-lists (which are global watchers), because all "In-Reply-To:" and "References:" fields only contain the ID of the original parent bugmail, and not the ID of the email to which you replied (they use email_in.pl to comment a lot). This breaks some of their external tools to list threads correctly.

Comment 1

[Reed Loden [:reed]]

Attachment: patch - v1 (untested)

Completely untested patch that I'm just looking for feedback on for how I'm creating the message ID.

Comment 2

[Reed Loden [:reed]]

this doesn't correct the entire problem... just gives each mail a unique message id while keeping in-reply-to and references as pointing to the original mail.

Comment 3

[Max Kanat-Alexander]

(In reply to comment #2)
> this doesn't correct the entire problem... just gives each mail a unique
> message id while keeping in-reply-to and references as pointing to the original
> mail.

  That's fine--LpSolit's point is that there is no Message-ID header in the follow-ups, so if people reply by email to the list, their replies don't thread. (At least, that's what I understand LpSolit's point to be.)

Comment 4

[Frédéric Buclin]

(In reply to comment #3)
> follow-ups, so if people reply by email to the list, their replies don't
> thread. (At least, that's what I understand LpSolit's point to be.)

More exactly, this prevents one of their external tool to identify the emails correctly, because they don't have a unique identifier, see the GCC bug in the See Also field.

Comment 5

[Frédéric Buclin]

In comment 1, you say you just want some feedback. I guess you got it. Are you going to attach an updated patch, or is v1 the one I should review? It would be cool if you could test it before I review it, though.

Comment 6

[Frédéric Buclin]

Comment on attachment 479568 [details] [diff] [review]
patch - v1 (untested)

>=== modified file 'Bugzilla/Mailer.pm'

>+        my $epoch_time = datetime_from($delta_ts)->epoch();

Thinking about it more, calling datetime_from() is not OK. This is going to create one DateTime() object per bugmail, which is slow. As you generate a random 6 characters long string, I think this is enough. It's very unlikely to get twice the same random string in a single bug.

Comment 7

[Reed Loden [:reed]]

Attachment: patch - v2

Ok, let's just add the random bits, but use 10 instead of 6.

Comment 8

[Frédéric Buclin]

Comment on attachment 484820 [details] [diff] [review]
patch - v2

Looks good. r=LpSolit

Comment 9

[Frédéric Buclin]

mkanat: would you object to take it for 4.0?

Comment 10

[Reed Loden [:reed]]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Mailer.pm
Committed revision 7569.

Comment 11

[Max Kanat-Alexander]

I would not object at all, sounds like a good fix.

Comment 12

[Reed Loden [:reed]]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Mailer.pm
Committed revision 7460.