From 3bc2c6e2767bb37650ff628390b53980a4c5482c Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 7 Sep 2023 20:47:59 +1200
Subject: [PATCH] Move handling of empty from obscure function to main email
handler class
I wondered if the zeta components update would make this fix obsolete but adding
a test says not...
So this locks in the fix on
https://issues.civicrm.org/jira/browse/CRM-19215
---
CRM/Utils/Mail/EmailProcessor.php | 3 +--
CRM/Utils/Mail/Incoming.php | 16 ++++-----------
CRM/Utils/Mail/IncomingMail.php | 14 +++++++++++++
.../Utils/Mail/EmailProcessorInboundTest.php | 20 ++++++++++++++++++-
.../Mail/data/inbound/test_broken_from.eml | 4 ++--
5 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/CRM/Utils/Mail/EmailProcessor.php b/CRM/Utils/Mail/EmailProcessor.php
index eb4f991f1c..8904a440b0 100644
--- a/CRM/Utils/Mail/EmailProcessor.php
+++ b/CRM/Utils/Mail/EmailProcessor.php
@@ -118,7 +118,7 @@ class CRM_Utils_Mail_EmailProcessor {
// if its the activities that needs to be processed ..
try {
- $mailParams = CRM_Utils_Mail_Incoming::parseMailingObject($mail, $incomingMail->getAttachments(), $createContact, $emailFields);
+ $mailParams = CRM_Utils_Mail_Incoming::parseMailingObject($mail, $incomingMail->getAttachments(), $createContact, $emailFields, [$incomingMail->getFrom()]);
$activityParams = [
'activity_type_id' => (int) $dao->activity_type_id,
'campaign_id' => $dao->campaign_id ? (int) $dao->campaign_id : NULL,
@@ -150,7 +150,6 @@ class CRM_Utils_Mail_EmailProcessor {
}
}
}
-
$numAttachments = Civi::settings()->get('max_attachments_backend') ?? CRM_Core_BAO_File::DEFAULT_MAX_ATTACHMENTS_BACKEND;
for ($i = 1; $i <= $numAttachments; $i++) {
if (isset($mailParams["attachFile_$i"])) {
diff --git a/CRM/Utils/Mail/Incoming.php b/CRM/Utils/Mail/Incoming.php
index a6a70213a2..81bec1dccb 100644
--- a/CRM/Utils/Mail/Incoming.php
+++ b/CRM/Utils/Mail/Incoming.php
@@ -290,25 +290,17 @@ class CRM_Utils_Mail_Incoming {
* @param $attachments
* @param bool $createContact
* @param array $emailFields
- * Which fields to process and create contacts for of from, to, cc, bcc
+ * Which fields to process and create contacts for - subset of [from, to, cc, bcc],
+ * @param array $from
*
* @return array
*/
- public static function parseMailingObject(&$mail, $attachments, $createContact = TRUE, $emailFields = ['from', 'to', 'cc', 'bcc']) {
-
- // Sometimes $mail->from is unset because ezcMail didn't handle format
- // of From header. CRM-19215.
- if (!isset($mail->from)) {
- if (preg_match('/^([^ ]*)( (.*))?$/', $mail->getHeader('from'), $matches)) {
- $mail->from = new ezcMailAddress($matches[1], trim($matches[2]));
- }
- }
-
+ public static function parseMailingObject(&$mail, $attachments, $createContact, $emailFields, $from) {
$params = [];
foreach ($emailFields as $field) {
// to, bcc, cc are arrays of objects, but from is an object, so make it an array of one object so we can handle it the same
if ($field === 'from') {
- $value = [$mail->$field];
+ $value = $from;
}
else {
$value = $mail->$field;
diff --git a/CRM/Utils/Mail/IncomingMail.php b/CRM/Utils/Mail/IncomingMail.php
index abf9f055c0..c11273a5fa 100644
--- a/CRM/Utils/Mail/IncomingMail.php
+++ b/CRM/Utils/Mail/IncomingMail.php
@@ -95,6 +95,13 @@ class CRM_Utils_Mail_IncomingMail {
return $this->hash;
}
+ /**
+ * @return ezcMailAddress
+ */
+ public function getFrom(): ezcMailAddress {
+ return $this->mail->from;
+ }
+
/**
* @return string
*/
@@ -126,6 +133,13 @@ class CRM_Utils_Mail_IncomingMail {
* @throws \CRM_Core_Exception
*/
public function __construct(ezcMail $mail, string $emailDomain, string $emailLocalPart) {
+ // Sometimes $mail->from is unset because ezcMail didn't handle format
+ // of From header. CRM-19215 (https://issues.civicrm.org/jira/browse/CRM-19215).
+ if (!isset($mail->from)) {
+ if (preg_match('/^([^ ]*)( (.*))?$/', $mail->getHeader('from'), $matches)) {
+ $mail->from = new ezcMailAddress($matches[1], trim($matches[2]));
+ }
+ }
$this->mail = $mail;
$this->body = CRM_Utils_Mail_Incoming::formatMailPart($mail->body, $this->attachments);
diff --git a/tests/phpunit/CRM/Utils/Mail/EmailProcessorInboundTest.php b/tests/phpunit/CRM/Utils/Mail/EmailProcessorInboundTest.php
index a2bd614ab5..f710c1ff0c 100644
--- a/tests/phpunit/CRM/Utils/Mail/EmailProcessorInboundTest.php
+++ b/tests/phpunit/CRM/Utils/Mail/EmailProcessorInboundTest.php
@@ -1,5 +1,7 @@
<?php
+use Civi\Api4\ActivityContact;
+
/**
* Class CRM_Utils_Mail_EmailProcessorInboundTest
* @group headless
@@ -152,12 +154,28 @@ class CRM_Utils_Mail_EmailProcessorInboundTest extends CiviUnitTestCase {
* Test messed up from.
*
* This ensures fix for https://issues.civicrm.org/jira/browse/CRM-19215.
+ *
+ * @throws \CRM_Core_Exception
*/
public function testBadFrom() :void {
$email = file_get_contents(__DIR__ . '/data/inbound/test_broken_from.eml');
+ $badEmails = [
+ 'foo@example.com' => 'foo@example.com (foo)',
+ "KO'Bananas@benders.com" => "KO'Bananas@benders.com",
+ ];
+ foreach ($badEmails as $index => $badEmail) {
+ $file = fopen(__DIR__ . '/data/mail/test_broken_from.eml' . $index, 'wb');
+ fwrite($file, str_replace('bad-email-placeholder', $badEmail, $email));
+ fclose($file);
+ }
- copy(__DIR__ . '/data/inbound/test_broken_from.eml', __DIR__ . '/data/mail/test_broken_from.eml');
$this->callAPISuccess('Job', 'fetch_activities', []);
+ $activities = ActivityContact::get()
+ ->addSelect('contact_id.email_primary.email', 'activity_id.activity_type_id:name', 'activity_id.subject')
+ ->addWhere('contact_id.email_primary.email', 'IN', array_keys($badEmails))
+ ->addWhere('record_type_id:name', '=', 'Activity Source')
+ ->execute();
+ $this->assertCount(2, $activities);
}
/**
diff --git a/tests/phpunit/CRM/Utils/Mail/data/inbound/test_broken_from.eml b/tests/phpunit/CRM/Utils/Mail/data/inbound/test_broken_from.eml
index 00c4b74f5b..80c6b70f3c 100644
--- a/tests/phpunit/CRM/Utils/Mail/data/inbound/test_broken_from.eml
+++ b/tests/phpunit/CRM/Utils/Mail/data/inbound/test_broken_from.eml
@@ -8,8 +8,8 @@ MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
-From: foo@example.com (foo)
+From: bad-email-placeholder
To: jjj@myorg.org
-Subject: This is for hooks
+Subject: Subject
Sample body for hook test.
--
2.25.1