From 05a478536ac00be277ef6b625adb5ba7aea1e83f 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 --- inc/mailcollector.class.php | 168 ++++++++++++++------------ tests/emails-tests/09-no-reply-to.eml | 35 ++++++ tests/imap/MailCollector.php | 119 +++++++++++------- 3 files changed, 202 insertions(+), 120 deletions(-) create mode 100644 tests/emails-tests/09-no-reply-to.eml diff --git a/inc/mailcollector.class.php b/inc/mailcollector.class.php index eeeccdcd3ac8..b66e46990ee7 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; } diff --git a/tests/emails-tests/09-no-reply-to.eml b/tests/emails-tests/09-no-reply-to.eml new file mode 100644 index 000000000000..a51c297043fc --- /dev/null +++ b/tests/emails-tests/09-no-reply-to.eml @@ -0,0 +1,35 @@ +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 +References: +In-Reply-To: +From: 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 no reply to header. + +--000000000000f50df905a9fce128 +Content-Type: text/html; charset="UTF-8" +Content-Transfer-Encoding: quoted-printable + +This message have no reply to header. + +--000000000000f50df905a9fce128-- diff --git a/tests/imap/MailCollector.php b/tests/imap/MailCollector.php index 1dd8f83767a8..aa7a7f79db18 100644 --- a/tests/imap/MailCollector.php +++ b/tests/imap/MailCollector.php @@ -188,7 +188,7 @@ public function testCounts() { $this->integer($this->testedInstance->countCollectors())->isIdenticalTo(1); } - private function doConnect() { + private function doConnect(array $custom_fields = []) { if (null === $this->collector) { $this->newTestedInstance(); $collector = $this->testedInstance; @@ -197,18 +197,22 @@ private function doConnect() { $collector = $this->collector; } - $this->mailgate_id = (int)$collector->add([ - 'name' => 'testuser', - 'login' => 'testuser', - 'is_active' => true, - 'passwd' => 'applesauce', - 'mail_server' => '127.0.0.1', - 'server_type' => '/imap', - 'server_port' => 143, - 'server_ssl' => '', - 'add_cc_to_observer' => 1, //add ccuser as observer in ticket - 'server_cert' => '/novalidate-cert' - ]); + $this->mailgate_id = (int)$collector->add( + array_merge( + [ + 'name' => 'testuser', + 'login' => 'testuser', + 'is_active' => true, + 'passwd' => 'applesauce', + 'mail_server' => '127.0.0.1', + 'server_type' => '/imap', + 'server_port' => 143, + 'server_ssl' => '', + 'server_cert' => '/novalidate-cert' + ], + $custom_fields + ) + ); $this->integer($this->mailgate_id)->isGreaterThan(0); @@ -218,7 +222,28 @@ private function doConnect() { $this->variable($collector->fields['errors'])->isEqualTo(0); } - public function testCollect() { + + /** + * @see self::testCollect() + */ + protected function collectProvider() { + return [ + [ + 'collector_fields' => [], + ], + [ + 'collector_fields' => ['requester_field' => \MailCollector::REQUESTER_FIELD_REPLY_TO], + ], + [ + 'collector_fields' => ['add_cc_to_observer' => 1], + ], + ]; + } + + /** + * @dataProvider collectProvider + */ + public function testCollect(array $collector_fields) { global $DB; //assign email to user @@ -254,9 +279,10 @@ public function testCollect() { //$this->calling($collector)->getReplyMatch = "/GLPI-Ticket-(1155)/"; //$this->collector = $collector; - $this->doConnect(); + $this->doConnect($collector_fields); + $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 +309,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 +318,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); @@ -326,37 +353,39 @@ public function testCollect() { ]; $this->array($names)->isIdenticalTo($expected_names); - //load ticket with observer for user normal - //see function doConnect - //wich allow to add cc as observer (add_cc_to_observer = true) - $iterator = $DB->request([ - 'SELECT' => ['t.id', 't.name', 'tu.users_id'], - 'FROM' => \Ticket::getTable() . " AS t", - 'INNER JOIN' => [ - \Ticket_User::getTable() . " AS tu" => [ - 'ON' => [ - 't' => 'id', - 'tu' => 'tickets_id' + if ($this->collector->fields['add_cc_to_observer']) { + //load ticket with observer for user normal + //see function doConnect + //wich allow to add cc as observer (add_cc_to_observer = true) + $iterator = $DB->request([ + 'SELECT' => ['t.id', 't.name', 'tu.users_id'], + 'FROM' => \Ticket::getTable() . " AS t", + 'INNER JOIN' => [ + \Ticket_User::getTable() . " AS tu" => [ + 'ON' => [ + 't' => 'id', + 'tu' => 'tickets_id' + ] ] + ], + 'WHERE' => [ + 'tu.users_id' => $nuid, + 'tu.type' => \CommonITILActor::OBSERVER ] - ], - 'WHERE' => [ - 'tu.users_id' => $nuid, - 'tu.type' => \CommonITILActor::OBSERVER - ] - ]); - - $this->integer(count($iterator))->isIdenticalTo(1); - $names = []; - while ($data = $iterator->next()) { - $names[] = $data['name']; + ]); + + $this->integer(count($iterator))->isIdenticalTo(1); + $names = []; + while ($data = $iterator->next()) { + $names[] = $data['name']; + } + + $expected_names = [ + 'Ticket with observer', + ]; + $this->array($names)->isIdenticalTo($expected_names); } - $expected_names = [ - 'Ticket with observer', - ]; - $this->array($names)->isIdenticalTo($expected_names); - /* FUPs * A followup should have een created from mail 04 * but I've not been able to setup tests correctly on this point.