iTop icon indicating copy to clipboard operation
iTop copied to clipboard

:bug: N°7917 SF#2272 EmailLaminas.php: Fix Message-ID format

Open vlk-charles opened this issue 1 year ago • 2 comments

Base information

Question Answer
Related to a SourceForge thead / Another PR / Combodo ticket? SourceForge ticket 2272
Type of change? Bug fix

Symptom

iTop since version 3.1 (after switching to Laminas-mail, N°4307) sends emails with incorrectly formatted Message-ID. This sometimes results in Gmail classifying them as spam or even outright refusing to accept them for delivery. In another case, a client was reporting issues with their (possibly custom) system for automated email processing. More information is in the above-linked SourceForge issue.

Reproduction procedure

  1. On iTop 3.1.1-1
  2. With PHP 7.4.33
  3. Make iTop send you an email (assuming Gmail inbox).
  4. If the email arrives at all, examine its message ID ("Show original" in Gmail web application).
  5. See that the message ID got changed to <[email protected]> and an X-Google-Original-Message-ID header was added.

Cause

After switching to the Laminas-mail library, the addHeaderLine function is used to add the Message-ID header to the email that is about to be sent. This function can add arbitrary headers and uses Q-encoding to encode the entire value. This is not allowed for structured headers such as Message-ID according to the relevant RFCs (RFC 2047, RFC 2822).

Proposed solution

Laminas-mail provides a MessageId class. An object of this type is created and added using addHeader.

Checklist before requesting a review

  • [x] I have performed a self-review of my code
  • [x] I have tested all changes I made on an iTop instance
  • [ ] I have added a unit test, otherwise I have explained why I couldn't
  • [x] Is the PR clear and detailed enough so anyone can understand digging in the code?

Checklist of things to do before PR is ready to merge

vlk-charles avatar Oct 21 '24 22:10 vlk-charles

Kudos for the detailed PR 🙌

Molkobain avatar Oct 22 '24 10:10 Molkobain

Thanks for your contribution I've logged your bug in our internal bug tracker under ref N°7917

steffunky avatar Oct 23 '24 07:10 steffunky

Below is a comparison of the header between emails sent out from different versions of iTop.

iTop 3.0:

Message-ID: <iTop_UserRequest_112260_1721928197.377377@ffb5bc6b0d3ad4e791061d65c153a36b-production.openitop.org>

iTop 3.1.1-1:

Message-ID: =?UTF-8?Q?iTop=5FUserRequest=5F9142=5F1823562304.671192@a968b15fb014ecc1b0fa085ab5c5c2f6-production.openitop.org?=

vlk-charles avatar Oct 24 '24 15:10 vlk-charles

I just tested it, and this indeed fixes the Message-Id.

support/3.1 before this PR

Message-ID: =?UTF-8?Q?iTop=5FUserRequest=5F288=5F1729847352.614665@ca38c053d295a42819b13ca584962411-production.openitop.org?=

support/3.1 after this PR

Message-ID: <iTop_UserRequest_288_1729846346.464874@ca38c053d295a42819b13ca584962411-production.openitop.org>

I noticed In-Reply-To is also messed up but I'll try to fix it in a different PR as it's more likely the data's fault.

steffunky avatar Oct 25 '24 09:10 steffunky

Thanks for your PR @vlk-charles!! Could you send me your full name and postal address to stephen.abello at combodo.com ? We would like to send you some of our contributor stickers :raised_hands:

steffunky avatar Nov 04 '24 13:11 steffunky

@steffunky Thank you for the stickers. Address sent.

Will the fix be forward-ported to 3.2 and future versions?

vlk-charles avatar Nov 04 '24 18:11 vlk-charles

Sure thing, this has been merged under 2519456 for 3.2.1 and under d0f9e57 for 3.3.0

steffunky avatar Nov 05 '24 07:11 steffunky