From b280b4eaee2f257a6b2fd67e0e297047c05a45dd 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 parsing in collector - Prevent exception on addresses 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 ++++ .../10-missing-content-transfer-encoding.eml | 85 +++++++++ tests/imap/MailCollector.php | 18 +- 4 files changed, 225 insertions(+), 83 deletions(-) create mode 100644 tests/emails-tests/09-reply-to-tech.eml create mode 100644 tests/emails-tests/10-missing-content-transfer-encoding.eml diff --git a/inc/mailcollector.class.php b/inc/mailcollector.class.php index eeeccdcd3ac..a517158bd93 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 00000000000..e82112cb4b3 --- /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/emails-tests/10-missing-content-transfer-encoding.eml b/tests/emails-tests/10-missing-content-transfer-encoding.eml new file mode 100644 index 00000000000..e40a006301c --- /dev/null +++ b/tests/emails-tests/10-missing-content-transfer-encoding.eml @@ -0,0 +1,85 @@ +From: Normal User +To: GLPI debug +Subject: Test Email from Outlook +Thread-Topic: Test Email from Outlook +Thread-Index: AdZW2I0jVbjPDha9QGyhAGwBB3ep4A== +Date: Fri, 10 Jul 2020 16:47:41 +0000 +Message-ID: + +Content-Language: en-US +X-MS-Has-Attach: +X-MS-Exchange-Organization-SCL: -1 +X-MS-TNEF-Correlator: +X-MS-Exchange-Organization-RecordReviewCfmType: 0 +Content-Type: multipart/alternative; + boundary="_000_DM6PR05MB5865518283B3393AA31FAEDFBC650DM6PR05MB5865namp_" +MIME-Version: 1.0 + +--_000_DM6PR05MB5865518283B3393AA31FAEDFBC650DM6PR05MB5865namp_ +Content-Type: text/plain; charset="us-ascii" + +With only text in the body, the email will not be imported and the following error will appear when forcing the retrieval by clicking the 'Get email tickets now' button on the receiver. + +'Uncaught Exception UnexpectedValueException: is not known in /var/www/glpi/inc/mailcollector.class.php at line 2002' + +--_000_DM6PR05MB5865518283B3393AA31FAEDFBC650DM6PR05MB5865namp_ +Content-Type: text/html; charset="us-ascii" + + + + + + + + +
+

With only text in the body, the email will not be imported and the following error will appear when forcing the retrieval by clicking the ‘Get email tickets now’ button on the receiver.

+

 

+

'Uncaught Exception UnexpectedValueException: is not known in /var/www/glpi/inc/mailcollector.class.php at line 2002'

+
+ + + +--_000_DM6PR05MB5865518283B3393AA31FAEDFBC650DM6PR05MB5865namp_-- diff --git a/tests/imap/MailCollector.php b/tests/imap/MailCollector.php index 1dd8f83767a..c52d3456893 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); @@ -255,8 +256,9 @@ public function testCollect() { //$this->collector = $collector; $this->doConnect(); + $this->collector->maxfetch_emails = 1000; // Be sure to fetch all mails from test suite $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=11, retrieved=11, refused=2, errors=1, blacklisted=0'); $rejecteds = iterator_to_array($DB->request(['FROM' => \NotImportedEmail::getTable()])); $this->array($rejecteds)->hasSize(2); @@ -266,6 +268,7 @@ public function testCollect() { ->variable['reason']->isEqualTo(\NotImportedEmail::USER_UNKNOWN); } + // Check mails having "tech" user as requester $iterator = $DB->request([ 'SELECT' => ['t.id', 't.name', 'tu.users_id'], 'FROM' => \Ticket::getTable() . " AS t", @@ -283,7 +286,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,10 +295,12 @@ 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); + // Check mails having "normal" user as requester $iterator = $DB->request([ 'SELECT' => ['t.id', 't.name', 'tu.users_id'], 'FROM' => \Ticket::getTable() . " AS t", @@ -313,7 +318,7 @@ public function testCollect() { ] ]); - $this->integer(count($iterator))->isIdenticalTo(3); + $this->integer(count($iterator))->isIdenticalTo(4); $names = []; while ($data = $iterator->next()) { $names[] = $data['name']; @@ -322,7 +327,8 @@ public function testCollect() { $expected_names = [ 'Test import mail avec emoticons unicode', 'Test images', - 'Test\'ed issue' + 'Test\'ed issue', + 'Test Email from Outlook', ]; $this->array($names)->isIdenticalTo($expected_names);