From cea7c1e291ed6dc053563d5278effbc5b1347462 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= Date: Fri, 10 Jul 2020 11:54:43 +0200 Subject: [PATCH] Fix mail headers parsing in collector - Prevent exception on adresses parsing; fixes #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 #7649 --- inc/mailcollector.class.php | 171 +++++++++++++----------- tests/emails-tests/09-reply-to-tech.eml | 34 +++++ tests/imap/MailCollector.php | 10 +- 3 files changed, 134 insertions(+), 81 deletions(-) create mode 100644 tests/emails-tests/09-reply-to-tech.eml diff --git a/inc/mailcollector.class.php b/inc/mailcollector.class.php index eeeccdcd3ac8..a517158bd933 100644 --- a/inc/mailcollector.class.php +++ b/inc/mailcollector.class.php @@ -35,6 +35,9 @@ } use LitEmoji\LitEmoji; +use Laminas\Mail\Address; +use Laminas\Mail\Header\AbstractAddressList; +use Laminas\Mail\Storage\Message; use Laminas\Mime\Mime as Laminas_Mime; /** @@ -717,9 +720,7 @@ function collect($mailgateID, $display = 0) { $rejinput = []; $rejinput['mailcollectors_id'] = $mailgateID; - $req_field = $this->getRequesterField(); - $h_requester = $message->getHeader($req_field)->getAddressList(); - $requester = $h_requester->current()->getEmail(); + $requester = $this->getRequesterEmail($message); if (!$tkt['_blacklisted']) { global $DB; @@ -951,9 +952,7 @@ function buildTicket($uid, \Laminas\Mail\Storage\Message $message, $options = [] } // Who is the user ? - $req_field = $this->getRequesterField(); - $h_requester = $message->getHeader($req_field)->getAddressList(); - $requester = $h_requester->current()->getEmail(); + $requester = $this->getRequesterEmail($message); $tkt['_users_id_requester'] = User::getOrImportByEmail($requester); $tkt["_users_id_requester_notif"]['use_notification'][0] = 1; @@ -1320,8 +1319,10 @@ function getAdditionnalHeaders(\Laminas\Mail\Storage\Message $message) { $head = []; $headers = $message->getHeaders(); - foreach ($headers as $key => $value) { + foreach ($headers as $header) { // is line with additional header? + $key = $header->getFieldName(); + $value = $header->getFieldValue(); if (preg_match("/^X-/i", $key) || preg_match("/^Auto-Submitted/i", $key) || preg_match("/^Received/i", $key)) { @@ -1354,79 +1355,71 @@ function getAdditionnalHeaders(\Laminas\Mail\Storage\Message $message) { **/ function getHeaders(\Laminas\Mail\Storage\Message $message) { - $h_sender = $message->getHeader('from')->getAddressList(); - $sender = $h_sender->current(); + $sender_email = $this->getEmailFromHeader($message, 'from'); - $h_to = $message->getHeader('to')->getAddressList(); - $h_to->rewind(); - $to = $h_to->current(); - - $reply_to_addr = null; - if (isset($message->reply_to)) { - $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) { + return []; } + $to = $this->getEmailFromHeader($message, 'to'); + + $reply_to_addr = Toolbox::strtolower($this->getEmailFromHeader($message, 'reply-to')); + $date = date("Y-m-d H:i:s", strtotime($message->date)); $mail_details = []; - if ((Toolbox::strtolower($sender->getEmail()) != 'mailer-daemon') - && (Toolbox::strtolower($sender->getEmail()) != 'postmaster')) { - - // Construct to and cc arrays - $h_tos = $message->getHeader('to'); - $tos = []; - foreach ($h_tos->getAddressList() as $address) { - $mailto = Toolbox::strtolower($address->getEmail()); - if ($mailto === $this->fields['name']) { - $to = $address; - } - $tos[] = $mailto; + // Construct to and cc arrays + $h_tos = $message->getHeader('to'); + $tos = []; + foreach ($h_tos->getAddressList() as $address) { + $mailto = Toolbox::strtolower($address->getEmail()); + if ($mailto === $this->fields['name']) { + $to = $mailto; } + $tos[] = $mailto; + } - $ccs = []; - if (isset($message->cc)) { - $h_ccs = $message->getHeader('cc'); - foreach ($h_ccs->getAddressList() as $address) { - $ccs[] = Toolbox::strtolower($address->getEmail()); - } + $ccs = []; + if (isset($message->cc)) { + $h_ccs = $message->getHeader('cc'); + foreach ($h_ccs->getAddressList() as $address) { + $ccs[] = Toolbox::strtolower($address->getEmail()); } + } - // secu on subject setting - try { - $subject = $message->getHeader('subject')->getFieldValue(); - } catch (Laminas\Mail\Storage\Exception\InvalidArgumentException $e) { - $subject = ''; - } + // secu on subject setting + try { + $subject = $message->getHeader('subject')->getFieldValue(); + } catch (Laminas\Mail\Storage\Exception\InvalidArgumentException $e) { + $subject = ''; + } + + $mail_details = [ + 'from' => Toolbox::strtolower($sender_email), + 'subject' => $subject, + 'reply-to' => $reply_to_addr, + 'to' => Toolbox::strtolower($to), + 'message_id' => $message->getHeader('message_id')->getFieldValue(), + 'tos' => $tos, + 'ccs' => $ccs, + 'date' => $date + ]; - $mail_details = [ - 'from' => Toolbox::strtolower($sender->getEmail()), - 'subject' => $subject, - 'reply-to' => $reply_to_addr, - 'to' => Toolbox::strtolower($to->getEmail()), - 'message_id' => $message->getHeader('message_id')->getFieldValue(), - 'tos' => $tos, - 'ccs' => $ccs, - 'date' => $date - ]; - - if (isset($message->references)) { - if ($reference = $message->getHeader('references')) { - $mail_details['references'] = $reference->getFieldValue(); - } + if (isset($message->references)) { + if ($reference = $message->getHeader('references')) { + $mail_details['references'] = $reference->getFieldValue(); } + } - if (isset($message->in_reply_to)) { - if ($inreplyto = $message->getHeader('in_reply_to')) { - $mail_details['in_reply_to'] = $inreplyto->getFieldValue(); - } + if (isset($message->in_reply_to)) { + if ($inreplyto = $message->getHeader('in_reply_to')) { + $mail_details['in_reply_to'] = $inreplyto->getFieldValue(); } + } - //Add additional headers in X- - foreach ($this->getAdditionnalHeaders($message) as $header => $value) { - $mail_details[$header] = $value; - } + //Add additional headers in X- + foreach ($this->getAdditionnalHeaders($message) as $header => $value) { + $mail_details[$header] = $value; } return $mail_details; @@ -1953,21 +1946,46 @@ function cleanDBonPurge() { Rule::cleanForItemCriteria($this, '_mailgate'); } + /** + * Get the requester email address. + * + * @param Message $message + * + * @return string|null + */ + private function getRequesterEmail(Message $message): ?string { + $email = null; + + if ($this->fields['requester_field'] === self::REQUESTER_FIELD_REPLY_TO) { + // Try to find requester in "reply-to" + $email = $this->getEmailFromHeader($message, 'reply-to'); + } + if ($email === null) { + // Fallback on default "from" + $email = $this->getEmailFromHeader($message, 'from'); + } + + return $email; + } /** - * Get the requester field + * Get the email address from given header. * - * @return string - **/ - private function getRequesterField() { - switch ($this->fields['requester_field']) { - case self::REQUESTER_FIELD_REPLY_TO: - return "reply-to"; - - default: - return "from"; + * @param Message $message + * @param string $header_name + * + * @return string|null + */ + private function getEmailFromHeader(Message $message, string $header_name): ?string { + if (!$message->getHeaders()->has($header_name)) { + return null; } + + $header = $message->getHeader($header_name); + $address = $header instanceof AbstractAddressList ? $header->getAddressList()->rewind() : null; + + return $address instanceof Address ? $address->getEmail() : null; } @@ -1996,10 +2014,9 @@ public function getDecodedContent(\Laminas\Mail\Storage\Part $part) { case '7bit': case '8bit': case 'binary': + default: // returned verbatim break; - default: - throw new \UnexpectedValueException("$encoding is not known"); } $contentType = $part->getHeader('contentType'); diff --git a/tests/emails-tests/09-reply-to-tech.eml b/tests/emails-tests/09-reply-to-tech.eml new file mode 100644 index 000000000000..e82112cb4b36 --- /dev/null +++ b/tests/emails-tests/09-reply-to-tech.eml @@ -0,0 +1,34 @@ +Delivered-To: REDACTED-EMAIL +Received: by 2002:a50:cfc3:0:0:0:0:0 with SMTP id i3csp1241712edk; + Thu, 9 Jul 2020 00:07:53 -0700 (PDT) +Return-Path: +Received: from REDACTED-SERVER (REDACTED-SERVER [REDACTED-IP]) + by mx.google.com with ESMTPS id ck17si1268608edb.508.2020.07.09.00.07.53 + for + (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); + Thu, 09 Jul 2020 00:07:53 -0700 (PDT) +Received: from mail-lf1-f69.google.com ([REDACTED-IP]) + by REDACTED-SERVER with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Jul 2020 08:07:53 +0100 +Received: by mail-lf1-f69.google.com with SMTP id f8so1572484lfh.22 + for ; Thu, 09 Jul 2020 00:07:52 -0700 (PDT) +MIME-Version: 1.0 +From: Tech (alt address) +Reply-To: Tech Ni Cian +Date: Thu, 9 Jul 2020 08:07:40 +0100 +Message-ID: +Subject: Re: [GLPI #0038927] Update - Issues with new Windows 10 machine +To: GLPI debug +Content-Type: multipart/alternative; boundary="000000000000f50df905a9fce128" + +--000000000000f50df905a9fce128 +Content-Type: text/plain; charset="UTF-8" + +This message have reply to header, requester should be get from this header. + +--000000000000f50df905a9fce128 +Content-Type: text/html; charset="UTF-8" +Content-Transfer-Encoding: quoted-printable + +This message have reply to header, requester should be get from this header. + +--000000000000f50df905a9fce128-- diff --git a/tests/imap/MailCollector.php b/tests/imap/MailCollector.php index 1dd8f83767a8..f6c4565df720 100644 --- a/tests/imap/MailCollector.php +++ b/tests/imap/MailCollector.php @@ -206,8 +206,9 @@ private function doConnect() { 'server_type' => '/imap', 'server_port' => 143, 'server_ssl' => '', + 'server_cert' => '/novalidate-cert', 'add_cc_to_observer' => 1, //add ccuser as observer in ticket - 'server_cert' => '/novalidate-cert' + 'requester_field' => \MailCollector::REQUESTER_FIELD_REPLY_TO, ]); $this->integer($this->mailgate_id)->isGreaterThan(0); @@ -256,7 +257,7 @@ public function testCollect() { $this->doConnect(); $msg = $this->collector->collect($this->mailgate_id); - $this->variable($msg)->isIdenticalTo('Number of messages: available=9, retrieved=9, refused=2, errors=1, blacklisted=0'); + $this->variable($msg)->isIdenticalTo('Number of messages: available=10, retrieved=10, refused=2, errors=1, blacklisted=0'); $rejecteds = iterator_to_array($DB->request(['FROM' => \NotImportedEmail::getTable()])); $this->array($rejecteds)->hasSize(2); @@ -283,7 +284,7 @@ public function testCollect() { ] ]); - $this->integer(count($iterator))->isIdenticalTo(3); + $this->integer(count($iterator))->isIdenticalTo(4); $names = []; while ($data = $iterator->next()) { $names[] = $data['name']; @@ -292,7 +293,8 @@ public function testCollect() { $expected_names = [ 'PHP fatal error', 'Re: [GLPI #0001155] New ticket database issue', - 'Ticket with observer' + 'Ticket with observer', + 'Re: [GLPI #0038927] Update - Issues with new Windows 10 machine', ]; $this->array($names)->isIdenticalTo($expected_names);