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

diff --git a/CRM/Utils/Mail/EmailProcessor.php b/CRM/Utils/Mail/EmailProcessor.php
index eb4f991f1c644bed30737c6f3c553b2835357a4f..8904a440b0d7f45e7e927ab468ff81a65af2ae87 100644 (file)
--- 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 a6a70213a22c6d334c3d6ad2917e3b10eadb9371..81bec1dccb65c22b2bb72723b96af5a508ba6551 100644 (file)
--- 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 abf9f055c0366005f952c0f2b28cd87b1012fb66..c11273a5fa0a6ce4b8f22edfef45caf2c8436c3f 100644 (file)
--- 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 a2bd614ab57f31c34220a487bc47bca669da0181..f710c1ff0cb46bb3f7551aa2a4eb685c0be590ab 100644 (file)
--- 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 00c4b74f5b81316e0dac49070b36c7180a4332ee..80c6b70f3c6af3f9ce6e8da5891ffc3f963c74ac 100644 (file)
--- 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.