Avoid competing duplicate early Email folder initialization on iOS

analysis-brief.md

@@ -0,0 +1,80 @@
+\# Problem statement
+
+
+
+Environment:
+
+\- Z-Push 2.7.6
+
+\- BackendCombined with IMAP mail
+
+\- iPhone Mail app via ActiveSync
+
+\- Small folders sync fine
+
+\- Large folders like "Bestellungen" and likely "Sent" trigger loop behavior
+
+\- The same suspect mails sync fine when moved into small test folders
+
+\- IMAP shows the mails normally, so raw mail corruption is unlikely
+
+
+
+Observed behavior:
+
+\- Large folders start to "drip-feed" messages
+
+\- Z-Push enters Mobile loop detected mode
+
+\- Earlier logs showed RFC822 validation and timezone conversion issues
+
+\- Current code inspection suggests the main problem may be in:
+
+  - backend/imap/imap.php
+
+  - lib/default/diffbackend/exportchangesdiff.php
+
+  - lib/core/loopdetection.php
+
+
+
+Evidence:
+
+\- Bestellungen triggers loop detection around internal message ids 340, 342, 347
+
+\- These mapped to normal mails (iFixit, Akku-King, DHL)
+
+\- The same mails are visible normally in Bestellungen-Looptest / Bestellungen-Quarantaene
+
+\- Therefore the issue likely depends on folder/state/diff context, not the raw mails themselves
+
+
+
+Task:
+
+Please perform root-cause analysis first, not a blind fix.
+
+Focus on:
+
+1\. GetMessageList()
+
+2\. ExportChangesDiff->InitializeExporter() / Synchronize()
+
+3\. StatMessage() / GetMessage()
+
+4\. LoopDetection as a consequence, not necessarily as primary cause
+
+
+
+Questions:
+
+\- Why do small folders work while large folders fail?
+
+\- Is there a race/inconsistency between GetMessageList(), StatMessage(), and GetMessage()?
+
+\- Is the diff engine unstable for large IMAP folders?
+
+\- Are there code paths that can repeatedly re-emit the same changes and trigger loop detection?
+
+\- Propose the smallest safe patch and explain the risk.
+

src/backend/imap/imap.php

@@ -53,6 +53,7 @@ class BackendIMAP extends BackendDiff implements ISearchProvider {
     private $excludedFolders;
     private static $mimeTypes = false;
     private $imapParams = array();
+    private $forwardedMessagesCache = array();
 
     private $dontStat = array();            //keys in this array represent mailboxes which can't be stat'd (ie, /NoSELECT status)
 
@@ -1033,16 +1034,16 @@ public function GetMessageList($folderid, $cutoffdate) {
 
             $sequence = implode(",", $search);
 
-            // search for forwarded messages in time range, because imap_fetch_overview() does not return $Forwarded flag
-            $forwardedMessages = @imap_search($this->mbox, 'KEYWORD $Forwarded ' . $searchCriteria, SE_UID);
         }
         else {
             $sequence = "1:*";
-
-            // search for forwarded messages
-            $forwardedMessages = @imap_search($this->mbox, 'KEYWORD $Forwarded', SE_UID);
         }
 
+        // imap_fetch_overview() does not expose $Forwarded. Cache the folder-wide
+        // UID set per request to avoid doing a full-folder keyword search for every
+        // message/stat lookup in large folders.
+        $forwardedMessages = $this->getForwardedMessageSet($folderid);
+
         ZLog::Write(LOGLEVEL_DEBUG, sprintf("BackendIMAP->GetMessageList(): searching with sequence '%s'", $sequence));
         $overviews = @imap_fetch_overview($this->mbox, $sequence);
 
@@ -1094,7 +1095,7 @@ public function GetMessageList($folderid, $cutoffdate) {
                 }
 
                 // 'forwarded'
-                if (is_array($forwardedMessages) && in_array($overview->uid, $forwardedMessages)) {
+                if (isset($forwardedMessages[$overview->uid])) {
                     $message["forwarded"] = 1;
                 }
                 else {
@@ -1135,8 +1136,9 @@ public function GetMessage($folderid, $id, $contentparameters) {
 
         $is_sent_folder = strcasecmp($folderImapid, $this->create_name_folder(IMAP_FOLDER_SENT)) == 0;
 
-        // Get flags, etc
-        $stat = $this->StatMessage($folderid, $id);
+        // Read the message overview directly to avoid a redundant StatMessage() call
+        // from the exporter path.
+        $stat = $this->getMessageStat($folderid, $id, $folderImapid);
 
         if ($stat) {
             $this->imap_reopen_folder($folderImapid);
@@ -1563,6 +1565,24 @@ public function StatMessage($folderid, $id) {
         ZLog::Write(LOGLEVEL_DEBUG, sprintf("BackendIMAP->StatMessage('%s','%s')", $folderid, $id));
         $folderImapid = $this->getImapIdFromFolderId($folderid);
 
+        return $this->getMessageStat($folderid, $id, $folderImapid);
+    }
+
+    /**
+     * Returns message stats, analogous to the folder stats from StatFolder().
+     *
+     * @param string        $folderid       id of the folder
+     * @param string        $id             id of the message
+     * @param string        $folderImapid   (opt) IMAP folder id if already known
+     *
+     * @access private
+     * @return array/boolean
+     */
+    private function getMessageStat($folderid, $id, $folderImapid = false) {
+        if ($folderImapid === false) {
+            $folderImapid = $this->getImapIdFromFolderId($folderid);
+        }
+
         $this->imap_reopen_folder($folderImapid);
         $overview = @imap_fetch_overview($this->mbox, $id, FT_UID);
 
@@ -1574,8 +1594,7 @@ public function StatMessage($folderid, $id) {
         // without uid it's not a valid message
         if (empty($overview[0]->uid)) return false;
 
-        // search for the specific uid and keyword/flag, because imap_fetch_overview() does not return $Forwarded flag
-        $forwardedMessages = @imap_search($this->mbox, 'KEYWORD $Forwarded', SE_UID);
+        $forwardedMessages = $this->getForwardedMessageSet($folderImapid);
 
         $entry = array();
         if (isset($overview[0]->udate)) {
@@ -1604,7 +1623,7 @@ public function StatMessage($folderid, $id) {
         }
 
         // 'forwarded'
-        if (is_array($forwardedMessages) && in_array($overview[0]->uid, $forwardedMessages)) {
+        if (isset($forwardedMessages[$overview[0]->uid])) {
             $entry["forwarded"] = 1;
         }
         else {
@@ -1622,6 +1641,27 @@ public function StatMessage($folderid, $id) {
         return $entry;
     }
 
+    /**
+     * Returns a UID lookup set of forwarded messages for the currently opened folder.
+     *
+     * @param string $folderImapid
+     *
+     * @access private
+     * @return array
+     */
+    private function getForwardedMessageSet($folderImapid) {
+        if (isset($this->forwardedMessagesCache[$folderImapid])) {
+            return $this->forwardedMessagesCache[$folderImapid];
+        }
+
+        $forwardedMessages = @imap_search($this->mbox, 'KEYWORD $Forwarded', SE_UID);
+        $this->forwardedMessagesCache[$folderImapid] = is_array($forwardedMessages)
+            ? array_fill_keys($forwardedMessages, true)
+            : array();
+
+        return $this->forwardedMessagesCache[$folderImapid];
+    }
+
     /**
      * Called when a message has been changed on the mobile.
      * Added support for FollowUp flag

src/backend/imap/mime_encode.php

@@ -204,7 +204,7 @@ function build_mime_message($message) {
                 $mimeHeaders['content_type'] = $new_value;
                 break;
             case 'content-transfer-encoding':
-                if (strcasecmp($value, "base64") == 0 || strcasecmp($value, "binary") == 0) {
+                if (is_string($value) && (strcasecmp($value, "base64") == 0 || strcasecmp($value, "binary") == 0)) {
                     $mimeHeaders['encoding'] = "base64";
                 }
                 else {
@@ -325,4 +325,4 @@ function is_encrypted($message) {
  */
 function is_multipart($message) {
     return isset($message->ctype_primary) && $message->ctype_primary == "multipart";
-}
+}

src/backend/imap/user_identity.php

@@ -208,9 +208,14 @@ function getIdentityFromSql($username, $domain, $identity, $encode = true) {
         $dbh = new PDO(IMAP_FROM_SQL_DSN, IMAP_FROM_SQL_USER, IMAP_FROM_SQL_PASSWORD, unserialize(IMAP_FROM_SQL_OPTIONS));
         ZLog::Write(LOGLEVEL_DEBUG, sprintf("BackendIMAP->getIdentityFromSql() - Connected to SQL Database"));
 
-        $sql = str_replace('#username', $username, str_replace('#domain', $domain, IMAP_FROM_SQL_QUERY));
+        //replace config placeholdes with parameter placeholders
+        $sql = str_replace("'#username'", ":username", str_replace("'#domain'", ":domain", str_replace("'#username@#domain'", ":usernameatdomain", IMAP_FROM_SQL_QUERY)));
+        $usernameatdomain = $username . '@' . $domain;
         ZLog::Write(LOGLEVEL_DEBUG, sprintf("BackendIMAP->getIdentityFromSql() - Searching From with filter: %s", $sql));
         $sth = $dbh->prepare($sql);
+        if(mb_strpos($sql, ':username') !== false) $sth->bindValue(':username', $username, PDO::PARAM_STR);
+        if(mb_strpos($sql, ':domain') !== false) $sth->bindValue(':domain', $domain, PDO::PARAM_STR);
+        if(mb_strpos($sql, ':usernameatdomain') !== false) $sth->bindValue(':usernameatdomain', $usernameatdomain, PDO::PARAM_STR);
         $sth->execute();
         $record = $sth->fetch(PDO::FETCH_ASSOC);
         if ($record) {

src/include/mimeDecode.php

@@ -330,25 +330,12 @@ function _decode($headers, $body, $default_ctype = 'text/plain')
         if (isset($content_type)) {
             switch (strtolower($content_type['value'])) {
                 case 'text/plain':
-                    $encoding = isset($content_transfer_encoding) ? $content_transfer_encoding['value'] : '7bit';
-                    $charset = isset($return->ctype_parameters['charset']) ? $return->ctype_parameters['charset'] : $this->_charset;
-                    $this->_include_bodies ? $return->body = ($this->_decode_bodies ? $this->_decodeBody($body, $encoding, $charset, true) : $body) : null;
-                    break;
-
                 case 'text/html':
                     $encoding = isset($content_transfer_encoding) ? $content_transfer_encoding['value'] : '7bit';
                     $charset = isset($return->ctype_parameters['charset']) ? $return->ctype_parameters['charset'] : $this->_charset;
                     $this->_include_bodies ? $return->body = ($this->_decode_bodies ? $this->_decodeBody($body, $encoding, $charset, true) : $body) : null;
                     break;
-
-                case 'multipart/signed': // PGP
-                    $parts = $this->_boundarySplit($body, $content_type['other']['boundary'], true);
-                    $return->parts['msg_body'] = $parts[0];
-                    list($part_header, $part_body) = $this->_splitBodyHeader($parts[1]);
-                    $return->parts['sig_hdr'] = $part_header;
-                    $return->parts['sig_body'] = $part_body;
-                    break;
-
+
                 case 'multipart/encrypted': // #190 encrypted parts will be treated as normal ones
                 case 'multipart/parallel':
                 case 'multipart/appledouble': // Appledouble mail
@@ -367,7 +354,7 @@ function _decode($headers, $body, $default_ctype = 'text/plain')
 
                     $default_ctype = (strtolower($content_type['value']) === 'multipart/digest') ? 'message/rfc822' : 'text/plain';
 
-                    $parts = $this->_boundarySplit($body, $content_type['other']['boundary']);
+                    $parts = $this->_boundarySplit($body, $content_type['other']['boundary'], strtolower($content_type['value']) === 'multipart/signed');
                     for ($i = 0; $i < count($parts); $i++) {
                         list($part_header, $part_body) = $this->_splitBodyHeader($parts[$i]);
                         $part = $this->_decode($part_header, $part_body, $default_ctype);
@@ -404,6 +391,7 @@ function _decode($headers, $body, $default_ctype = 'text/plain')
                     }
                     // if there is no explicit charset, then don't try to convert to default charset, and make sure that only text mimetypes are converted
                     $charset = (isset($return->ctype_parameters['charset']) && ((isset($return->ctype_primary) && $return->ctype_primary == 'text') || !isset($return->ctype_primary)) ) ? $return->ctype_parameters['charset'] : '';
+                    $part = new stdClass();
                     $part->body = ($this->_decode_bodies ? $this->_decodeBody($body, $content_transfer_encoding['value'], $charset, false) : $body);
                     $ctype = explode('/', strtolower($content_type['value']));
                     $part->ctype_parameters['name'] = 'smime.p7m';

src/include/z_carddav.php

@@ -657,9 +657,10 @@ private function simplify($response, $include_vcards = true, $remove_duplicates
 
             foreach ($xml->response as $response) {
                 if (isset($response->propstat)) {
-                    if ((strlen($this->url_vcard_extension) > 0 && preg_match('/'.$this->url_vcard_extension.'/', $response->href) &&
+                    if ((((strlen($this->url_vcard_extension) > 0 && preg_match('/'.$this->url_vcard_extension.'/', $response->href)) 
+                        || preg_match('/vcard/', $response->propstat->prop->getcontenttype)) &&
                         !(isset($response->propstat->prop->resourcetype) && isset($response->propstat->prop->resourcetype->addressbook)))
-                        || preg_match('/vcard/', $response->propstat->prop->getcontenttype) || isset($response->propstat->prop->{'address-data'}) || isset($response->propstat->prop->{'addressbook-data'})) {
+                        || isset($response->propstat->prop->{'address-data'}) || isset($response->propstat->prop->{'addressbook-data'})) {
                         // It's a vcard
                         $id = basename($response->href);
                         $id = str_replace($this->url_vcard_extension, null, $id);

src/lib/core/loopdetection.php

@@ -683,7 +683,6 @@ public function Detect($folderid, $uuid, $counter, $maxItems, $queuedMessages) {
         ZLog::Write(LOGLEVEL_DEBUG, sprintf("LoopDetection->Detect(): folderid:'%s' uuid:'%s' counter:'%s' max:'%s' queued:'%s'", $folderid, $uuid, $counter, $maxItems, $queuedMessages));
         $this->broken_message_uuid = $uuid;
         $this->broken_message_counter = $counter;
-
         // if an incoming loop is already detected, do nothing
         if ($maxItems === 0 && $queuedMessages > 0) {
             ZPush::GetTopCollector()->AnnounceInformation("Incoming loop!", true);
@@ -705,8 +704,9 @@ public function Detect($folderid, $uuid, $counter, $maxItems, $queuedMessages) {
             $current = $loopdata[self::$devid][self::$user][$folderid];
 
             // completely new/unknown UUID
-            if (empty($current))
+            if (empty($current)) {
                 $current = array("uuid" => $uuid, "count" => $counter-1, "queued" => $queuedMessages);
+            }
 
             // old UUID in cache - the device requested a new state!!
             if (isset($current['uuid']) && $current['uuid'] != $uuid ) {
@@ -740,7 +740,6 @@ public function Detect($folderid, $uuid, $counter, $maxItems, $queuedMessages) {
 
                 // case 1 - standard, during loop-resolving & resolving
                 if ($current['count'] < $counter) {
-
                     // case 1.1
                     $current['count'] = $counter;
                     $current['queued'] = $queuedMessages;
@@ -779,7 +778,6 @@ public function Detect($folderid, $uuid, $counter, $maxItems, $queuedMessages) {
 
                 // case 3 - same counter, changes sent before, hanging loop and ignoring
                 else if ($current['count'] == $counter && $current['queued'] > 0) {
-
                     if (!isset($current['loopcount'])) {
                         // ZP-1213 we are potentially synching a lot of data, e.g. OL with 512 WindowSize
                         // In case there are more then 40 items in the last request, we limit to 25 items

src/lib/default/diffbackend/diffstate.php

@@ -38,7 +38,6 @@ class DiffState implements IChanges {
     protected $cutoffdate;
     protected $moveSrcState;
     protected $moveDstState;
-
     /**
      * Initializes the state
      *
@@ -159,6 +158,9 @@ protected function getDiffTo($new) {
                 // Message in new seems to be new (add)
                 $change["type"] = "change";
                 $change['flags'] = SYNC_NEWMESSAGE;
+                // Keep the snapshot state that triggered this diff so exporters can
+                // commit a stable state even if the live backend view changes later.
+                $change["stat"] = $item;
                 $changes[] = $change;
             } else {
                 // Both messages are still available, compare states
@@ -174,26 +176,31 @@ protected function getDiffTo($new) {
                 if(isset($old_item["answered"], $item["answered"]) && $old_item["answered"] != $item["answered"]) {
                     // 'answered' changed
                     $change["type"] = "change";
+                    $change["stat"] = $item;
                     $changes[] = $change;
                 }
                 elseif(isset($old_item["forwarded"], $item["forwarded"]) && $old_item["forwarded"] != $item["forwarded"]) {
                     // 'forwarded' changed
                     $change["type"] = "change";
+                    $change["stat"] = $item;
                     $changes[] = $change;
                 }
                 elseif(isset($old_item["star"], $item["star"]) && $old_item["star"] != $item["star"]) {
                     // 'flagged' aka 'FollowUp' aka 'starred' changed
                     $change["type"] = "change";
+                    $change["stat"] = $item;
                     $changes[] = $change;
                 }
                 elseif(isset($old_item['mod'], $item['mod']) && $old_item['mod'] != $item['mod']) {
                     // message modified
                     $change["type"] = "change";
+                    $change["stat"] = $item;
                     $changes[] = $change;
                 }
                 elseif(isset($old_item['mod']) xor isset($item['mod'])) {
                     // modified date missing
                     $change["type"] = "change";
+                    $change["stat"] = $item;
                     $changes[] = $change;
                 }
 
@@ -227,7 +234,7 @@ protected function getDiffTo($new) {
     protected function updateState($type, $change) {
         // Change can be a change or an add
         if($type == "change") {
-            for($i=0; $i < count($this->syncstate); $i++) {
+            for($i=0; $i < count((array)$this->syncstate); $i++) {
                 if($this->syncstate[$i]["id"] == $change["id"]) {
                     $this->syncstate[$i] = $change;
                     return;
@@ -236,7 +243,7 @@ protected function updateState($type, $change) {
             // Not found, add as new
             $this->syncstate[] = $change;
         } else {
-            for($i=0; $i < count($this->syncstate); $i++) {
+            for($i=0; $i < count((array)$this->syncstate); $i++) {
                 // Search for the entry for this item
                 if($this->syncstate[$i]["id"] == $change["id"]) {
                     if($type == "flags") {