diff --git a/.phpstan-baseline.php b/.phpstan-baseline.php index a0cc591a764..b55a0c1afe1 100644 --- a/.phpstan-baseline.php +++ b/.phpstan-baseline.php @@ -14671,12 +14671,6 @@ 'count' => 1, 'path' => __DIR__ . '/src/MailCollector.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Offset 1 might not exist on array\\{\\}\\|array\\{non\\-falsy\\-string, numeric\\-string\\}\\.$#', - 'identifier' => 'offsetAccess.notFound', - 'count' => 1, - 'path' => __DIR__ . '/src/MailCollector.php', -]; $ignoreErrors[] = [ 'message' => '#^Parameter \\#1 \\$datetime of function Safe\\\\strtotime expects string, array\\|ArrayIterator\\|Laminas\\\\Mail\\\\Header\\\\HeaderInterface\\|string given\\.$#', 'identifier' => 'argument.type', diff --git a/src/MailCollector.php b/src/MailCollector.php index d8c2fca631f..e8f5d6fa38d 100644 --- a/src/MailCollector.php +++ b/src/MailCollector.php @@ -57,6 +57,7 @@ use function Safe\iconv; use function Safe\mb_convert_encoding; use function Safe\preg_match; +use function Safe\preg_match_all; use function Safe\preg_replace; use function Safe\strtotime; @@ -77,6 +78,11 @@ class MailCollector extends CommonDBTM * IMAP / POP connection */ private ?AbstractStorage $storage = null; + /** + * Notification subject tags used across entities, memoized for the current run. + * @var ?string[] + */ + private ?array $notification_subject_tags = null; /** * UID of the current message * @var int @@ -2256,20 +2262,72 @@ public function getItemFromHeaders(Message $message): ?CommonDBTM // Check in subject if ($message->getHeaders()->has('subject')) { $subject = $message->getHeader('subject')->getFieldValue(); - $matches = []; $ticket = new Ticket(); - if ( - preg_match('/\[.+#(\d+)\]/', $subject, $matches) === 1 - && $ticket->getFromDB($matches[1]) - ) { - return $ticket; + + // GLPI prefixes ITIL notification subjects with `[ #]`, where the + // id is zero-padded to at least 7 digits and always preceded by a space + // (see NotificationTargetCommonITILObject::getSubjectPrefix()). Requiring + // that shape prevents foreign references such as `[Ticket#123]` from being + // mistaken for a ticket id. + if (preg_match_all('/\[([^\]]*)\s#(\d{7,})\]/', $subject, $matches, PREG_SET_ORDER) > 0) { + $candidate = null; + if (count($matches) === 1) { + // Single match: trust it, even if the entity tag has been edited + // since the notification was sent. + $candidate = $matches[0][2]; + } else { + // Multiple matches: prefer the one whose prefix matches a known + // notification subject tag, and fall back to the last match. + $known_tags = $this->getNotificationSubjectTags(); + foreach ($matches as $match) { + // Keep the last seen id as fallback, but stop as soon as a + // known notification subject tag is found. + $candidate = $match[2]; + if (in_array(trim($match[1]), $known_tags, true)) { + break; + } + } + } + + if ($candidate !== null && $ticket->getFromDB($candidate)) { + return $ticket; + } } } return null; } + /** + * Get the notification subject tags (`[ #]`) configured across the + * entities, plus the default `GLPI` tag. Computed once per collector instance. + * + * @return string[] + */ + private function getNotificationSubjectTags(): array + { + if ($this->notification_subject_tags === null) { + /** @var DBmysql $DB */ + global $DB; + + $tags = ['GLPI']; + $iterator = $DB->request([ + 'SELECT' => 'notification_subject_tag', + 'DISTINCT' => true, + 'FROM' => Entity::getTable(), + 'WHERE' => ['notification_subject_tag' => ['<>', '']], + ]); + foreach ($iterator as $row) { + $tags[] = trim((string) $row['notification_subject_tag']); + } + + $this->notification_subject_tags = array_values(array_unique(array_filter($tags))); + } + + return $this->notification_subject_tags; + } + /** * Retrieve the message ID from headers. * If multiple matching headers are found, the first one parsed as a {@link MessageId} is returned. diff --git a/tests/functional/TicketTest.php b/tests/functional/TicketTest.php index 622345f9ca7..899dfe4be52 100644 --- a/tests/functional/TicketTest.php +++ b/tests/functional/TicketTest.php @@ -7483,12 +7483,14 @@ public function testMailCollectorFollowupSetAssignee(string $from_user, int $set '_skip_auto_assign' => true, ]); $ticket_id = $ticket->getID(); + // GLPI zero-pads the ticket id to at least 7 digits in notification subjects. + $padded_ticket_id = sprintf('%07d', $ticket_id); // Build a raw email from the sender replying to the ticket (linked via subject line) $raw = implode("\r\n", [ "From: {$from_user} <{$sender_email}>", "To: helpdesk@glpi.com", - "Subject: Re: [GLPI #{$ticket_id}]", + "Subject: Re: [GLPI #{$padded_ticket_id}]", "Message-ID: ", "Date: Mon, 01 Jan 2024 12:00:00 +0000", "", diff --git a/tests/imap/MailCollectorTest.php b/tests/imap/MailCollectorTest.php index 2e973ef27ef..411d7f9d261 100644 --- a/tests/imap/MailCollectorTest.php +++ b/tests/imap/MailCollectorTest.php @@ -420,6 +420,7 @@ public static function itemReferenceHeaderProvider() $root_ent_id = getItemByTypeName('Entity', '_test_root_entity', true); $ticket_id = getItemByTypeName('Ticket', '_ticket01', true); + $padded_ticket_id = sprintf('%07d', $ticket_id); $ticket_notif = new NotificationTargetTicket($root_ent_id, 'test_event', getItemByTypeName('Ticket', '_ticket01')); $soft_id = getItemByTypeName('SoftwareLicense', '_test_softlic_1', true); @@ -633,6 +634,42 @@ public static function itemReferenceHeaderProvider() 'expected_items_id' => null, 'accepted' => false, ], + // Subject fallback - single GLPI tag, foreign `[Ticket#...]` (no space) ignored + [ + 'headers' => [ + 'subject' => "Re: [GLPI #{$padded_ticket_id}] [Ticket#2026072803024161] Foo", + ], + 'expected_itemtype' => Ticket::class, + 'expected_items_id' => $ticket_id, + 'accepted' => true, + ], + // Subject fallback - a too-short foreign `[Case #4711]` reference is ignored (< 7 digits) + [ + 'headers' => [ + 'subject' => "[Case #4711] Re: [GLPI #{$padded_ticket_id}] Foo", + ], + 'expected_itemtype' => Ticket::class, + 'expected_items_id' => $ticket_id, + 'accepted' => true, + ], + // Subject fallback - multiple matches: the configured tag (GLPI) wins over a foreign one + [ + 'headers' => [ + 'subject' => "[Foreign #1234567] Re: [GLPI #{$padded_ticket_id}] Foo", + ], + 'expected_itemtype' => Ticket::class, + 'expected_items_id' => $ticket_id, + 'accepted' => true, + ], + // Subject fallback - multiple matches, none with a known tag: falls back to the last one + [ + 'headers' => [ + 'subject' => "[Foreign #7777777] Re: [Unknown #{$padded_ticket_id}] Foo", + ], + 'expected_itemtype' => Ticket::class, + 'expected_items_id' => $ticket_id, + 'accepted' => true, + ], ]; }