Closed Bug 600230 Opened 14 years ago Closed 14 years ago

All emails should have a unique Message-ID

Categories

(Bugzilla :: Email Notifications, enhancement)

3.6.2
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: LpSolit, Assigned: reed)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch patch - v1 (untested) (obsolete) — Splinter Review
Completely untested patch that I'm just looking for feedback on for how I'm creating the message ID.
Assignee: email-notifications → reed
Status: NEW → ASSIGNED
Attachment #479568 - Flags: review?(LpSolit)
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.
(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.)
(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.
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 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.
Attachment #479568 - Flags: review?(LpSolit) → review-
Attached patch patch - v2Splinter Review
Ok, let's just add the random bits, but use 10 instead of 6.
Attachment #479568 - Attachment is obsolete: true
Attachment #484820 - Flags: review?(LpSolit)
Comment on attachment 484820 [details] [diff] [review]
patch - v2

Looks good. r=LpSolit
Attachment #484820 - Flags: review?(LpSolit) → review+
mkanat: would you object to take it for 4.0?
Flags: approval+
Target Milestone: --- → Bugzilla 4.2
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Mailer.pm
Committed revision 7569.
Flags: approval4.0?
I would not object at all, sounds like a good fix.
Flags: approval4.0? → approval4.0+
Target Milestone: Bugzilla 4.2 → Bugzilla 4.0
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Mailer.pm
Committed revision 7460.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: