-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix mail parsing in collector #7652
Fix mail parsing in collector #7652
Conversation
cedric-anne
commented
Jul 10, 2020
•
edited
Loading
edited
Q | A |
---|---|
Bug fix? | yes/no |
New feature? | yes/no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #7643 , #7649 |
- Prevent exception on addresses parsing; fixes [9.5] New IMAP Library Does Not Handle No Reply-To in Email Headers #7643
- Correctly fetch additionnal headers that may be used in rules
- Fix exclusion of mailer-daemon/postmaster
- Prevent exception when "Content-Transfer-Encoding" is not set; fixes Email import fails without image in body, succeeds with image in body but email body not added to ticket description #7649
cea7c1e
to
9b6c501
Compare
- Prevent exception on addresses parsing; fixes glpi-project#7643 - Correctly fetch additionnal headers that may be used in rules - Fix exclusion of mailer-daemon/postmaster - Prevent exception when "Content-Transfer-Encoding" is not set; fixes glpi-project#7649
9b6c501
to
b280b4e
Compare
foreach ($headers as $header) { | ||
// is line with additional header? | ||
$key = $header->getFieldName(); | ||
$value = $header->getFieldValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$key
was a numeric key here, so following preg_match()
calls were always returning false.
$req_field = $this->getRequesterField(); | ||
$h_requester = $message->getHeader($req_field)->getAddressList(); | ||
$requester = $h_requester->current()->getEmail(); | ||
$requester = $this->getRequesterEmail($message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reply-to
was always defined when using the imap extension (see #4554 (comment)), but now this header is not available if not set in the message.
$h_reply_to = $message->getHeader('reply_to')->getAddressList(); | ||
$reply_to = $h_reply_to->current(); | ||
$reply_to_addr = Toolbox::strtolower($reply_to->getEmail()); | ||
if (preg_match('/^(mailer-daemon|postmaster)@/i', $sender_email) === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With imap extension, we were comparing $sender->mailbox
, which was not containing the host (i.e. postmaster@domain.org
was returning postmater
).
Now, we have to deal with the full email address.
@@ -1996,10 +2014,9 @@ public function getDecodedContent(\Laminas\Mail\Storage\Part $part) { | |||
case '7bit': | |||
case '8bit': | |||
case 'binary': | |||
default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In GLPI 9.4, we were returning verbatim content if no transfer encoding was defined. I put back this behaviour as it seems that emails sent by Outlook are not containing this header for text parts (see attached email from #7649).
Hi @cedric-anne sorry, but I tried this fix (inc/mailcollector.class.php) and I still have Uncaught Exception Laminas\Mail\Storage\Exception\InvalidArgumentException: Header with Name to or to not found in /var/www/html/glpi/vendor/laminas/laminas-mail/src/Storage/Part.php at line 307. Am I missing some step? I am using Office365, IMAP, SSL, NOTLS (I dont understand why NOTLS because MS says it is IMAPS TLS), and NO VALIDATE CERT. After I did upload this mailcollector... I did my connector sync ONLY ONE TIME. After that... just this error. |
This one should be fixed by #7694 |
100% worked! Thanks! |