Fix: mail collector attaches reply to the wrong ticket when subject holds a foreign reference

.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',

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 `[<tag> #<id>]`, 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 (`[<tag> #<id>]`) 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.

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: <test-{$from_user}-followup@glpi-test.com>",
             "Date: Mon, 01 Jan 2024 12:00:00 +0000",
             "",

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,
+            ],
         ];
     }