fix(activesync): separate PING watermark from SYNC modseq to stop iOS…

lib/Horde/ActiveSync/Collections.php

@@ -1089,6 +1089,7 @@ public function save($preserve_folders = false)
      *
      * @throws Horde_ActiveSync_Exception_InvalidRequest
      * @throws Horde_ActiveSync_Exception_FolderGone
+     * @throws Horde_ActiveSync_Exception_StaleState
      */
     public function initCollectionState(array &$collection, $requireSyncKey = false)
     {
@@ -1239,6 +1240,23 @@ public function pollForChanges($heartbeat, $interval, array $options = [])
                 // Initialize the collection's state data in the state handler.
                 try {
                     $this->initCollectionState($collection, true);
+                } catch (Horde_ActiveSync_Exception_StaleState $e) {
+                    $this->_logger->notice(
+                        sprintf(
+                            'COLLECTIONS: Corrupt state for %s; forcing collection resync: %s',
+                            $id,
+                            $e->getMessage()
+                        )
+                    );
+                    $this->_as->state->loadState(
+                        [],
+                        null,
+                        Horde_ActiveSync::REQUEST_TYPE_SYNC,
+                        $id
+                    );
+                    $this->setGetChangesFlag($id);
+                    $dataavailable = true;
+                    continue;
                 } catch (Horde_ActiveSync_Exception_StateGone $e) {
                     if (!empty($options['pingable'])) {
                         $this->_logger->notice(

lib/Horde/ActiveSync/Connector/Exporter/Sync.php

@@ -52,10 +52,13 @@ public function setChanges($changes, $collection = null)
 
     /**
      * Sends the next change in the set to the client.
+     * Send the next change to the client.
      *
-     * @return boolean|Horde_Exception True if more changes can be sent false if
-     *                                 all changes were sent, Horde_Exception if
-     *                                 there was an error sending an item.
+     * @return boolean  True if a change was exported successfully. False if
+     *                  there are no more changes or the export must stop
+     *                  (caller should break out of the send loop).
+     *                  Throws on temporary/fatal failures instead of returning
+     *                  an exception object.
      */
     public function sendNextChange()
     {
@@ -452,7 +455,9 @@ protected function _getNextChange()
     /**
      * Sends the next message change to the client.
      *
-     * @return @see self::sendNextChange()
+     * @return boolean  True on successful export, false to stop the loop.
+     *
+     * @throws Horde_ActiveSync_Exception_TemporaryFailure
      */
     protected function _sendNextChange()
     {
@@ -493,9 +498,12 @@ protected function _sendNextChange()
                             'Message gone or error reading message from server: %s',
                             $e->getMessage()
                         ));
+                        // Permanently unavailable on the mail server (expunged,
+                        // corrupt, or otherwise unreadable). Drop from pending
+                        // and continue exporting the rest of the batch.
                         $this->_as->state->updateState($change['type'], $change);
                         $this->_step++;
-                        return $e;
+                        return true;
                     } catch (Horde_ActiveSync_Exception_TemporaryFailure $e) {
                         $this->_logger->err(
                             sprintf(
@@ -511,9 +519,11 @@ protected function _sendNextChange()
                                 $e->getMessage()
                             )
                         );
-                        $this->_as->state->updateState($change['type'], $change);
-                        $this->_step++;
-                        return $e;
+                        /* Do not drop pending changes on export failure.
+                         * Return false to break the caller's send loop; the
+                         * remaining batch stays in sync_pending for the next
+                         * SYNC request. */
+                        return false;
                     }
                     break;
 

lib/Horde/ActiveSync/Folder/Imap.php

@@ -102,6 +102,20 @@ class Horde_ActiveSync_Folder_Imap extends Horde_ActiveSync_Folder_Base implemen
      */
     protected $_primed = false;
 
+    /**
+     * PING watermark (IMAP STATUS last acknowledged to the client).
+     *
+     * Part of the three-state email sync model:
+     *   - $_status / modseq()     — SYNC watermark (CHANGEDSINCE)
+     *   - $_pingStatus            — PING watermark (this property)
+     *   - horde_activesync_state.sync_pending — MOREAVAILABLE batch (SYNC only)
+     *
+     * @see Horde_ActiveSync_State_Base::getChanges() for the PING vs SYNC paths.
+     *
+     * @var array
+     */
+    protected $_pingStatus = [];
+
     /**
      * Set message changes.
      *
@@ -269,6 +283,7 @@ public function setRemoved(array $uids)
      */
     public function updateState()
     {
+        $deferInitialComplete = false;
         if (empty($this->_status[self::HIGHESTMODSEQ])) {
             $this->_messages = array_diff(array_keys($this->_messages), $this->_removed);
             foreach ($this->_added as $add) {
@@ -277,8 +292,11 @@ public function updateState()
             $this->_messages = $this->_flags + array_flip($this->_messages);
         } else {
             if ($this->_primed) {
-                $this->_messages = $this->_added;
+                // CONDSTORE initial priming: the full UID list is exported via
+                // sync_pending. Only add UIDs to _messages as they are actually
+                // sent to the client (acknowledgeExportedMessage()).
                 $this->_primed = false;
+                $deferInitialComplete = true;
             } else {
                 foreach ($this->_added as $add) {
                     $this->_messages[] = $add;
@@ -293,9 +311,138 @@ public function updateState()
         $this->_changed = [];
         $this->_flags = [];
         $this->_softDeleted = [];
+        if (!$deferInitialComplete) {
+            $this->haveInitialSync = true;
+        }
+        $this->_syncPingStatus();
+    }
+
+    /**
+     * Track a message UID successfully exported to the client.
+     *
+     * During CONDSTORE initial sync the server's _messages cache must reflect
+     * only mail the client has actually received, not the full primed UID
+     * list polled from IMAP.
+     *
+     * @param integer|string $uid  The IMAP UID.
+     */
+    public function acknowledgeExportedMessage($uid)
+    {
+        if ($uid === '' || $uid === null) {
+            return;
+        }
+
+        if (empty($this->_status[self::HIGHESTMODSEQ])) {
+            $this->_messages[$uid] = $uid;
+        } else {
+            $this->_messages[] = $uid;
+        }
+    }
+
+    /**
+     * Mark an initial folder sync as complete.
+     *
+     * Called once sync_pending is empty and all MOREAVAILABLE batches have
+     * been delivered to the client.
+     */
+    public function markInitialSyncComplete()
+    {
         $this->haveInitialSync = true;
     }
 
+    /**
+     * Return the MODSEQ value used for PING change detection.
+     *
+     * @return integer
+     */
+    public function pingModseq()
+    {
+        if (!empty($this->_pingStatus[self::HIGHESTMODSEQ])) {
+            return $this->_pingStatus[self::HIGHESTMODSEQ];
+        }
+
+        return $this->modseq();
+    }
+
+    /**
+     * Return the UIDNEXT value used for PING change detection.
+     *
+     * @return integer
+     */
+    public function pingUidnext()
+    {
+        if (array_key_exists(self::UIDNEXT, $this->_pingStatus)) {
+            return $this->_pingStatus[self::UIDNEXT];
+        }
+
+        return $this->uidnext();
+    }
+
+    /**
+     * Return the message count used for PING change detection.
+     *
+     * @return integer
+     */
+    public function pingTotalMessages()
+    {
+        if (array_key_exists(self::MESSAGES, $this->_pingStatus)) {
+            return $this->_pingStatus[self::MESSAGES];
+        }
+
+        return $this->total_messages();
+    }
+
+    /**
+     * Advance the PING checkpoint to the current IMAP STATUS.
+     *
+     * @param array $status  An IMAP STATUS result.
+     */
+    public function acknowledgePingStatus(array $status)
+    {
+        $this->_pingStatus = [
+            self::UIDVALIDITY => $status[self::UIDVALIDITY] ?? $this->uidvalidity(),
+            self::UIDNEXT => $status[self::UIDNEXT] ?? $this->uidnext(),
+            self::HIGHESTMODSEQ => $status[self::HIGHESTMODSEQ] ?? 0,
+            self::MESSAGES => $status[self::MESSAGES] ?? 0,
+        ];
+    }
+
+    /**
+     * Align the PING checkpoint with the current SYNC state.
+     */
+    protected function _syncPingStatus()
+    {
+        $synced = [
+            self::UIDVALIDITY => $this->uidvalidity(),
+            self::UIDNEXT => $this->uidnext(),
+            self::HIGHESTMODSEQ => $this->modseq(),
+            self::MESSAGES => $this->total_messages(),
+        ];
+
+        // Never move the PING checkpoint backward. Partial SYNC responses can
+        // leave the SYNC modseq behind the server while the PING checkpoint was
+        // already advanced during PING.
+        if (empty($this->_pingStatus)) {
+            $this->_pingStatus = $synced;
+            return;
+        }
+
+        if (!empty($synced[self::HIGHESTMODSEQ])
+            && (empty($this->_pingStatus[self::HIGHESTMODSEQ])
+                || $synced[self::HIGHESTMODSEQ] > $this->_pingStatus[self::HIGHESTMODSEQ])) {
+            $this->_pingStatus[self::HIGHESTMODSEQ] = $synced[self::HIGHESTMODSEQ];
+        }
+        if ($synced[self::UIDNEXT] > ($this->_pingStatus[self::UIDNEXT] ?? 0)) {
+            $this->_pingStatus[self::UIDNEXT] = $synced[self::UIDNEXT];
+        }
+        if ($synced[self::MESSAGES] != ($this->_pingStatus[self::MESSAGES] ?? null)) {
+            $this->_pingStatus[self::MESSAGES] = $synced[self::MESSAGES];
+        }
+        if (!empty($synced[self::UIDVALIDITY])) {
+            $this->_pingStatus[self::UIDVALIDITY] = $synced[self::UIDVALIDITY];
+        }
+    }
+
     /**
      * Return the folder's UID validity.
      *
@@ -441,17 +588,21 @@ public function serialize()
             $msgs = $this->_messages;
         }
 
-        return json_encode(
-            [
-                's' => $this->_status,
-                'm' => $msgs,
-                'f' => $this->_serverid,
-                'c' => $this->_class,
-                'lsd' => $this->_lastSinceDate,
-                'sd' => $this->_softDelete,
-                'hi' => $this->haveInitialSync,
-                'v' => self::VERSION]
-        );
+        $data = [
+            's' => $this->_status,
+            'm' => $msgs,
+            'f' => $this->_serverid,
+            'c' => $this->_class,
+            'lsd' => $this->_lastSinceDate,
+            'sd' => $this->_softDelete,
+            'hi' => $this->haveInitialSync,
+            'v' => self::VERSION,
+        ];
+        if (!empty($this->_pingStatus)) {
+            $data['ps'] = $this->_pingStatus;
+        }
+
+        return json_encode($data);
     }
 
     /**
@@ -477,7 +628,10 @@ public function unserialize($data)
         $this->_class = $d_data['c'];
         $this->_lastSinceDate = $d_data['lsd'];
         $this->_softDelete = $d_data['sd'];
-        $this->haveInitialSync = empty($d_data['hi']) ? !empty($this->_messages) : $d_data['hi'];
+        $this->haveInitialSync = array_key_exists('hi', $d_data)
+            ? (bool) $d_data['hi']
+            : !empty($this->_messages);
+        $this->_pingStatus = !empty($d_data['ps']) ? $d_data['ps'] : [];
 
         if (!empty($this->_status[self::HIGHESTMODSEQ]) && is_string($this->_messages)) {
             $this->_messages = $this->_fromSequenceString($this->_messages);

lib/Horde/ActiveSync/Imap/Adapter.php

@@ -437,6 +437,12 @@ public function getMessages($folderid, array $messages, array $options = [])
                 try {
                     $ret[] = $this->_buildMailMessage($mbox, $data, $options);
                 } catch (Horde_Exception_NotFound $e) {
+                    $this->_logger->notice(sprintf(
+                        'Unable to build message UID %s in %s: %s',
+                        $data->getUid(),
+                        $folderid,
+                        $e->getMessage()
+                    ));
                 }
             }
         }
@@ -568,19 +574,24 @@ public function ping(Horde_ActiveSync_Folder_Imap $folder)
             )
         );
 
-        // If we have per mailbox MODSEQ then we can pick up flag changes too.
+        // PING path: compare against $_pingStatus, not SYNC modseq() in
+        // $_status. See Horde_ActiveSync_State_Base::getChanges() and
+        // Horde_ActiveSync_Folder_Imap::$_pingStatus.
         $modseq = $status[Horde_ActiveSync_Folder_Imap::HIGHESTMODSEQ];
-        if ($modseq && $folder->modseq() > 0 && $folder->modseq() < $modseq) {
+        if ($modseq && $folder->pingModseq() > 0 && $folder->pingModseq() < $modseq) {
+            $folder->acknowledgePingStatus($status);
             return true;
         }
 
         // Increase in UIDNEXT is always a positive PING.
-        if ($folder->uidnext() < $status['uidnext']) {
+        if ($folder->pingUidnext() < $status['uidnext']) {
+            $folder->acknowledgePingStatus($status);
             return true;
         }
 
         // If the message count changes, something certainly changed.
-        if ($folder->total_messages() != $status['messages']) {
+        if ($folder->pingTotalMessages() != $status['messages']) {
+            $folder->acknowledgePingStatus($status);
             return true;
         }