diff --git a/lib/Address.php b/lib/Address.php index 6353da61c5..a0aac170c3 100644 --- a/lib/Address.php +++ b/lib/Address.php @@ -29,14 +29,21 @@ private function __construct(Horde_Mail_Rfc822_Address $wrapped) { $this->wrapped = $wrapped; } - public static function fromHorde(Horde_Mail_Rfc822_Address $horde): self { + public static function fromHorde(Horde_Mail_Rfc822_Address $horde, bool $normalize = false): self { + if ($normalize) { + return self::fromRaw($horde->personal, $horde->bare_address, $normalize); + } return new self($horde); } - public static function fromRaw(string $label, string $email): self { - $wrapped = new Horde_Mail_Rfc822_Address($email); + public static function fromRaw(?string $label, string $email, bool $normalize = false): self { + if ($normalize) { + $wrapped = new Horde_Mail_Rfc822_Address(self::normalizeAddress($email)); + } else { + $wrapped = new Horde_Mail_Rfc822_Address($email); + } // If no label is set we use the email - if ($label !== $email) { + if ($label !== null && $label !== $email) { $wrapped->personal = $label; } return new self($wrapped); @@ -117,4 +124,13 @@ public function equals($object): bool { return $this->getEmail() === $object->getEmail() && $this->getLabel() === $object->getLabel(); } + + private static function normalizeAddress(string $address): string { + // remove single quotes and whitespace the might exist + // Examples: + // user1@example.com + // 'user1@example.com' + // ' user1@example.com' + return strtolower(trim(trim($address, "'"))); + } } diff --git a/lib/AddressList.php b/lib/AddressList.php index eefdf458b9..1d4212e240 100644 --- a/lib/AddressList.php +++ b/lib/AddressList.php @@ -47,9 +47,9 @@ public static function parse($str) { * @param Horde_Mail_Rfc822_List $hordeList * @return AddressList */ - public static function fromHorde(Horde_Mail_Rfc822_List $hordeList) { - $addresses = array_map(static function (Horde_Mail_Rfc822_Address $addr) { - return Address::fromHorde($addr); + public static function fromHorde(Horde_Mail_Rfc822_List $hordeList, bool $normalize = false): self { + $addresses = array_map(static function (Horde_Mail_Rfc822_Address $addr) use ($normalize) { + return Address::fromHorde($addr, $normalize); }, array_filter(iterator_to_array($hordeList), static function (Horde_Mail_Rfc822_Object $obj) { // TODO: how to handle non-addresses? This doesn't seem right … return $obj instanceof Horde_Mail_Rfc822_Address; diff --git a/lib/BackgroundJob/RepairRecipients.php b/lib/BackgroundJob/RepairRecipients.php new file mode 100644 index 0000000000..e7b0e2b123 --- /dev/null +++ b/lib/BackgroundJob/RepairRecipients.php @@ -0,0 +1,58 @@ +setInterval(300); + } + + protected function run($argument): void { + // fetch all quoted emails + $select = $this->db->getQueryBuilder(); + $select->select('id', 'email') + ->from('mail_recipients') + ->where( + $select->expr()->like('email', $select->createNamedParameter('\'%\'', IQueryBuilder::PARAM_STR)) + ) + ->setMaxResults(1000); + $recipients = $select->executeQuery()->fetchAll(); + // update emails + $update = $this->db->getQueryBuilder(); + $update->update('mail_recipients') + ->set('email', $update->createParameter('email')) + ->where($update->expr()->in('id', $update->createParameter('id'), IQueryBuilder::PARAM_STR)); + foreach ($recipients as $recipient) { + $id = $recipient['id']; + $email = $recipient['email']; + $email = trim(str_replace('\'', '', (string)$email)); + $update->setParameter('id', $id, IQueryBuilder::PARAM_STR); + $update->setParameter('email', $email, IQueryBuilder::PARAM_STR); + $update->executeStatement(); + } + // remove job depending on the result + if ($recipients === []) { + $this->jobService->remove(RepairRecipients::class); + } + } + +} diff --git a/lib/IMAP/ImapMessageFetcher.php b/lib/IMAP/ImapMessageFetcher.php index aacacd5e0e..dc51500342 100644 --- a/lib/IMAP/ImapMessageFetcher.php +++ b/lib/IMAP/ImapMessageFetcher.php @@ -250,11 +250,11 @@ public function fetchMessage(?Horde_Imap_Client_Data_Fetch $fetch = null): IMAPM $this->uid, $envelope->message_id, $fetch->getFlags(), - AddressList::fromHorde($envelope->from), - AddressList::fromHorde($envelope->to), - AddressList::fromHorde($envelope->cc), - AddressList::fromHorde($envelope->bcc), - AddressList::fromHorde($envelope->reply_to), + AddressList::fromHorde($envelope->from, true), + AddressList::fromHorde($envelope->to, true), + AddressList::fromHorde($envelope->cc, true), + AddressList::fromHorde($envelope->bcc, true), + AddressList::fromHorde($envelope->reply_to, true), $this->decodeSubject($envelope), $this->plainMessage, $this->htmlMessage, diff --git a/lib/Migration/Version5000Date20250507000000.php b/lib/Migration/Version5000Date20250507000000.php new file mode 100644 index 0000000000..8dc5cc78a1 --- /dev/null +++ b/lib/Migration/Version5000Date20250507000000.php @@ -0,0 +1,28 @@ +jobService->add(\OCA\Mail\BackgroundJob\RepairRecipients::class); + } + +} diff --git a/lib/Service/PhishingDetection/PhishingDetectionService.php b/lib/Service/PhishingDetection/PhishingDetectionService.php index 4a3fb48e2d..4c3e084a1e 100644 --- a/lib/Service/PhishingDetection/PhishingDetectionService.php +++ b/lib/Service/PhishingDetection/PhishingDetectionService.php @@ -33,7 +33,7 @@ public function checkHeadersForPhishing(Horde_Mime_Headers $headers, bool $hasHt $customEmail = null; $fromHeader = $headers->getHeader('From'); if ($fromHeader instanceof Horde_Mime_Headers_Element_Address) { - $firstAddr = AddressList::fromHorde($fromHeader->getAddressList(true))?->first(); + $firstAddr = AddressList::fromHorde($fromHeader->getAddressList(true), true)->first(); $fromFN = $firstAddr?->getLabel(); $fromEmail = $firstAddr?->getEmail(); $customEmail = $firstAddr?->getCustomEmail(); @@ -45,7 +45,7 @@ public function checkHeadersForPhishing(Horde_Mime_Headers $headers, bool $hasHt if ($replyToHeader instanceof Horde_Mime_Headers_Element_Address) { $replyToAddrs = $replyToHeader->getAddressList(true); if (isset($replyToAddrs)) { - $replyToEmail = AddressList::fromHorde($replyToAddrs)->first()?->getEmail(); + $replyToEmail = AddressList::fromHorde($replyToAddrs, true)->first()?->getEmail(); } } diff --git a/tests/AddressTest.php b/tests/AddressTest.php index 6c24380d68..07b3afa889 100644 --- a/tests/AddressTest.php +++ b/tests/AddressTest.php @@ -68,4 +68,36 @@ public function testDoesNotEqualBecauseDifferentLabel() { $this->assertFalse($equals); } + + public function testNormalizedWithSingleQuotes() { + $address = Address::fromRaw(null, "'user1@test.com'", true)->toHorde(); + $this->assertEquals('user1@test.com', $address->bare_address); + + $address = Address::fromHorde(new Horde_Mail_Rfc822_Address("'user1@test.com'"), true)->toHorde(); + $this->assertEquals('user1@test.com', $address->bare_address); + } + + public function testUnnormalizedWithSingleQuotes() { + $address = Address::fromRaw(null, "'user1@test.com'", false)->toHorde(); + $this->assertEquals("'user1@test.com'", $address->bare_address); + + $address = Address::fromHorde(new Horde_Mail_Rfc822_Address("'user1@test.com'"), false)->toHorde(); + $this->assertEquals("'user1@test.com'", $address->bare_address); + } + + public function testNormalizedWithUpperCaseLetters() { + $address = Address::fromRaw(null, 'UserOne@test.com', true)->toHorde(); + $this->assertEquals('userone@test.com', $address->bare_address); + + $address = Address::fromHorde(new Horde_Mail_Rfc822_Address('UserOne@test.com'), true)->toHorde(); + $this->assertEquals('userone@test.com', $address->bare_address); + } + + public function testUnnormalizedWithUpperCaseLetters() { + $address = Address::fromRaw(null, 'UserOne@test.com', false)->toHorde(); + $this->assertEquals('UserOne@test.com', $address->bare_address); + + $address = Address::fromHorde(new Horde_Mail_Rfc822_Address('UserOne@test.com'), false)->toHorde(); + $this->assertEquals('UserOne@test.com', $address->bare_address); + } }